The method is enough simple, and uses bad cast from point of view of OOP.
Therefore, this patch make the sub classes implement the method only for each.
Differential Revision: https://phabricator.services.mozilla.com/D157406
It does different thing for `TextEditor` and `HTMLEditor`, and used almost
internally. Therefore, it should be implemented in the sub classes and
we should name them better.
Differential Revision: https://phabricator.services.mozilla.com/D157407
The method is enough simple, and uses bad cast from point of view of OOP.
Therefore, this patch make the sub classes implement the method only for each.
Differential Revision: https://phabricator.services.mozilla.com/D157406
Unfortunately, the result is really different from the other browsers.
E.g., when `execCommand("indent")` at `<div>abc<br>{}<br></div>`, Gecko inserts
a `<div>` and set its `margin-left`. However, Chrome splits parent `<div>`
element and wrap the `<div>` which contains the caret in new `<blockquote>`.
Therefore, this does not contain new WPT.
Depends on D157404
Differential Revision: https://phabricator.services.mozilla.com/D157405
These macros will produce better outputs when they fail than these existing
patterns using `ENSURE_TRUE(NS_SUCCEEDED(...))` or similar, so this is a bulk
rewrite of existing tests to use them.
It should also help with discoverability when people base their tests off of
other existing tests.
Differential Revision: https://phabricator.services.mozilla.com/D157214
After this change, it's possible to set the pref, or use editor.document.execCommand("styleWithCSS", false, "true") to enable css in mail compose.
Differential Revision: https://phabricator.services.mozilla.com/D157276
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
It's called only by `HTMLEditor::HandleCSSIndentAtSelection` which is called
only by `HTMLEditor::HandleIndentAtSelection`. They don't touch `Selection`
after calling it. Therefore, we can make it adjust collapsing selection point
by itself.
Differential Revision: https://phabricator.services.mozilla.com/D152972
With taking an `AutoRangeArray` which is initialized with `Selection`, it can
restore selection and check if it's collapsed and if collapsed in the expected
list element. Then, its caller can apply the returned range to `Selection`
because it's an edit sub-action handler.
Differential Revision: https://phabricator.services.mozilla.com/D152969
It oddly retrieve an ancestor list element which contains one range in the
selection ranges. So, working it with `AutoRangeArray` which is initialized
with `Selection` makes `HTMLEditor` smaller...
Differential Revision: https://phabricator.services.mozilla.com/D152966
They set `TopLevelEditSubActionData::mNewBlockElement` and their root callers
in edit sub-action level are only `InsertParagraphSeparatorAsSubAction` and
`FormatBlockContainerAsSubAction`, and they are called each other. Therefore,
this patch changes these 3 methods once.
Differential Revision: https://phabricator.services.mozilla.com/D152964
The following patches touch the logic to restore selection after handling
commands. However, it seems that they are not tested because I found some
regressions by manual testing, but didn't cause orange on tryserver.
`outdent-preserving-selection.tentative.html` causes a crash due to the
`MOZ_ASSERTION` in `HTMLEditor::HandleOutdentAtSelectionInternal`. It means
that I misunderstand the logic and had put the assertion. I should fix it
later with reading the complicated code again. For now, I just change it
to `NS_WARNING_ASSERTION` instead.
And also `test_cmd_absPos.html` is the first test to check toggling `position`
between `static` and `absolute`. Therefore, it detects wrong `MOZ_ASSERT`s
which test whether `EditActionData` or `TopLevelEditSubActionData` is created
or not **before** they create them by themselves. So, this patch removes the
wrong assertions.
Differential Revision: https://phabricator.services.mozilla.com/D152962
The biggest set of APIs from ns[T]StringObsolete which are still heavily used
are the string searching APIs. It appears the intention was for these to be
replaced by the `FindInReadable` APIs, however that doesn't appear to have
happened.
In addition, the APIs have some quirks around their handling of mixed character
widths. These APIs generally supported both narrow strings and the native
string type, probably because char16_t string literals weren't available until
c++11. Finally they also used easy-to-confuse unlabeled boolean and integer
optional arguments to control behaviour.
These patches do the following major changes to the searching APIs:
1. The ASCII case-insensitive search method was split out as
LowerCaseFindASCII, rather than using a boolean. This should be less
error-prone and more explicit, and allows the method to continue to use
narrow string literals for all string types (as only ASCII is supported).
2. The other [R]Find methods were restricted to only support arguments with
matching character types. I considered adding a FindASCII method which would
use narrow string literals for both wide and narrow strings but it would've
been the same amount of work as changing all of the literals to unicode
literals.
This ends up being the bulk of the changes in the patch.
3. All find methods were re-implemented using std::basic_string_view's find
algorithm or stl algorithms to reduce code complexity, and avoid the need to
carry around the logic from nsStringObsolete.cpp.
4. The implementations were moved to nsTStringRepr.cpp.
5. An overload of Find was added to try to catch callers which previously
called `Find(..., false)` or `Find(..., true)` to set case-sensitivity, due
to booleans normally implicitly coercing to `index_type`. This should
probably be removed at some point, but may be useful during the transition.
Differential Revision: https://phabricator.services.mozilla.com/D148300
Currently, `HTMLEditor` assumes that padding `<br>` element for empty last line
is outside of inline elements, but it may happen because of both:
* starting from bug 1778091, `HTMLEditor` move `<br>` element into new empty
inline elements at inserting new paragraph.
* web apps can put it into inline elements.
After splitting inline elements and which do not have meaningful content, users
cannot put caret into the empty inline elements so that the elements stay unless
delete around there.
For avoiding the leak due to meaningless elements, we should delete them at
splitting inline elements at inserting new text.
Note that Chrome does not pass the new tests of resetting ancestor bold style
because Chrome wraps the `<b>` with `<i>`, however, the `<i>` has odd
`style=""`. Perhaps, the test framework should ignore it because it's not
important for the web-compatibility.
On the other hand, Chrome completely fails only the last testcase since it
unwraps the `<b>` from the last `<br>`, so the bold style which was applied by
the web app to the last `<br>` is lost. This is not reasonable.
Differential Revision: https://phabricator.services.mozilla.com/D152616
In Yahoo! Mail, the paragraph has `<br>` after `<span>` element which has
`background-color`. In this case, Gecko creates the following DOM tree after
splitting the paragraph:
```
<div><span>foo</span></div><div><span></span><br></div>
```
Then, the empty `<span>` in the right paragraph will be removed by the
post-processing. However, in this case, the inline element is required for
preserving the style continued from the previous paragraph.
In this case, we should move the `<br>` element into the `<span>` to make
it non-empty and avoid it to be removed. This is compatible with Chrome.
Differential Revision: https://phabricator.services.mozilla.com/D151345
Currently I'm think that the split/join node direction should be managed per
`HTMLEditor` instance because per transaction makes undo/redo handling
complicated and may require additional space to save it in each transaction
instance. However, the direction should be changeable with new Gecko specific
`execCommand` to support both direction until creating first split/join node
transaction or anytime with clearing the transactions.
Differential Revision: https://phabricator.services.mozilla.com/D149192
This adds a check to see if the encoded word is not empty and does not start
with the null character. Hunspell accepts C-style strings and marks the empty
string as correctly spelled. This prevents other dictionaries from detecting
misspelled strings in languages that use a different charset, which has lead
to problems when using the en-US dictionary packaged with Firefox and Greek,
Hebrew and Russian dictionaries, where misspellings are not detected in the
non-English language.
Differential Revision: https://phabricator.services.mozilla.com/D149899
This removes HTMLMenuItemElement and all the code and tests preffed off
by dom.menuitem.enabled.
The HTML parser changes are the result of applying the previous patch.
Differential Revision: https://phabricator.services.mozilla.com/D149979
Note that the odd path which always returns `NS_ERROR_FAILURE` is not covered by
the tests. Therefore, this patch adds `MOZ_ASSERT` to make somebody hit it and
report a bug.
Differential Revision: https://phabricator.services.mozilla.com/D149108
Note that `CSSEditUtils` does not change `Selection` except
`RemoveCSSInlineStyleWithTransaction` which is used only by aligning in a block.
Therefore, this patch does not touch `CSSEditUtils`.
Differential Revision: https://phabricator.services.mozilla.com/D149106
The preceding call of `InsertBRElement` may collapse selection at the inserted
padding `<br>` element. Only when calling `HandleInsertParagraphInParagraph`,
`InsertParagraphSeparatorAsSubAction` uses `Selection`. So, only in this case,
we need to recompute the point to split for keeping current (odd) behavior.
Note that in normal cases, using `atStartOfSelection` gets same result.
However, if there are some invisible nodes such as comment nodes, doing it
may change the behavior. For now, we should keep the current behavior. It
should be sorted out when we make it stop inserting `<br>` elements for the
preparation of split without checking whether it's actually necessary.
Differential Revision: https://phabricator.services.mozilla.com/D149101
It touches `Selection` redundantly (i.e., immediately after that, it or its
caller collapse `Selection`). So we can just stop it because we can ignore
the cases when the handling fails after the redundant `Selection` update and
it can be caused by tricky things with the mutation event listeners.
Note that without changing `SplitNodeResult` a lot, we cannot make it return
`SplitNodeResult`, and currently there is no motivation to do it because the
only caller does not need the detail of the split. Therefore, I give up doing
it.
Differential Revision: https://phabricator.services.mozilla.com/D149099
I tried to make the latter half preparation to call
`HTMLEditor::SplitParagraphWithTransaction`, but I cannot make it cleaner
because it needs to return `HTMLBRElement*` and `EditorDOMPoint` with maybe
modifying the DOM tree. So, I keep it as-is, but I make replace the unnecessary
duplicated `EditorDOMPoint` with the original one which is adjusted to avoid
to create new empty link in the right paragraph.
Differential Revision: https://phabricator.services.mozilla.com/D149096
Using the wide scope `EditorDOMPoint pointToInsertBR` makes it harder to read.
Although this duplicates a call of `HTMLEditor::InsertBRElement` and a couple of
post processing, but I think that it's easier to read/understand.
Differential Revision: https://phabricator.services.mozilla.com/D149094
If it needs to insert a `<br>` but the editor does not want to create new
paragraph, its caller should be default to insert a `<br>` element. Rather
than checking this at inserting `<br>` element, it should do it at considering
the place to insert the new `<br>`.
Differential Revision: https://phabricator.services.mozilla.com/D149093
This patch makes it take `AutoRangeArray` and it can save a snapshot of the
ranges and restore it like `AutoSelectionRestorer`.
For accessing `HTMLEditor::RangeUpdaterRef` for clearing the saved ranges,
`AutoRangeArray` needs to store `HTMLEditor`. Therefore, its constructors
and the destructor cannot be inlined in the header due to the include-hell.
Differential Revision: https://phabricator.services.mozilla.com/D149089
They at most twice updates the `Selection` right now. We can make them update
`Selection` once. And this patch makes the scope of `AutoRangeArray` clearer.
Differential Revision: https://phabricator.services.mozilla.com/D149087
Although they update the DOM tree with transactions, but for making `HTMLEditor`
smaller, and the following patches makes the related code simpler, we need to
move them into `AutoRangeArray`.
Differential Revision: https://phabricator.services.mozilla.com/D149084
It's always work with `Selection` ranges. Then, it should be treated within
`AutoRangeArray` which can be initialized with `Selection` ranges automatically.
Then, we can making `HTMLEditor` simpler.
Note tha this tries to make it get editing host when the corresponding
public method is called (after dispatched `beforeinput` event if handling a
users' operation). However, as commented in
`HTMLEditor::SetParagraphFormatAsAction`, it breaks a tricky crash test.
Therefore, only the format block path computes it after the preparation.
Differential Revision: https://phabricator.services.mozilla.com/D149079
For making extending the range to wrap the lines containing start and end
boundaries of each range of `AutoRangeArray::mRanges`, we need a method which
updates existing `nsRange` instances rather than creating new one.
Differential Revision: https://phabricator.services.mozilla.com/D149078
`AutoRangeArran` uses `OwningNonNull`. So making the following patch simpler
and smaller, all places should use array of `OwningNonNull<nsRange>` instead of
`RefPtr<nsRange>`.
Differential Revision: https://phabricator.services.mozilla.com/D149076
Similar to the previous patch, you must feel odd to move the methods. They
will be static methods in the `EditorUtils.cpp` file later.
Differential Revision: https://phabricator.services.mozilla.com/D149075
This must make you feel odd. However, this is a separate patch for making easier
to review the following patches. This will be a private member of
`AutoRangeArray`.
Differential Revision: https://phabricator.services.mozilla.com/D149074
Similar to the previous patch, but there are more callers than the previous one,
we should unwrap it temporarily for the following patches.
Differential Revision: https://phabricator.services.mozilla.com/D149071
It's used only by `HTMLEditor::MoveOneHardLineContentsWithTransaction` and
it just calls 2 methods with some simple additional error handlings.
I'd like to make `SplitInlinesAndCollectEditTargetNodes` work with
`AutoRangeArray`. Therefore, I'd like to do this for avoiding to use
`AutoRangeArray` in `HTMLEditor.h`.
Differential Revision: https://phabricator.services.mozilla.com/D149070
The constructor of `SplitRangeOffFromNodeResult` is used only by the method.
Therefore, we can customize the constructor and make it store the caret
suggestion.
Differential Revision: https://phabricator.services.mozilla.com/D149066
Although the its only caller does not need the caret position, it should return
a candidate caret position for aligning to the other methods.
Differential Revision: https://phabricator.services.mozilla.com/D149065
The biggest set of APIs from ns[T]StringObsolete which are still heavily used
are the string searching APIs. It appears the intention was for these to be
replaced by the `FindInReadable` APIs, however that doesn't appear to have
happened.
In addition, the APIs have some quirks around their handling of mixed character
widths. These APIs generally supported both narrow strings and the native
string type, probably because char16_t string literals weren't available until
c++11. Finally they also used easy-to-confuse unlabeled boolean and integer
optional arguments to control behaviour.
These patches do the following major changes to the searching APIs:
1. The ASCII case-insensitive search method was split out as
LowerCaseFindASCII, rather than using a boolean. This should be less
error-prone and more explicit, and allows the method to continue to use
narrow string literals for all string types (as only ASCII is supported).
2. The other [R]Find methods were restricted to only support arguments with
matching character types. I considered adding a FindASCII method which would
use narrow string literals for both wide and narrow strings but it would've
been the same amount of work as changing all of the literals to unicode
literals.
This ends up being the bulk of the changes in the patch.
3. All find methods were re-implemented using std::basic_string_view's find
algorithm or stl algorithms to reduce code complexity, and avoid the need to
carry around the logic from nsStringObsolete.cpp.
4. The implementations were moved to nsTStringRepr.cpp.
5. An overload of Find was added to try to catch callers which previously
called `Find(..., false)` or `Find(..., true)` to set case-sensitivity, due
to booleans normally implicitly coercing to `index_type`. This should
probably be removed at some point, but may be useful during the transition.
Differential Revision: https://phabricator.services.mozilla.com/D148300
Add a dom/base/rust crate called just "dom" where we can share these.
Most of the changes are automatic:
s/mozilla::EventStates/mozilla::dom::ElementState/
s/EventStates/ElementState/
s/NS_EVENT_STATE_/ElementState::/
s/NS_DOCUMENT_STATE_/DocumentState::/
And so on. This requires a new cbindgen version to avoid ugly casts for
large shifts.
Differential Revision: https://phabricator.services.mozilla.com/D148537
Its callers check `EditorBase::Destroyed()` with at least adding 4 lines, and
some callers do not check this important state. So, we should make it check
`Destroyed()` at last and omit the additional error check in the caller sites.
Note that it's a virtual method, but `HTMLEditor` checks whether it's a
removable node or not. So, we should can merge it into `EditorBase`. This
patch does it too.
Differential Revision: https://phabricator.services.mozilla.com/D148084
In the testcase, editing host is removed when first node removing, but
`HTMLEditor::Destroyed()` still returns `false`. So, ideally, we should add
a check of editing host validity in `Destroyed()`, but for now, we should make
the method check whether the handling range is still editable after running
each transaction.
Depends on D147723
Differential Revision: https://phabricator.services.mozilla.com/D147724
Currently, `TextEditor::OnFocus` does the things which were in
`EditorEventListener::OnFocus`, and they are not necessary at re-initializing
`Selection` in the anonymous subtree. Therefore, we should make it call
`EditorBase::OnFocus` directly.
Depends on D147623
Differential Revision: https://phabricator.services.mozilla.com/D147723
The reported testcase causes an incompatible behavior with the other browsers,
that is, `selectAll` command without focus in document whose body ends with
editable nodes selects the nodes in the selection root for the focused node (end
of the range in the normal direction). Therefore, `HTMLEditor` handles deletion
in odd state (without focus handling). So the root cause is a bug in selection,
but this bug as of the editor module should avoid the try of joining nodes which
are moved by a DOM mutation event listener at the initial deletion.
Differential Revision: https://phabricator.services.mozilla.com/D147623
It's currently computes the corresponding editing host from the focus node of
`Selection` with climbing up the DOM tree. So, it does not just return a stored
element. Therefore, some callers use it multiple times. For avoiding it, we
should rename it to explain that it computes the editing host.
Note that I think that we should make it takes a node to compute editing host
without `Selection` for solving the case of no selection ranges. Therefore,
I don't like to include more information into the name.
Differential Revision: https://phabricator.services.mozilla.com/D147504
It returns the anonymous `<div>` element if the instance is a `TextEditor`, and
compute editing host otherwise. So it's unclear what it returns. Additionally,
all users except `EditorBase::CreateTransactionForCollapsedRange` are the
methods of `HTMLEditor`. Therefore, we should remove it and unwrap the code
which it's done.
Differential Revision: https://phabricator.services.mozilla.com/D147503
None of the consumer need to mutate styles, and this saves some ugly
const_casting on the next patch.
Doesn't change behavior.
Differential Revision: https://phabricator.services.mozilla.com/D147555
I'd like to split it for `TextEditor` and `HTMLEditor`. `HTMLEditor` does not
require flushing the layout because it's required by `TextEditor` if and only if
a preceding `focus` event listener or the `Element.focus` caller reframes
`<input>` or `<textarea>` element (bug 1755104). So this would fix bug 1760045.
Differential Revision: https://phabricator.services.mozilla.com/D147143
`nsFocusManager` defines that focused content is at least `dom::Element`.
So `IMEStateManager` can handle focused content with `dom::Element` too.
Additionally, this patch makes `IMEStateManager` methods take references instead
of pointers as far as possible if they just return error for `nullptr`.
Differential Revision: https://phabricator.services.mozilla.com/D147133
Creating both `DeleteNodeTransaction` and `InsertNodeTransaction` wastes
memory. They should be done in an instance instead.
Fortunately, no edit action listener checks whether the deleted node is still
in the composed document or not, etc. Therefore, we can simply notify them of
both deletion and insertion which were done in
`EditorBase::InsertNodeWithTransaction` and
`EditorBase::DeleteNodeWithTransaction`. Note that previously, the range
updater needs to ignore the notifications from them while the node is being
moved. However, it does not require anymore. Therefore, this patch makes it
stop locking, and that would fix minor problem in the case of legacy mutation
event listeners run another edit action.
On the other hand, this changes some edge cases handling of
`MoveNodeWithTransaction` which are detected by the WPT. According to the
previous result of applying this patch, `nsINode::InsertBefore` fails and that
leads some errors at updating the changed range. I guess that the cause is
that there is some bugs at updating insertion point after deleting the node from
the DOM tree around here:
https://searchfox.org/mozilla-central/rev/0ffae75b690219858e5a45a39f8759a8aee7b9a2/editor/libeditor/HTMLEditor.cpp#5058-5071
However, it's safely fixed by the new code which does not remove the node from
the DOM tree explicitly. So, I think that it's safe to accept this behavior
change for web apps in the wild.
Differential Revision: https://phabricator.services.mozilla.com/D146397
This patch also makes it inserts table rows immediately after the previous
row element of the found reference row element to make it insert new table
rows immediately after there when inserting position is "after". Typically,
it's the parent `<tr>` element of `aCellElement`. Therefore, this approach
can preserve text nodes immediately after it which was intentionally inserted
by the web apps.
Differential Revision: https://phabricator.services.mozilla.com/D146365
It needs to work with the latest layout information to consider which cell
element is the insertion point due to rowspan and colspan. Therefore,
this patch makes it collects all cell data before touching the DOM except
the case that it needs to normalize the table to make it rectanble.
Note that the case requiring the normalizer should be fixed in a later patch.
This method is corresponding to an XPCOM method. Therefore, this is tested
by `test_nsITableEditor_insertTableColumn.html`.
And also it's used by the inline table editor, but we don't have automated tests
for this because of no API to get the buttons. Therefore, I tested it by my
hand.
Note that the old code fails to put caret to newly inserted cell at the reftest
situation. This fixes the bug too. Therefore, this changes the reftest's
reference.
Differential Revision: https://phabricator.services.mozilla.com/D146364
There is no direct test because of no corresponding XPCOM method, but this is
called only by `HTMLEditor::InsertTableColumnsWithTransaction()` which is
tested by `test_nsITableEditor_insertTableColumn.html`. Anyway, the chagne is
really simple.
Differential Revision: https://phabricator.services.mozilla.com/D146363
It refers the layout information for getting row/column index of selected cell
element or a cell element which contains selection. However, it can stop
referring it if all callers can specify insertion point of new cells, and it's
possible. Therefore, we can make it free from layout information.
Note that this blocks legacy mutation events during inserting table cell
elements, but perhaps this does not cause problems in the wild.
`DOMNodeInserted` will be fired after all cell elements are inserted, so
from point of view of the event listeners, editor content is different from
traditional behavior, but this works only when user uses inline table editor
which is disabled by default. I.e., in the wild, this path is rarely run.
Additionally, this patch makes it return `CreateElementResult` for the latter
changes which want to put caret to first inserted cell element.
Note that the changes are tested by `test_nsITableEditor_insertTableColumn.html`
and `test_nsITableEditor_insertTableCell.html`.
Differential Revision: https://phabricator.services.mozilla.com/D146361
Our table editor depends on layout information for getting raw/column position
of a cell, checking rawspan/colspan information and table size. Therefore,
they require the latest information, but they don't flush pending layout by
themselves. Therefore, this patch makes them do it by themselves and deleting
unnecessary hack from their tests.
Differential Revision: https://phabricator.services.mozilla.com/D146359
It needs to work with the latest layout information to consider which cell
element is the insertion point due to rowspan and colspan. Therefore,
this patch makes it collects all cell data before touching the DOM except
the case that it needs to normalize the table to make it rectanble.
Note that the case requring the normalizer should be fixed in a later patch.
This method is corresponding to an XPCOM method. Therefore, this is tested
by `test_nsITableEditor_insertTableColumn.html`.
And also it's used by the inline table editor, but we don't have automated tests
for this because of no API to get the buttons. Therefore, I tested it by my
hand.
Depends on D146363
Differential Revision: https://phabricator.services.mozilla.com/D146364
There is no direct test because of no corresponding XPCOM method, but this is
called only by `HTMLEditor::InsertTableColumnsWithTransaction()` which is
tested by `test_nsITableEditor_insertTableColumn.html`. Anyway, the chagne is
really simple.
Depends on D146362
Differential Revision: https://phabricator.services.mozilla.com/D146363
This change is tested by `test_nsITableEditor_getFirstRow.html` and
`test_nsITableEditor_insertTableColumn.html`.
Depends on D146361
Differential Revision: https://phabricator.services.mozilla.com/D146362
It refers the layout information for getting row/column index of selected cell
element or a cell element which contains selection. However, it can stop
referring it if all callers can specify insertion point of new cells, and it's
possible. Therefore, we can make it free from layout information.
Note that this blocks legacy mutation events during inserting table cell
elements, but perhaps this does not cause problems in the wild.
`DOMNodeInserted` will be fired after all cell elements are inserted, so
from point of view of the event listeners, editor content is different from
traditional behavior, but this works only when user uses inline table editor
which is disabled by default. I.e., in the wild, this path is rarely run.
Note that the changes are tested by `test_nsITableEditor_insertTableColumn.html`
and `test_nsITableEditor_insertTableCell.html`.
Depends on D146360
Differential Revision: https://phabricator.services.mozilla.com/D146361
Our table editor depends on layout information for getting raw/column position
of a cell, checking rawspan/colspan information and table size. Therefore,
they require the latest information, but they don't flush pending layout by
themselves. Therefore, this patch makes them do it by themselves and deleting
unnecessary hack from their tests.
Differential Revision: https://phabricator.services.mozilla.com/D146359
For avoiding updating `CellData` member(s) to make the static analysis work,
`CellData::Update` shouldn't be used. Instead, it should be created for each
cell whose detail is required.
Differential Revision: https://phabricator.services.mozilla.com/D146137
The callers don't need the detail of the error, and the runtime cost of
`ErrorResult` may not be cheap because it sometimes appear in profiles.
Therefore, they should be removed.
Differential Revision: https://phabricator.services.mozilla.com/D146136
They will be changed to factory methods. Therefore, they shouldn't be in
middle of the cpp file.
This patch just moves them without any changes.
Differential Revision: https://phabricator.services.mozilla.com/D146135
For avoiding updating `CellData` member(s) to make the static analysis work,
`CellData::Update` shouldn't be used. Instead, it should be created for each
cell whose detail is required.
Differential Revision: https://phabricator.services.mozilla.com/D146137
The callers don't need the detail of the error, and the runtime cost of
`ErrorResult` may not be cheap because it sometimes appear in profiles.
Therefore, they should be removed.
Differential Revision: https://phabricator.services.mozilla.com/D146136
They will be changed to factory methods. Therefore, they shouldn't be in
middle of the cpp file.
This patch just moves them without any changes.
Differential Revision: https://phabricator.services.mozilla.com/D146135
Persist dictionary preference in document content language field for mail editors so multiple dictionary selection can persist.
Differential Revision: https://phabricator.services.mozilla.com/D144935
Masayuki Nakano
Iulian Moraru
Nika Layzell
Magnus Melin
Ting-Yu Lin
Makoto Kato
Mark Banner
Daniel Holbert
André Bargull
Tooru Fujisawa
Neil Deakin
Joel Maher
Kris Maglione
Brad Werth
Dan Minor
Emilio Cobos Álvarez
Norisz Fay
criss
Butkovits Atila
Tyson Smith
Molnar Sandor
Kagami Sascha Rosylight
Bogdan Szekely
Daisuke Akatsuka
Martin Giger
John Bieling