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
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
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
This patch also makes that `HTMLWithContextInserter::InsertContents`
collect moving children first. Then, move each one with a transaction.
This is standard manner in these days to avoid infinite loop caused by
moving the removed child back by a mutation event listener.
Differential Revision: https://phabricator.services.mozilla.com/D144658
Now, it does not require to update selection, and its result may have a point
to put caret it is used to doing. This patch makes it stop collapsing selection
after each split, and instead, makes all callers of it collapse selection if
it's (probably) necessary.
Note that the method may not split any nodes but return it as "handled".
Therefore, the callers do not check whether a splitting node occurred **but**
suggesting point to put caret is invalid or the other cases. Therefore, in
strictly speaking, this patch changes the existing behavior, but it should not
affects web apps in the wild because of the low usage of the legacy mutation
event listeners.
Differential Revision: https://phabricator.services.mozilla.com/D144650
This patch allows methods which split some nodes to return new candidate
caret position and makes callers of them consider whether applying it to
the selection immediately or not.
Differential Revision: https://phabricator.services.mozilla.com/D144648
Some of them were `HTMLEditor` so that we need to fix it to `EditorBase`, and
also this patch fixes `NS_ERROR_EDITOR_DESTROYED` handling around them.
Differential Revision: https://phabricator.services.mozilla.com/D143815
Capturing everything causes increasing the binary size especially of debug
build so that we should add more arguments to the callback and make some
lambda methods capture individual variables when they are necessary.
Note that nobody uses the 3rd argument, `aPointToInsert`, it'll be used to
fix bug 1759370 and makes the change smaller.
This decreases the size of xul.dll of debug build on Windows 123MB from 323MB.
I tried to delete all captures from the lambdas, but it does not affect to the
binary size and it makes the code harder to read, killing static analysis.
Therefore, we should not do it.
Depends on D141196
Differential Revision: https://phabricator.services.mozilla.com/D141447
For avoiding simple back-out of the patches when we get serious regression
reports, we should have a pref to disable the new pref.
Differential Revision: https://phabricator.services.mozilla.com/D140475
With the change of the previous patch, `HTMLEditor` won't delete empty elements
in the inserted HTML content. However, at bug 1123505, it intentionally tried
to delete empty list elements which have no list item elements since such
list elements won't be editable. Therefore, the following patch makes some
tests in `test_copypaste.html` fail.
https://searchfox.org/mozilla-central/rev/3de56eb5f266f523340e739ae1b53258e0a95dfe/dom/base/test/copypaste.js#343,360
Unfortunately, the broken behavior is compatible with Chrome (Chrome accept
list elements which have no list item elements), but it does not make sense for
Gecko's builtin editor. Therefore, I think that we should keep our traditional
behavior with deny-list.
This patch makes `FragmentFromPasteCreator` delete list elements which have no
list item elements and are empty from the inserting document fragment.
Differential Revision: https://phabricator.services.mozilla.com/D136211
This is same issue as bug 1747008, but the remover is different method, that is
`HTMLEditor::RemoveEmptyNodesIn` called by
`HTMLEditor::OnEndHandlingTopLevelEditSubActionInternal`. It should be called
only when the flag which was added by bug 1747008 is **not** set to `false`.
Differential Revision: https://phabricator.services.mozilla.com/D136210
Starting from bug 1730429, we strip empty inline elements at caret for
compatibility with Blink/WebKit. However, we should not do it for the elements
which are intentionally inserted (from `inserthtml` command, paste and DnD).
All the cases are handled by `HTMLEditor::HTMLWithContextInserter` so that
it should prevent the new clean up with `TopLevelEditSubActionData`.
Note that `inserthtml` command handling of Blink is really odd. It inserts
the empty inline elements of the adding testcases into different place so that
Chrome does not pass the new tests. However, it does not make sense and I
believe that it's out of scope of this bug.
Differential Revision: https://phabricator.services.mozilla.com/D135195
First, left/right node accessors are not used so that we can get rid of them.
However, we should have similar methods which can retrieve original node and
new node. Therefore, this adds `GetNewContent()` and `GetOriginalContent()`.
Next, `SplitPoint()` should return both `EditorDOMPoint` and `EditorRawDOMPoint`
for avoiding unnecessary conversion from `EditorRawDOMPoint` to
`EditorDOMPoint`. Therefore, this patch makes it a template method too.
Finally, this patch adds helper methods to get point of each related content.
E.g., `AtNewContent()` corresponding to `GetNewContent()`.
Differential Revision: https://phabricator.services.mozilla.com/D131748
It touches the DOM tree only with `SplitNodeTransaction()` and it now returns
`NS_ERROR_EDITOR_DESTROYED` so that the callers don't need to check whether
the editor is destroyed or alive by themselves.
Depends on D131043
Differential Revision: https://phabricator.services.mozilla.com/D131044
Currently, it uses "normal text" and "normal white-spaces" for naming
`enum class` members and their accessors. However, this is unclear what
does the normal mean since the word depends on context.
Therefore, this patch replaces the former with "non-collapsible characters" and
the latter is "collapsible white-spaces".
Differential Revision: https://phabricator.services.mozilla.com/D123872
On Thunderbird, we still have a bug to delete `<html>` and `<body>` elements
(bug 1727201). However, it's hard to know where deletes the unexpected elements
from warning messages in the log. Additionally, it's really serious bug
because editor and layout code rely on the basic structure of HTML document.
Therefore, we should block the worst scenario before deleting such nodes.
Differential Revision: https://phabricator.services.mozilla.com/D123418
The method assumes two wrong things:
* The padding `<br>` element flag may not be set (another bug)
* There may be `<br>` element which is created by the web app
But its callers want to put caret at **invisible** `<br>` element if selection
is collapsed after it. Therefore, this patch fixes the method for passing
the new tests and rename it.
And also this patch changes the expected result of some tests in `inserthtml.js`
because their expected result are based on Gecko, i.e., both Blink/WebKit
fail, but their result is better for keeping the invisible `<br>` element
visibility.
https://wpt.fyi/results/editing/run/inserthtml.html?run_id=5747864689967104&run_id=5201845715730432&run_id=5735315550502912&run_id=5763864667881472
Depends on D123068
Differential Revision: https://phabricator.services.mozilla.com/D123069
Web apps can modify normal selection even during IME composition and no
browsers stop composition by it. However, our editor tries to delete
non-collapsed selected range before updating composition. Therefore,
we need additional state at handling inserting text whether selection
should be deleted or ignored.
Depends on D121371
Differential Revision: https://phabricator.services.mozilla.com/D121372
Previously, `HTMLEditor::GetBetterInsertionPoint()` didn't check whether
given point is in an editing host or not. However, now
`HTMLEditUtils::GetBetterInsertionPoint()` does it with editing host which
is returned by `HTMLEditor::GetActiveEditingHost(LimitInBodyElement::No)`.
However, the old behavior is exactly same as
`HTMLEditor::GetActiveEditingHost(LimitInBodyElement::Yes)` if editing host
is outside the `<body>` element.
For taking back the original behavior, we should call the method with the
result of the latter.
Differential Revision: https://phabricator.services.mozilla.com/D121370
Unfortunately, marking its constructor and destructor as `MOZ_CAN_RUN_SCRIPT`,
`Maybe<AutoSelectionRestorer>::reset()` and
`Maybe<AutoSelectionRestorer>::emplace()` cause bustage. Therefore, this patch
just mark them as `MOZ_CAN_RUN_SCRIPT_BOUNDARY`.
Note that `EditorBase::SavedSelectionRef()` cannot be moved to `HTMLEditor`
because `mEditActionData` is a private member of `EditorBase`.
Depends on D119001
Differential Revision: https://phabricator.services.mozilla.com/D119002
It should be treated as `uint32_t` since DOM API does so. However, there are
some exceptions:
* Result of `nsINode::ComputeIndexOf()`
* Result of `nsAString` methods
They return `-1` as not found, and anyway, they cannot treat large integer
than `INT32_MAX`. Therefore, this patch does not touch around them.
Differential Revision: https://phabricator.services.mozilla.com/D118933
Developers may be confused at `IsTextEditor()` and `IsPlaintextEditor()`. When
the latter is `true`, the former is always `true`, but it may be `true` when the
editor is `HTMLEditor` too. So, it's a mode of `HTMLEditor`.
Differential Revision: https://phabricator.services.mozilla.com/D118246
For making it clearer that `TextEditor` has only its specific members. For
guaranteeing that, we should split `TextEditor` and `HTMLEditor`.
Differential Revision: https://phabricator.services.mozilla.com/D117382
`DataTransfer` manages its items with `uint32_t` now, but editor methods still
access with `int32_t`. Therefore, editor module should use `uint32_t`.
Depends on D117115
Differential Revision: https://phabricator.services.mozilla.com/D117116
`TextEditor::OnDrop()` handles both cases, in `TextEditor` and in `HTMLEditor`
because the common part is too complicated to duplicate. However, most
different part is inserting the dropped items part. So, let's make them
into a virtual method.
In this patch, creating a method only in `HTMLEditor` and moves the part
into it.
Depends on D116569
Differential Revision: https://phabricator.services.mozilla.com/D116801
It should be a virtual method derived from `EditorBase`, and `TextEditor`
and `HTMLEditor` should override it. Then, `nsIEditor::Paste()` requires
referring vtable again if we keep implementing it only in `EditorBase`.
Therefore, this patch avoid it with implementing it in both `TextEditor`
and `HTMLEditor`.
Depends on D116567
Differential Revision: https://phabricator.services.mozilla.com/D116568
It just creates an `nsITransferable` instance and add 2 flavors for storing
plain text. Therefore, it can be in `EditorUtils` instead.
Depends on D116566
Differential Revision: https://phabricator.services.mozilla.com/D116567
