Making them return next insertion point with `nsresult` makes the callers
easier to understand. Therefore, this patch declares `MoveNodeResult` class
newly and make them use it.
And also we can get rid of the case of setting `*aInOutDestOffset` to -1
because it's a hacky case of `HTMLEditRules::MoveBlock()`. If we keep it,
`MoveNodeSmart()` needs to compute new insertion point with different logic.
Therefore, this patch makes `HTMLEditRules::MoveBlock()` adjust new insertion
point with checking its argument.
Differential Revision: https://phabricator.services.mozilla.com/D44199
--HG--
extra : moz-landing-system : lando
This is defined as too complicated than what it does actually since there
was not `EditorDOMPointBase`.
If given point's offset is `0`, returns `nullptr`. If `aWhere` is
`BRLocation::blockEnd`, treats `aOffset` is length of `aBlock`. Otherwise,
using given point. Then, if the `WSRun` start reason is `br`, returns the
start reason node. This means that this method just retrieves invisible
`<br>` element if it starts `WSRun` at given point.
Now, callers can specify end of block with `EditorDOMPointBase::SetToEndOf()`
easier. Therefore, we can make it take only an `EditorDOMPointBase` instance.
Differential Revision: https://phabricator.services.mozilla.com/D44197
--HG--
extra : moz-landing-system : lando
`HTMLEditRules::WillMakeDefinitionList()` just calls
`HTMLEditRules::WillMakeList()` and `HTMLEditRules::WillMakeList()` can be
called as `HTMLEditRules::MakeOrChangeListAndListItemAsSubAction()` so that
we should merge them and make `HTMLEditor` call it directly.
This patch also removes default action part of
`HTMLEditor::MakeOrChangeListAsAction()` because it runs only when
`HTMLEditRules::WillDoAction()` does not return canceled nor error but not
handled, however, it's won't occur since `HTMLEditRules::WillMakeList()`
always sets `aHandled` to `true` when it returns `NS_OK`.
Differential Revision: https://phabricator.services.mozilla.com/D44195
--HG--
extra : moz-landing-system : lando
Additionally, this patch makes it use early-return style with `continue` as
far as making number of changing line minimized.
Differential Revision: https://phabricator.services.mozilla.com/D44194
--HG--
extra : moz-landing-system : lando
Meaningful job of `HTMLEditor::InsertParagraphSeparatorAsSubAction()` is only
calling `HTMLEditRules::WillInsertParagraph()` via
`HTMLEditRules::WillDoAction()`. Therefore, we can move all jobs in them
into `HTMLEditRules::WillInsertParagraph()` and rename it to
`HTMLEditor::InsertParagraphSeparatorAsSubAction()`.
Differential Revision: https://phabricator.services.mozilla.com/D44190
--HG--
extra : moz-landing-system : lando
It's used both by `TextEditRules` and `HTMLEditRules` so that `EditorBase`
should have it instead.
Differential Revision: https://phabricator.services.mozilla.com/D44188
--HG--
extra : moz-landing-system : lando
`HTMLEditRules::WillMakeBasicBlock()` just calls
`HTMLEditor::FormatBlockContainer()` and it's called only for
`EditSubAction::eCreateOrRemoveBlock` and it's used only in
`HTMLEditor::InsertBasicBlockWithTransaction()`. Therefore, we can replace
calling `HTMLEditRules::WillDoAction()` in it with what
`HTMLEditRules::WIllMakeBasicBlock()` does.
First, `HTMLEditRules::WillDoAction()` checks whether first selection range
is editable. If it's not so, it returns `NS_OK` with setting `aCancel` to
`true`. Therefore, this patch moves this part to
`HTMLEditor::CanHandleHTMLEditSubAction()` for making
`HTMLEditor::InsertBasicBlockWithTransaction()` can check it easier.
Next, `HTMLEditor::InsertBasicBlockWithTransaction()` does something if
`HTMLEditRules::WillDoAction()` returns as not handled nor canceled.
However, this special handling requires at least one selection range:
https://searchfox.org/mozilla-central/rev/597a69c70a5cce6f42f159eb54ad1ef6745f5432/editor/libeditor/HTMLEditor.cpp#2284-2288
Surprisingly, `handled` is set to `false` only when there is an error or
there is no selection range. Therefore, this long block has already been
dead code so that we can remove this. Removing this block causes that we
become not throwing exception when `Document.execCommand("formatblock")`
without selection ranges. But this is better since Chrome does not throw
excption.
Finally, this patch renames some related methods:
- `HTMLEditor::FormatBlockContainer()` -> `HTMLEditor::FormatBlockContainerWithTransaction()`
- `HTMLEditor::InsertBasicBlockWithTransaction()` -> `HTMLEditor::FormatBlockContainerAsSubAction()`
Differential Revision: https://phabricator.services.mozilla.com/D44179
--HG--
extra : moz-landing-system : lando
And this fixes the caller which has not guaranteed the lifetime of the
start container.
Differential Revision: https://phabricator.services.mozilla.com/D44175
--HG--
extra : moz-landing-system : lando
Since bug 1577443 is landed in comm-central, no one uses `nsIHTMLEditor.getLinkedObject`.
Differential Revision: https://phabricator.services.mozilla.com/D44361
--HG--
extra : moz-landing-system : lando
This patch fixes an existing bug with this clean up.
Except `HTMLEditRules::MoveBlock()`, `GetListActionNodes()` is called after
calling `SplitInlinesAndCollectEditTargetNodesInExtendedSelectionRanges()`
or `CollectEditTargetNodesInExtendedSelectionRanges()` **with**
`EditSubAction::eCreateOrChangeList`. I think that `HTMLEditRules::MoveBlock()`
using the edit sub-action is a simple mistake. Perhaps, it should be
`EditSubAction::eCreateOrRemvoeBlock`. However, I'm not 100% sure because
`HTMLEditor::CollectEditTargetNodes()` does special handling for
`EditSubAction::eCreateOrRemvoeBlock` but not so for
`EditSubAction::eCreateOrChangeList`. The behavior of difference between
those edit sub-actions are only here. Therefore, this patch creates new
`EditSubAction` `eMergeBlockContents` for `MoveBlock()`.
Then, this patch makes `HTMLEditor::CollectEditTargetNodes()` handle
`EditSubAction::eCreateOrChangeList` insead of `GetListActionNodes()`.
This causes one logic change in `SplitInlinesAndCollectEditTargetNodes()`.
It calls `MaybeSplitElementsAtEveryBRElement()` after `CollectEditTargetNodes()`
so that this change makes calling `MaybeSplitElementsAtEveryBRElement()` at
last. According to my tests, new behavior must be expected since `<br>`
elements outside and in `<table>` should be handled consistently. Therefore,
this patch adds some simple testcases into WPT.
Differential Revision: https://phabricator.services.mozilla.com/D43190
--HG--
extra : moz-landing-system : lando
`HTMLEditRules::GetListActionNodes()` removes non-editable element. However,
this should've been done by collector methods.
Differential Revision: https://phabricator.services.mozilla.com/D43026
--HG--
extra : moz-landing-system : lando
First half of `HTMLEditoRules::GetListActionNodes()` does 2 things. One is
trying to get parent list element of `Selection` ranges if `aEntireList` is
`EntireList::yes`. If it found a list element, it does nothing anymore.
Otherwise, falls backs to `EntireList::no` case. So, if each caller which
calls it with `EntireList::yes`, `GetListActionNodes()` does not need the
argument. Therefore, this patch does it first.
Then, `GetListActionNodes()` calls
`CollectEditTargetNodesInExtendedSelectionRanges()` or
`SplitInlinesAndCollectEditTargetNodesInExtendedSelectionRanges()`. It's
considered with `aTouchContent` is `yes` or `no`. So, this should be done
by each caller for making it clearer.
Differential Revision: https://phabricator.services.mozilla.com/D43024
--HG--
extra : moz-landing-system : lando
`HTMLEditRules::LookInsideDivBQandList()` does complicated things and I cannot
explain with a method name what it does. Fortunately, there are only 2 callers.
Therefore, this patch duplicates the part of modifying the range and creates
a method to retrieve "deepest and only editable child of `<div>`, `<blockquote>`
and one of list items".
Differential Revision: https://phabricator.services.mozilla.com/D43023
--HG--
extra : moz-landing-system : lando
It just collects all children of given node so that it can be a static method
in `HTMLEditor`.
Differential Revision: https://phabricator.services.mozilla.com/D43022
--HG--
extra : moz-landing-system : lando
It's called only from `HTMLEditRules::MoveBlock()`. Even though it has 4
patters to call different methods, we need only one of them. Therefore,
this patch moves it into `HTMLEditor.h` as
`SplitInlinesAndCollectEditTargetNodesInOneHardLine()`.
Differential Revision: https://phabricator.services.mozilla.com/D43021
--HG--
extra : moz-landing-system : lando
`HTMLEditRules::GetNodesFromSelection()` has a patch to call
`HTMLEditor::GetSelectionRangesExtendedToIncludeAdjuscentWhiteSpaces()`.
However, nobody uses this path so that we can get rid of this path.
Then, it becomes just calling `SplitInlinesAndCollectEditTargetNodes()` or
`CollectEditTargetNodes()` with result of
`GetSelectionRangesExtendedToHardLineStartAndEnd()`. Therefore, we can split
it to `SplitInlinesAndCollectEditTargetNodesInExtendedSelectionRanges()` and
`CollectEditTargetNodesInExtendedSelectionRanges()`. Then, we can mark
only the former as `MOZ_CAN_RUN_SCRIPT`.
Differential Revision: https://phabricator.services.mozilla.com/D43020
--HG--
extra : moz-landing-system : lando
`HTMLEditRules::GetPromotedRanges()` does 2 things. Calling
`CreateRangeIncludingAdjuscentWhiteSpaces()` or
`CreateRangeExtendedToHardLineStartAndEnd()` with each range of `Selection`.
The difference is considered with current edit sub-action. Therefore, we cal
split it to `GetSelectionRangesExtendedToIncludeAdjuscentWhiteSpaces()` and
`GetSelectionRangesExtendedToHardLineStartAndEnd()`. Then, we got clearer code
at each caller.
Differential Revision: https://phabricator.services.mozilla.com/D43018
--HG--
extra : moz-landing-system : lando
The method name is really unclear what's done. The method does 3 things.
One is try to select a `<br>` element in empty block if given range is
collapsed in the block. This is moved as
`HTMLEditor::SelectBRElementIfCollapsedInEmptyBlock()`. Next, it extends the
given range to include adjuscent whitespaces only when edit sub-action is
inserting or deleting text. This is moved as
`HTMLEditor::CreateRangeIncludingAdjuscentWhiteSpaces()`. Finally, when
handling the other edit sub-actions, extends the given range to start/end
of line at both edges. This is moved as
`HTMLEditor::CreateRangeExtendedToHardLineStartAndEnd()`.
And also this patch makes each caller of `PromoteRange()` to check edit
sub-action by themselves. Unfortunately, this duplicates same logic to
multiple places, but makes what they do clearer. I think that the duplication
issue should be fixed later if necessary. Instead, we should shine the
blackbox of `HTMLEditRules` right now.
Differential Revision: https://phabricator.services.mozilla.com/D43017
--HG--
extra : moz-landing-system : lando
`HTMLEditRules::GetPromotedPoint()` does too many things. Therefore, this patch
splits it to 3 methods. One is for specific `EditSubAction` values, that's
the first block in `GetPromotedPoint()`. It's named as
`GetWhiteSpaceEndPoint()`. Next one is for expanding start of the range to
start of its line. It's named as `GetCurrentHardLineStartPoint()`. The last
one is for expanding end of the range to end of its line. It's named as
`GetCurrentHardLineEndPoint()`.
Differential Revision: https://phabricator.services.mozilla.com/D42791
--HG--
extra : moz-landing-system : lando
This does not move the method simply. Instead, splits it to 4 simpler
methods.
Despite of the name, it modifies the DOM tree if `aTouchContent` is
`TouchContent::yes`. For avoiding this confusion and avoiding unnecessary
`MOZ_CAN_RUN_SCRIPT` attribute, the main part is split to
`HTMLEditor::CollectEditTargetNodes()`.
If callers want to call `HTMLEditRules::GetNodesForOperation()` with
`TouchContent::no`, only calling this method equals to the call of
`GetNodesForOperation()`.
Otherwise, the callers should call
`HTMLEditor::SplitInlinesAndCollectEditTargetNodes()`. This calls internal
methods automatically.
First, `HTMLEditor::SplitTextNodesAtRangeEnd()` splits text nodes at end of
each range. Then, `HTMLEditor::SplitParentInlineElementsAtRangeEdges()`
overload splits inline elements at both edges of each range. Then, collects
event target nodes with calling `HTMLEditor::CollectEditTargetNodes()`.
Finally, `HTMLEditor::MaybeSplitElementsAtEveryBRElement()` may split the
result nodes at every <br> element in them.
Differential Revision: https://phabricator.services.mozilla.com/D42790
--HG--
extra : moz-landing-system : lando
The fix here is to first check `NS_FAILED(rv)`, because if that's the
case, `cancel` wasn't necessarily set to a value.
As best practice I initialized `cancel` and `handled` with default
values.
Differential Revision: https://phabricator.services.mozilla.com/D43071
--HG--
extra : moz-landing-system : lando
This is regression by bug 1543312.
By bug 1543312, we destroy editor when destroying PresShell. But when destroying PresShell, editor doesn't remove anonymous content of editing UI from PresShell. Then, when destroying the frame manager in `PresShell::Destroy`, it hits assertion due to uncomposed doc.
We shouldn't hide editing UI during destroying PresShell and we should hide it after destroyed.
Differential Revision: https://phabricator.services.mozilla.com/D42773
--HG--
extra : moz-landing-system : lando
It's called immediately before setting `mHTMLEditor` and sets `mHTMLEditor` to
`nullptr`. So, it does nothing actually. We can get rid of it.
Differential Revision: https://phabricator.services.mozilla.com/D42771
--HG--
extra : moz-landing-system : lando
`HTMLEditRules::IsInlineNode()` is a wrapper of
`HTMLEditor::NodeIsInlineStatic()`, but returns opposite value.
This patch moves it into `HTMLEditor` and names it with same rule as
`NodeIsBlockStatic()`.
Note that this method may return true if given node is unexpected node type.
E.g., comment node, CDATA node, etc. However, it's not scope of this bug.
Differential Revision: https://phabricator.services.mozilla.com/D42770
--HG--
extra : moz-landing-system : lando
`HTMLEditRules::IsBlockNode()` just wraps `HTMLEditor::NodeIsBlockStatic()`
and all its users will be moved into `HTMLEditor`. Therefore, we should
unwrap it.
Differential Revision: https://phabricator.services.mozilla.com/D42769
--HG--
extra : moz-landing-system : lando
`HTMLEditRules::mReturnInEmptyLIKillsList` stores value of
`editor.html.typing.returnInEmptyListItemClosesList` at construction, and it's
set to true unless the pref value is `"false"`. However, this pref isn't
registered in anywhere (all.js, firefox.js, etc) nor used in comm-central
and BlueGriffon.
Even if I search the pref in the web, I don't see any information so that
this hidden pref must not be used anybody. Therefore, this patch just removes
this member.
Differential Revision: https://phabricator.services.mozilla.com/D42262
--HG--
extra : moz-landing-system : lando
This patch includes a logic change. `HTMLEditRules::mListenerEnabled` is used
for storing 2 state. One is that whether `HTMLEditRules` can handle the legacy
listener methods (they came from `nsIEditActionListener`) with `mHTMLEditor` or
not. The other is that whether current edit sub-action handler temporarily
disables the listeners for performance or not.
For the former, we can check same result with `HTMLEditRules::mInitialized`
and `HTMLEditRules::mHTMLEditor`.
However, the latter, current design is obviously wrong. Currently,
`HTMLEditRules::mListenerEnabled` is set only to `false` temporarily from
`WillInsertText()`, but the state will be affected to other edit sub-actions
which are nested by mutation event listeners. So, currently,
`HTMLEditRules::mDocChangeRange` may not contain modified range caused by
nested edit sub-actions.
For solving this bug, this patch moves it into `EditSubActionData` rather
than `HTMLEditor` and `TopLevelEditSubActionData`.
Differential Revision: https://phabricator.services.mozilla.com/D42107
--HG--
extra : moz-landing-system : lando
The members of `HTMLEditRules` which are used only while `WillDoAction()` and
`DidDoAction()` are called should be moved to specific stack only struct
`EditorBase::EditSubActionData`.
Differential Revision: https://phabricator.services.mozilla.com/D42106
--HG--
extra : moz-landing-system : lando
`HTMLEditRules::mUtilRange` is used only for the argument of
`HTMLEditRules::UpdateDocChangeRange()`. However, it does not need to be
a range instance since it compares its start and
`TopLevelEditSubAction::mChangedRange->StartRef()`, and its end and
`TopLevelEditSubAction::mChangedRange->EndRef()`. Therefore, with rewriting
it as taking 2 `EditorRawDOMPoint`s, we don't need `mUtilRange`.
Differential Revision: https://phabricator.services.mozilla.com/D42105
--HG--
extra : moz-landing-system : lando
This patch makes `StyleCache` not inherit `ItemProp` because `MOZ_COUNT_CTOR`
and `MOZ_COUNT_DTOR` do not work well with `AutoTArray` and there is no reason
to do that since nobody treat `StyleCache` instance with `ItemProp` pointer.
Differential Revision: https://phabricator.services.mozilla.com/D42103
--HG--
extra : moz-landing-system : lando
For avoiding allocation cost of `RangeItem`, it should be stored by `HTMLEditor`
and reused by all `TopLevelEditSubActionData` instances for the editor instance.
Differential Revision: https://phabricator.services.mozilla.com/D42102
--HG--
extra : moz-landing-system : lando
Most member variables of `HTMLEditRules` are temporary used while between
`BeforeEdit()` and `AfterEdit()` are called or `WillDoAction()` and
`DidDoAction()` are called.
The former means the data should be stored during top-level edit sub-action
is set. Therefore, this patch creates a struct, `TopLevelEditSubActionData`
and make it a member of `AutoEditActionDataSetter`. Then, makes
`EditorBase::TopLevelEditActionDataRef()` return reference of it.
And also this patch moves `HTMLEditRules::mDidDeleteSelection` into it.
Differential Revision: https://phabricator.services.mozilla.com/D42096
--HG--
extra : moz-landing-system : lando
This condition was needed when FindInsertionPrevSibling and co didn't understand
display: contents.
Editor is pretty broken (and calls into PresShell::ContentRemoved directly, and
incorrectly, using anonymous nodes).
In this case we were taking the XBL path because of display: contents, which
means that we tried to seek to the editor anonymous node, and crash (since it's
not an explicit kid).
Editor needs to get fixed, but this is technically more correct and fixes the
crash, so we may as well take it in the interim.
Differential Revision: https://phabricator.services.mozilla.com/D42472
--HG--
extra : moz-landing-system : lando
They're bad, specially if they do vastly different thing in overloaded
functions, like aUseSystemPrincipal and aIsPreload. :)
Differential Revision: https://phabricator.services.mozilla.com/D40851
--HG--
extra : moz-landing-system : lando
`TextEditRules::mDidExplicitlySetInterline` is set to true only by
`HTMLEditRules`, but `TextEditRules::DidDeleteSelection()` refers it.
So, it's enough to make `TextEditRules::DidDeleteSelection()` take the
value and we can move it into `HTMLEditRules`.
Differential Revision: https://phabricator.services.mozilla.com/D41401
--HG--
extra : moz-landing-system : lando
`TextEditRules::mDeleteBidiImmediately` is cache of
`bidi.edit.delete_immediately` pref value at creation time of `TextEditRules`.
However, this is referred when user removes selection. So, there is no
reason to keep same behavior starting from editor creation. In other words,
it must be better to take same behavior in all editor instances.
Therefore, we should remove it and the pref value should be referred directly
when user tries to remove selection.
Differential Revision: https://phabricator.services.mozilla.com/D41400
--HG--
extra : moz-landing-system : lando
`TextEditRules::BeforeEdit()`, `TextEditRules::AfterEdit()`,
`HTMLEditRules::BeforeEdit()` and `HTMLEditRules::AfterEdit()` are always
called with same values as the result of
`EditorBase::GetTopLevelEditSubAction()` and
`EditorBase::GetDirectionOfTopLevelEditSubAction()`.
For making what they do clearer, we should make them access with those
`EditorBase` members for now. This makes those methods ugly due to increasing
number of long lines. However, this issue should be solved when we move them
into `TextEditor` and `HTMLEditor`.
Differential Revision: https://phabricator.services.mozilla.com/D41399
--HG--
extra : moz-landing-system : lando
`TextEditRules::mLockRulesSniffing` is set by `AutoLockRulesSniffing`.
It's created during `BeforeEdit()` and `AfterEdit()` are called, and
`HTMLEditRules::Init()` is initializing `mDocChangeRange`.
The purpose of it is, preventing `BeforeEdit()` and `AfterEdit()` to do
something in that time. For the former cases, we don't need this member
anymore since they won't be nested. Therefore, we need to manage
`HTMLEditRules::BeforeEdit()` and `HTMLEditRules::AfterEdit()` won't do
anything only while `HTMLEditRules::Init()` is called. Therefore,
there should be only `HTMLEditRules::mInitialized` instead.
Differential Revision: https://phabricator.services.mozilla.com/D41398
--HG--
extra : moz-landing-system : lando
`TextEditRules::BeforeEdit()`, `TextEditRules::AfterEdit()`,
`HTMLEditRules::BeforeEdit()` and `HTMLEditRules::AfterEdit()` manages
`TextEditRules::mActionNesting` for preventing that they won't do same thing
per top-level edit sub-action. However, this has already been checked by
their caller, `AutoTopLevelEditSubActionNotifier`. So, we can get rid of it.
Then, `TextEditRules::mTopLevelEditSubAction` is also always same as
`EditorBase::GetTopLevelEditSubAction()`. Therefore, we can get rid of it
too.
Differential Revision: https://phabricator.services.mozilla.com/D41397
--HG--
extra : moz-landing-system : lando
Despite of their names, `TextEditRules::mCachedSelectionNode` and
`TextEditRules::mCachedSelectionOffset` are used only for calling
`EditorBase::HandleInlineSpellCheck()` so that they should be renamed to
explain the purpose.
Additionally, they are not necessary to be in the heap since they are
necessary until `TextEditRules::AfterEdit()` is called. Therefore, we can
move them into `EditorBase::HandleInlineSpellCheck()`.
Finally, `TextEditRules::BeforeEdit()` and `TextEditRules::AfterEdit()` are
called only by `TextEditor::OnStartToHandleTopLevelEditSubAction()` and
`TextEditor::OnEndHandlingTopLevelEditSubAction()`. Therefore, we can move
the setter to `TextEditor::OnStartToHandleTopLevelEditSubAction()`.
Differential Revision: https://phabricator.services.mozilla.com/D41396
--HG--
extra : moz-landing-system : lando
When browsing reported site (https://minecraft.curseforge.com/), user that uses Android cannot set caret on some editable contents.
GV's IME code uses `NOTIFY_IME_OF_SELECTION_CHANGE` to update selection on native IME. But GV doesn't receives this notification on this reported site. This notification is fired by editor's selection listener, but when this occurs, this notification is no logner fired unfortunately. Because selection listener isn't registered by current selection.
At first, editor registers selection listener by `EditorBase::Init`. But like test case, if PresShell is destroyed after editor is created, this registration is no longer used, and we have to register it by new selection again. So we should tear down editor when PresShell is destroyed. (or we have to add a way to re-initialize selection listener?)
This isn't related to Android. Android's IME code depends on this notification, so this occurs easily.
Also, `PresShell::SetCaretEnabled` only works when caret is valid (`PresShell::GetCaret` has caret). If caret is nothing, it hits assertion.
And, attached test case is reproduced sample.
Differential Revision: https://phabricator.services.mozilla.com/D41356
--HG--
extra : moz-landing-system : lando
`TextEditRules` and `HTMLEditRules` still refer
`EditorBase::mPaddingBRElementForEmptyEditor` directly but this is really ugly.
Therefore, this patch creates `EditorBase::HasPaddingBRElementForEmptyEditor()`
for wrapping its access.
Differential Revision: https://phabricator.services.mozilla.com/D41162
--HG--
extra : moz-landing-system : lando
`HTMLEditRules::OnModifyDocument()` is same as just calling
`EditorBase::EnsureNoPaddingBRElementForEmptyEditor()` and
`EditorBase::MaybeCreatePaddingBRElementForEmptyEditor()` so that this patch
gets rid of the method, then, creates `HTMLEditor::OnModifyDocumentInternal()`
and makes it do same thing.
Differential Revision: https://phabricator.services.mozilla.com/D41161
--HG--
extra : moz-landing-system : lando
`HTMLEditRules::WillLoadHTML()` does exactly same thing as
`EditorBase::EnsureNoPaddingBRElementForEmptyEditor()`. Therefore, we can
get rid of it and make `HTMLEditor::LoadHTML()` not use `HTMLEditRules`.
Differential Revision: https://phabricator.services.mozilla.com/D41160
--HG--
extra : moz-landing-system : lando
`TextEditRules::CreatePaddingBRElementForEmptyEditorIfNeeded()` is used by
both `TextEditor` and `HTMLEditor` so that this moves it into `EditorBase`.
Additionally, making `TextEditor::MaybeChangePaddingBRElementForEmptyEditor()`
call it if there is no content. Then, we can avoid the dependency of them.
Differential Revision: https://phabricator.services.mozilla.com/D41159
--HG--
extra : moz-landing-system : lando
`TextEditRules::RemoveRedundantTrailingBR()` is used only by multiline text
editor (i.e., `<textarea>`). Therefore, it should be moved into `TextEditor`.
Differential Revision: https://phabricator.services.mozilla.com/D41158
--HG--
extra : moz-landing-system : lando
`TextEditRules::WillUndo()` and `TextEditRules::WillRedo()` only check whether
the editor is readonly/disabled or not. So, `TextEditor::UndoAsAction()` and
`TextEditor::RedoAsAction()` should do it first.
`TextEditRules::DidUndo()` and `TextEditRules::DidRedo()` only set or unset
`mPaddingBRElementForEmptyEditor` if it's restored by undo or redo.
Therefore, we can move the code into `TextEditor::UndoAsAction()` and
`TextEditor::RedoAsAction()`.
Note that this patch makes `TextEditor::UndoAsAction()` discard the result of
`TransactionManager::Undo()` because this is inconsistent from what
`TextEditor::RedoAsAction()` does and this was changed by part 5 of bug 1447924.
https://hg.mozilla.org/mozilla-central/rev/869a1445816be7f43f54f7c97f28e4c6273fa75f
Differential Revision: https://phabricator.services.mozilla.com/D41157
--HG--
extra : moz-landing-system : lando
`TextEditRules::WillInsert()` is not used with initial purpose since
`HTMLEditor` always works with `HTMLEditRules` and its `WillDoAction()`
always handles `EditSubAction::eInsertElement`.
Additionally, its name is too generic since it does non-related 3 things.
One is checking whether the editor is readonly or disabled. However, this
may not be necessary since its callers may have already checked it or
just ignored the result. So, this should be check by each caller.
Next one is masking password if auto-masking is enabled. This is `TextEditor`
specific feature so that this patch moves the code into
`TextEditor::MaybeDoAutoPasswordMasking()`.
Final one is removing empty `<br>` element for empty editor if there is.
This is common feature so that this patch moves this code into
`EditorBase::EnsureNoPaddingBRElementForEmptyEditor()`.
Differential Revision: https://phabricator.services.mozilla.com/D41156
--HG--
extra : moz-landing-system : lando
`TextEditRules::mPaddingBRElementForEmptyEditor` are used by both `TextEditor`
and `HTMLEditor`. Therefore, it should be in `EditorBase`.
This patch makes `TextEditRules` and `HTMLEditRules` directly access the
private member of `EditorBase` temporarily. It'll be fixed by the following
patches.
Differential Revision: https://phabricator.services.mozilla.com/D41155
--HG--
extra : moz-landing-system : lando
This allows users of `WSRunScanner`'s functionality to pass a `const
HTMLEditor*`, allowing themselves to become const-correct.
Differential Revision: https://phabricator.services.mozilla.com/D41384
--HG--
extra : moz-landing-system : lando
We check surrogate pair at specific index in `nsTextFragement` in a lot of
places. This requires boundary check of the index so that it can cause
security issue and crash reason with simple mistake, and also it steals
your time to understand the code what it does especially when it's a
part of an `if` condition.
Therefore, this patch adds new API to `nsTextFragment` and makes the all
surrogate pair handlers of `nsTextFragument` use new API.
Differential Revision: https://phabricator.services.mozilla.com/D39689
--HG--
extra : moz-landing-system : lando
Original regression was by bug 1362858, and bug 1418629 wasn't enough to fix.
By bug 1362858, we use `CHAR_CLASS_SEPARATOR` in additional to DOM word separator. But some characters such as single quote, `@` and etc are `CHAR_CLASS_SEPARATOR`, so we may check spell by incomplete word.
We shouldn't separate word by characters that is email part, URL part or conditional punctuation.
And I also update test cases for this situation. `<textarea>` is better for spell checking since it can has multiple anonymous text nodes.
Differential Revision: https://phabricator.services.mozilla.com/D39829
--HG--
extra : moz-landing-system : lando
When using `<textarea>`, spell checker sometimes check spell when editing last text node. Because editor passes invalid previous selection node and offset via `HandleInlineSpellCheck`.
Although previous selected node uses `mCachedSelectionNode` that is stored by `BeforeEdit`, when this occurs, it isn't text node.
When editing last text node in `<textarea>`, anchror node might be root node, not current text node. So we should use text node instead when this is inserting text operation.
Differential Revision: https://phabricator.services.mozilla.com/D40368
--HG--
extra : moz-landing-system : lando
Stopping using attribute for "moz-br", `IMEContentObserver` cannot know when
new `<br>` element is changed to padding element for empty last line.
Therefore, editor needs to insert padding `<br>` element after setting the
flag properly. Then, `IMEContentObserver` does not need to recompute the
length of `<br>` element (if it's for padding, it computes the length as 0).
Unfortunately, `TextEditor::InsertBrElementWithTransaction()` is used in too
many places and it already has optional argument. Therefore, it's difficult
to change it. However, we should share the preparation before creating `<br>`
element in it with new method. Therefore, this patch creates
`EditorBase::PrepareToInsertBRElement()` to share the preparation point (almost
just moved from the method). Then, new method is created as
`EditorBase::InsertPaddingBRElementForEmptyLastLineWithTransaction()` because
it's used both in `TextEditor` and `HTMLEditor`. Finally, `TextEditor` won't
insert `<br>` element with `InsertBrElementWithTransaction()`. Therefore, it's
moved to `HTMLEditor` with renaming to `InsertBRElementWithTransaction()`.
Differential Revision: https://phabricator.services.mozilla.com/D39860
--HG--
extra : moz-landing-system : lando
`TextEditRules::CreateBR()` is used only by
`HTMLEditRules::InsertBRIfNeededInternal()` and it just calls
`TextEditor::InsertBrElementWithTransaction()`. Therefore, we can get rid of
it. Then, `CreateBRInternal()` can be renamed to
`CreatePaddingBRElementForEmptyLastLine()` since it's shared only by
`CreateBR()` and `CreatePaddingBRElementForEmptyLastLine()`.
Differential Revision: https://phabricator.services.mozilla.com/D39859
--HG--
extra : moz-landing-system : lando
Editor creates a `<br>` element to end of a block if last line
of the block is empty because caret should be placed as there is an empty
line. Such special `<br>` element has `type` attribute whose value is "_moz".
However, adding/removing the attribute is expensive and such hacky attribute
shouldn't be referred nor changed by web apps.
Therefore, this patch makes `HTMLBRElement` take another specific flag whether
it's a special node for empty last line. For making the meaning clearer,
this patch calls the such `<br>` elements as "padding `<br>` element for
empty last line" insead of "moz-br". So, this patch also includes a lot of
renaming methods and variables, and modifying related comments.
Note that with this change, `IMEContentObserver` counts the padding `<br>`
element in `<textarea>` because it's inserted before setting the new flag
and setting the flag does not cause DOM tree mutation. This issue will be
fixed by the following patches.
Differential Revision: https://phabricator.services.mozilla.com/D39858
--HG--
extra : moz-landing-system : lando
Editor creates a `<br>` element when it's root element is empty.
Then, it's stored by `TextEditRules::mBogusNode` and used for checking
whether the editor is empty quickly. However, this `<br>` element has
`mozeditorbogusnode` attribute whose value is `true`. However, adding or
removing the attribute is not cheap and web apps can refer such illegal
attribute.
Therefore, this patch makes `HTMLBRElement` take a specific flag whether
it's a bogus node or not. However, this means that this hacky thing will be
exposed outside editor module. For making what is the bogus node clearer,
this patch calls the such `<br>` elements as "padding `<br>` element for
empty editor". So, this patch also includes a lot of renaming methods and
variables, and modifying related comments.
Differential Revision: https://phabricator.services.mozilla.com/D39857
--HG--
extra : moz-landing-system : lando
Currently, if you need to unmask all password characters, you need to call
`nsIEditor.unmask(0)`. However, this is ugly and this must be the most use
case. So, I think that we should make it optional.
Differential Revision: https://phabricator.services.mozilla.com/D39327
--HG--
extra : moz-landing-system : lando
`aStart` of `TextEditor::SetUnmaskRangeInternal()` may be `UINT32_MAX` if
it's called for stopping unmasking. However, it returns `NS_ERROR_INVALID_ARG`
if there is no text node and `aStart` is not 0, but this is unexpected behavior.
Differential Revision: https://phabricator.services.mozilla.com/D39326
--HG--
extra : moz-landing-system : lando
First, we need to make `nsCopySupport::FireClipboardEvent()` keep handling
`eCopy` and `eCut` event even in password field, only if `TextEditor` allows
them.
Then, we need to make `nsPlainTextSerializer::AppendText()` not expose
masked password for making users safer. Although `TextEditor` does not allow
`eCopy` nor `eCut` when selection is not in unmasked range. Fortunately,
retrieving masked and unmasked password from `nsTextFragment` has already
been implemented in `ContentEventHandler.cpp`. This patch moves it into
`EditorUtils` and makes `ContentEventHandler.cpp` and `nsPlaintextSerializer`
share it.
Differential Revision: https://phabricator.services.mozilla.com/D39000
--HG--
extra : moz-landing-system : lando
It does not make sense to copy masked password with mask characters.
Therefore, we should allow users to copy/cut in password fields only when
selected range is in unmasked range.
Note that for web-compat, copy/cut are always enabled in HTML/XHTML document
in content. Therefore this patch changes the behavior only in chrome's
password fields.
Additionally, only the test uses `nsIEditor.canDelete()`. Therefore, this
removes it and make the test use `nsIDocShell.isCommandEnabled()` instead.
Unfortunately, `nsIEditor.canCopy()` and `nsIEditor.canCut()` are used by
BlueGriffon, therefore, we cannot get rid of them for now.
Differential Revision: https://phabricator.services.mozilla.com/D38999
--HG--
rename : editor/libeditor/tests/test_bug1067255.html => editor/libeditor/tests/test_cut_copy_delete_command_enabled.html
extra : moz-landing-system : lando
This requires replacing inclusions of it with inclusions of more specific prefs
files.
The exception is that StaticPrefsAll.h, which is equivalent to StaticPrefs.h,
and is used in `Codegen.py` because doing something smarter is tricky and
suitable for a follow-up. As a result, any change to StaticPrefList.yaml will
still trigger recompilation of all the generated DOM bindings files, but that's
still a big improvement over trigger recompilation of every file that uses
static prefs.
Most of the changes in this commit are very boring. The only changes that are
not boring are modules/libpref/*, Codegen.py, and ServoBindings.toml.
Differential Revision: https://phabricator.services.mozilla.com/D39138
--HG--
extra : moz-landing-system : lando
Unmasking is an optional style of showing password. Therefore, if callers of
`nsIEditor::Unmask()` specify middle of surrogate pair(s), it may mean that
they want to expand the unmask range from shorter range which does not include
the high and/or low surrogate. Therefore, one of the surrogates is in unmasked
range, we unmask the surrogate pair. However, we handle this in a lot of
places, i..e., we have duplicated code. This can get rid of these duplicates
with making `nsIEditor::Unmask()` expand the range automatically.
Differential Revision: https://phabricator.services.mozilla.com/D38432
--HG--
extra : moz-landing-system : lando
Double click, long tap, moving selection with keyboard and deleting text
scan word boundary. With these changes, the text node has raw password
value even if it's masked visually. For making safer,
`nsTextFrame::PeekOffsetWord()` should search word boundary in masked text.
Then, we can hide word boundary at masked range, but keep allowing to look
for word boundary only in unmasked range.
Differential Revision: https://phabricator.services.mozilla.com/D38013
--HG--
extra : moz-landing-system : lando
With the previous patches, editor has stopped masking characters in password
field. Instead, layout code needs to handle it. However, layout code
especially around `nsTextFrame` is performance critical area. Therefore,
layout code requires a quick way to check whether a text node in password
field or not.
This patch creates new flag for `CharacterData` node and marks all text nodes
whose characters should be masked with the flag when `EditorBase` or
`nsTextControlFrame` creates them.
Differential Revision: https://phabricator.services.mozilla.com/D38006
--HG--
extra : moz-landing-system : lando
Now, `TextEditRules` should use the new editor API to mask/unmask characters
if it's for a password editor. With this change, it does not need to manage
masked characters and unmasked characters separately since the anonymous
text node always have unmasked characters and `nsTextFrame` will mask the
characters at painting time.
Differential Revision: https://phabricator.services.mozilla.com/D38005
--HG--
extra : moz-landing-system : lando
This patch creates editor API to get/set unmask-range of password field.
Its design is similar to `setSelectionRange()` and `selectionStart`/
`selectionEnd` attributes. The unmasked range is automatically
masked if `aTimeout` of `unmask()` is set to non-zero.
Otherwise, unmasked range won't be masked automatically even after user
or web apps modifies the editor, and inserting new character expands
unmasking range.
The following patch makes `TextEditRules` use these API to implement
delayed masking of password.
Note that editor has never supported dynamic `eEditorPasswordMask` change.
E.g., if you have typed some characters into an editor and toggle the flag,
the characters are not unmasked nor masked. Then, if you type new characters,
only they are correctly masked or unmasked. This patch puts `MOZ_ASSERT()`
to reject this feature completely on debug build for making the unmasking
implementation simpler.
Differential Revision: https://phabricator.services.mozilla.com/D38004
--HG--
extra : moz-landing-system : lando
In the next patch, we need to update unmasked range when anonymous text node
in `<input type="password">` is modified. This patch makes editor manage it
easier to update the range.
Note that the next patch also handles text node creation and destruction.
Differential Revision: https://phabricator.services.mozilla.com/D38003
--HG--
extra : moz-landing-system : lando
Previously, `runRemoveContentTest()` was the last test which synthesizes
composition in the `<textarea>`. However, new test order move it to middle
of the other tests. Then, it hits an `MOZ_ASSERT()` in
`TextComposition::EditorWillHandleCompositionChangeEvent()` that detects a
bug. When editable element is removed from the DOM tree during a composition,
editor cannot listen to `eCompositionEnd` event. Therefore, when the editor
gets back, it still has destroyed `TextComposition` and keeps handling new
composition with the old one. Therefore this patch makes
`EditorBase::InstallEventListeners()` forget the destroyed composition.
Differential Revision: https://phabricator.services.mozilla.com/D37061
--HG--
extra : moz-landing-system : lando
To avoid populating the clipboard cache,
`nsContentUtils::IPCTransferableToTransferable` should set the
IsPrivateData flag on the output transferable BEFORE assigning data to
it, not therafter.
This patch includes a new regression test for this specific scenario.
The patch also includes fixes for incorrect IsPrivateData flags in some
other locations with `transferable->Init(nullptr)`, but without unit
tests.
Differential Revision: https://phabricator.services.mozilla.com/D36440
--HG--
extra : moz-landing-system : lando
Since JSWindowActors don't have direct access to synchronous messaging,
ChromeScript callers are going to need to migrate to asynchronous messaging
and queries instead.
Since there's no comparable API to sendQuery for frame message managers, this
patch adds a stub that uses synchronous messaging, but makes the API appear
asynchronous, and migrates callers to use it instead of direct synchronous
messaging. This will be replaced with a true synchronous API in the actor
migration.
Fortunately, most of the time, this actually leads to simpler code. The
`sendQuery` API doesn't have the odd return value semantics of
`sendSyncMessage`, and can usually just be used as a drop-in replacement. Many
of the `sendSyncMessage` callers don't actually use the result, and can just
be changed to `sendAsyncMessage`. And many of the existing async messaging
users can be changed to just use `sendQuery` rather than sending messages and
adding response listeners.
However, the APZ code is an exception. It relies on intricate properties of
the event loop, and doesn't have an easy way to slot in promise handlers, so I
migrated it to using sync messaging via process message managers instead.
Differential Revision: https://phabricator.services.mozilla.com/D35055
--HG--
extra : rebase_source : d5707e87f293a831a5cf2e0b0a7e977090267f78
extra : source : 75ebd6fce136ab3bd0e591c2b8b2d06d3b5bf923
The SpecialPowers set*Pref/get*Pref APIs currently use synchronous messaging
to set and get preference values from the parent process. Aside from directly
affecting callers of those APIs, it also affects callers of `pushPrefEnv`,
which is meant to be asynchronous, but is in practice usually synchronous due
to the synchronous messaging it uses.
This patch updates the getPref APIs to use the in-process preference service
(which most callers are expecting anyway), and also updates the callers of
the setPref and pushPrefEnv APIs to await the result if they're relying on it
taking effect immediately.
Unfortunately, there are some corner cases in tests that appear to only work
because of the quirks of the current sync messaging approach. The synchronous
setPref APIs, for instance, trigger preference changes in the parent
instantly, but don't update the values in the child until we've returned to
the event loop and had a chance to process the notifications from the parent.
The differnce in timing leads some tests to fail in strange ways, which this
patch works around by just adding timeouts.
There should be follow-ups for test owners to fix the flakiness.
Differential Revision: https://phabricator.services.mozilla.com/D35054
--HG--
extra : rebase_source : 941298157e7c82f420cf50ce057154ce9b85301c
extra : source : 189dc8a359815e059a4a217f788d183260bb2bfe
This patch makes `HTMLEditRules::PinSelectionToNewBlock()` use `StaticRange`
instead of `nsRange` for comparing a point and a range (i.e., the DOM tree
won't be changed during it's alive). Unfortunately, we still have allocation
cost, but we can save the cost of registering and unregistering mutation
observer and computing common ancestor of the range.
Differential Revision: https://phabricator.services.mozilla.com/D35145
--HG--
extra : moz-landing-system : lando
This patch makes `HTMLEditRules::ExpandSelectionForDeletion()` use `StaticRange`
instead of `nsRange` for comparing a point and a range (i.e., the DOM tree
won't be changed during it's alive). Unfortunately, we still have allocation
cost, but we can save the cost of registering and unregistering mutation
observer and computing common ancestor of the range.
Differential Revision: https://phabricator.services.mozilla.com/D35144
--HG--
extra : moz-landing-system : lando
Some `nsRange` static methods are useful in `StaticRange` and some of them
are used in a lot of places but not related to `nsRange` directly. This
patch moves them into new static method only class, `mozilla::RangeUtils`.
Differential Revision: https://phabricator.services.mozilla.com/D35142
--HG--
extra : moz-landing-system : lando
For avoiding confusion between API of `nsRange` and `StaticRange`, I'd like to
rename `nsRange::CreateRange()` to `nsRange::Create()` because
`StaticRange::CreateStaticRange()` is too long name and
`StaticRange::CreateRange()` sounds odd. This patch renames it and changes
related methods to template methods to avoid runtime cost of temporary
`RawRangeBoundary` instance creation.
Differential Revision: https://phabricator.services.mozilla.com/D35141
--HG--
extra : moz-landing-system : lando
For content HTML/XHTML copy/paste should always be enabled, but for chrome
docs we can support enabling/disabling copy/paste.
Also, restores tests to how they were before copy/paste was always enabled.
Differential Revision: https://phabricator.services.mozilla.com/D34805
--HG--
extra : moz-landing-system : lando
For content HTML/XHTML copy/paste should always be enabled, but for chrome
docs we can support enabling/disabling copy/paste.
Also, restores tests to how they were before copy/paste was always enabled.
Differential Revision: https://phabricator.services.mozilla.com/D34805
--HG--
extra : moz-landing-system : lando
For content HTML/XHTML copy/paste should always be enabled, but for chrome
docs we can support enabling/disabling copy/paste.
Also, restores tests to how they were before copy/paste was always enabled.
Differential Revision: https://phabricator.services.mozilla.com/D34805
--HG--
extra : moz-landing-system : lando
Masayuki Nakano
Makoto Kato
Cosmin Sabou
Mirko Brodesser
Noemi Erli
Mark Banner
Emilio Cobos Álvarez
Kannan Vijayan
Nicholas Nethercote
Dorel Luca
Victor Porof
Razvan Maries
Rob Wu
Kris Maglione
Brendan Dahl
Ciure Andrei
Narcis Beleuzu
Jonathan Kingston
Boris Zbarsky