When selection is `abc<b>[def</b>]ghi`, `insertParagraph` command will delete
the `<b>` element first, then, `Selection` becomes `abc{}ghi`. Then,
`HTMLEditor::InsertParagraphSeparatorAsSubAction` wraps all of the line in
the default paragraph, `<div>`, with
`HTMLEditor::FormatBlockContainerWithTransaction` (although this is incompatible
behavior with the other browsers). At this time, new `<div>` is inserted before
the first text node and then, move the text nodes into the new `<div>`.
However, `RangeUpdater::DidMoveNode` just slides the offsets if containers of
registered DOM points are the ex-parent of the moving nodes. Therefore, the
tracked selection range in `HTMLEditor::FormatBlockContainerWithTransaction`
become `<div></div>abc{}def`, then, `<div>abcdef</div>{}`, but the expected
behavior is of course, `<div>abc{}def</div>`, then, split the new `<div>`.
So the problem is, `DidMoveNode` assumes that DOM points won't point the moving
content node. If the node is pointed, it should keep pointing in the new
parent.
Note that the expectations of new tests are based on Chrome, therefore, the
new known failures are incompatible with Chrome.
Differential Revision: https://phabricator.services.mozilla.com/D156798
It's hard to know how some tests failed in test_bug1318312.html. Therefore,
this patch makes the selection range comparison easier to read, and add
description to explain what was doing at the failure.
Differential Revision: https://phabricator.services.mozilla.com/D156526
This patch fixed a regression caused by Bug 1730084 Part 4
https://phabricator.services.mozilla.com/D125434, which is intended to preserve
the behavior of `WordBreaker::FindWord()`.
Before Bug 1730084 Part 4, `WordBreaker::FindWord()` returns
`[aTextLen + 1, aTextLen + 1]` only when `aOffset > aTextLen`.
But `WordBreaker::FindWord()` had an assertion `aOffset <= aTextLen`
to guarantee the caller never passes `aOffset > aTextLen`. Thus, we can never
go into the `if (res.mBegin > strLen)` in
`TextServicesDocument::OffsetEntryArray::FindWordRange()`.
However, Bug 1730084 Part 4 wrongly changes the if-statement to
`if (range.mEnd == range.mBegin)`, and makes it reachable when
`TextServicesDocument::OffsetEntryArray::FindWordRange()` passes
`aPos == aLen` into `WordBreaker::FindWord()`. This makes `InitSpellChecker()`
failed when `enableSelectionChecking=true`.
This patch deletes the originally unreachable error handling if-statement, and
adds an assertion to guarantee `strOffset` and `strLen` is valid.
Differential Revision: https://phabricator.services.mozilla.com/D156210
Additionally,
* `PropItem` -> `PendingStyle`
* `StyleCache` -> `PendingStyleCache`
* `AutoStyleCacheArray` -> `AutoPendingStyleCacheArray`
And finally, `PendingStyle` (formally `PropItem`) is changed to `class` and
its members are encapsuled.
Differential Revision: https://phabricator.services.mozilla.com/D155318
For consistency with the previous patch, we should rename them too. Then, we're
getting rid of unclear word "Prop" from the public methods.
Differential Revision: https://phabricator.services.mozilla.com/D155316
I usually retry to understand what they mean. Therefore, I'd like to give new
names for them (and rename `TypeInState` class in a following patch).
Differential Revision: https://phabricator.services.mozilla.com/D155315
Only the value member needs to be updated when setting the prop multiple times.
Therefore, we cannot change all members to constants.
Differential Revision: https://phabricator.services.mozilla.com/D155313
According to the debug, its value can be CSS property value if in the CSS mode.
For making the value meaning easier to understand, this renames it to
mAttributeValueOrCSSValue.
Differential Revision: https://phabricator.services.mozilla.com/D155312
It's currently no problem to manage it with a raw pointer because it may be
set to a dynamic atom only when `nsIHTMLEditor.insertLinkAroundSelection` is
called with unknown attribute, but comm-central uses it only with `href`
attribute.
I think that we should change the API just to take `href` value in the future,
but for now, it should be `RefPtr<nsAtom>`.
Differential Revision: https://phabricator.services.mozilla.com/D155311
It's always a pointer to `nsStaticAtom` instance or `nullptr`. Therefore,
it can be `nsStaticAtom*` and we can make its users treat `nsStaticAtom`
instead of `nsAtom`.
Additionally, this patch changes some pointer parameters to references if
they are never `nullptr`.
Differential Revision: https://phabricator.services.mozilla.com/D155310
In bug 1739524, I misunderstood the meaning of `aOffset` of `SelAdjJoinNodes`.
After joining 2 nodes, and a point points right node which will have ex-left
node content, the point needs to point ex-start of the right node to keep
next insertion point as-is. Therefore, it's not useful with new join nodes
direction, it needs to know the ex-offset of the right node.
Differential Revision: https://phabricator.services.mozilla.com/D155438
Before the fix of bug 1758420, `TextComposition`'s text node and offset in it
are updated at creating `CompositionTransaction`. However, it's now put off
until getting the mutations. Therefore, it always unsets `pointToInsert` at
first time of the composition and fails to notify the listeners.
This patch makes it retrieve the result after calling `DoTransactionInternal`
and handle the post processing with the new point.
Differential Revision: https://phabricator.services.mozilla.com/D155052
This patch doesn't impact behavior.
The pref "gfx.font_loader.interval" used to control certain aspects of
font-loading behavior, but that code has evolved and we no longer read the
value of this pref anywhere.
Differential Revision: https://phabricator.services.mozilla.com/D155183
It's a Gecko specific feature, and it sets background color of parent block
elements of selection ranges. This does similar things to
`SetInlinePropertiesAsSubAction`, but still refers `Selection` directly and
uses `AutoSelectionRestorer`. For consistency between similar methods, this
patch makes it use `AutoRangeArray`.
Depends on D154353
Differential Revision: https://phabricator.services.mozilla.com/D154354
`HTMLEditor::RelativeFontChange()` and its helpers are based on
`SetInlinePropertiesAsSubAction()` and its helpers. Therefore, they may have
similar problem to switch join/split direction. Therefore, this and the
following patches clean them up too.
Depends on D154350
Differential Revision: https://phabricator.services.mozilla.com/D154351
I think that `HTMLEditor::SetInlinePropertyAsSubAction` and
`HTMLEditor::RemoveInlinePropertyAsSubAction` should not be called multiple
times in one edit action. Therefore, I'd like to make them take multiple
styles once. Then, we could collect all targets before touching the DOM tree
in the future.
Depends on D154348
Differential Revision: https://phabricator.services.mozilla.com/D154349
This is a pre-fix for bug 1735608. Currently, the loop assumes that collected
nodes are always right node, but it'll be changed, therefore, it creates
new array for the follow up loops handle expected nodes.
Depends on D154346
Differential Revision: https://phabricator.services.mozilla.com/D154347
I'd like to get rid of `AutoSelectionRangeArray` and `AutoRestoreSelection`,
and I'd like to make it stop touching `Selection` while it is removing style
of each content nodes in the range. Therefore, this patch rewrites it with
`AutoRangeArray`. Then, we can reduce the indent level of the nested `for`
loops.
Note that it creates `AutoEditSubActionNotifier`, so it's a sub-edit action
handler. Therefore, this patch renames it.
Depends on D154345
Differential Revision: https://phabricator.services.mozilla.com/D154346
I've already made a caller of `HTMLEditor::ClearStyleAt`,
`HTMLEditor::CreateStyleForInsertText`, in bug 1770877, so this fixes a bug of
the patch.
`HTMLEditor::ClearStyleAt` is still updates `Selection` only in some cases.
And one of the caller has not handle the `Selection` update. Therefore, once
it completely stop touching `Selection`, `ComputeEditingHost` will fail and
the other paths update `Selection`, so it should do it too. (Without the
change, `test_dragdrop.html` fails.)
Finally, we don't need `EditResult` anymore because we have
`Result<EditorDOMPoint, nsresult>`.
Differential Revision: https://phabricator.services.mozilla.com/D154343
Similar to the previous patch, and for consistency between editor helper
classes, we should make result of getter methods of `EditorDOMPointBase` too.
Differential Revision: https://phabricator.services.mozilla.com/D153841
We'll need the version to return `dom::Text*` in coming patches for bug 1735608.
For avoiding the class to have a lot of getters, we should make result of the
getters templated.
Differential Revision: https://phabricator.services.mozilla.com/D153840
`GetLeftContent()` etc is explained as:
> This may return nullptr if the method didn't split at start edge of the node.
On the other hand, `SplitNodeResult::GetPreviousContent()` and
`SplitNodeResult::GetNextContent()` returns previous or next content node
of split point when the split is not performed, but not in error.
Therefore, it should check `SplitNodeResult::DidSplit()` instead of
`SplitNodeResult::isOk()`.
However, there is another problem. The constructor cannot specify the order
of the 2 splits. Therefore, it's hard to maintain the constructor.
Fortunately, there is only one user, so we can make the user create the
result with another constructor.
Differential Revision: https://phabricator.services.mozilla.com/D153838
If splitting node creates new right node instead of new left node,
`TopLevelEditSubActionData::mChangedRange` ends by start of the right node.
However, `RemoveEmptyNodesIn` uses `PostContentIterator` which collects DOM
nodes whose "end tag" appear in the range. Therefore, for cleaning up new
empty right nodes correctly, we need to extend the range to contain the node.
Additionally, it removes unexpected element which is editable but shouldn't be
removed from the DOM tree. Therefore, this patch adds new check such as
`HTMLEditUtils::IsRemovalNode()`.
Finally, this patch adds a check whether the editor is a mail editor or not
at considering whether a node is a "mail-cite" because `contenteditable` in
web apps do not need special handling for such Gecko-specific element.
Differential Revision: https://phabricator.services.mozilla.com/D153834
The test oddly passes without the fix in `HandleInsertBRElement`. I guess that
post-processing in `HTMLEditor::OnEndHandlingTopLevelEditSubActionInternal()`
handles `Selection`, and anyway the insertion point of the following
`insertText` is wrong. It seems that we don't need to uplift this.
Differential Revision: https://phabricator.services.mozilla.com/D153832
`increaseFontSize`, `decreaseFontSize`, `gethtml`, `heading` and `readonly`
commands were disabled for a year in all channels, but no regression reports
have been filed. Therefore, we can delete the commands and the telemetry
probes.
Note that `cmd_getContents` command which is the internal command of `gethtml`
is not used in comm-central too. Therefore, this patch deletes the command
handler, `nsClipboardGetContentsCommand`, and `Command::GetHTML` too.
Differential Revision: https://phabricator.services.mozilla.com/D153720
For doing that, this patch also making `HTMLEditor::AlignContentsAtSelection`
which the only caller of `HTMLEditor::AlignNodesAndDescendants` handle
`Selection` with `AutoRangeArray` because it uses `AutoSelectionRestorer` and
we need to adjust `Selection` after restored.
Differential Revision: https://phabricator.services.mozilla.com/D152980
It's only caller is `HTMLEditor::AlignContentsAtSelection` which is called only
by `HTMLEditor::AlignAsSubAction` at almost last of it. Therefore, it's safe
to handle `mNewBlockElement` at the caller of
`AlignContentsAtSelectionWithEmptyDivElement` is safe.
Differential Revision: https://phabricator.services.mozilla.com/D152978
The change of expected assertion count in the crash test is caused by that the
test calls `execCommand` recursively from the legacy mutation event listeners.
Therefore, stopping updating `Selection` for each DOM tree change causes that
the target range in the nested edit action is changed. However, the nested
handling has already been disabled by default, so this should not be a problem
for the users.
Differential Revision: https://phabricator.services.mozilla.com/D152973
