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
This is a regression of bug 1311934. The traditional
`TextServicesDocument::IsBlockNode()` was wrong name, the users of the method
intent to check whether a word is not split by the found sibling. Therefore,
the method returned `true` for `<br>` element. However, of course,
`HTMLEditUtils::IsBlockElement()` is not so because of an inline element.
For making the callers clearer, this patch adds `<br>` element check into
each place. Without this patch, the new test fails to replace a misspelled
word immediately after `<br>` element.
Depends on D123283
Differential Revision: https://phabricator.services.mozilla.com/D123284
The new editor utility method does not stop scanning editable elements even if
it reaches the document root nor the (primary) `<body>` element. Of course,
they should stop there if scanning editable block. And
`ScanEmptyBlockInclusiveAncestor()` shouldn't store the removable empty block
element to them.
Differential Revision: https://phabricator.services.mozilla.com/D123316
I have no idea how to test this because this method is a clean up method
after handling everything. This patch just adds same editing host check,
but the others are not changed.
Finally, the legacy APIs are not used anymore. So, let's remove them!
Differential Revision: https://phabricator.services.mozilla.com/D123075
I have no idea how to write test for this kind of preparation method of
following range computation to do something.
Depends on D123072
Differential Revision: https://phabricator.services.mozilla.com/D123073
Comparing found editable block with editing host does not make sense if
focused editing host has another editing host and selection is in the
child editing host since the child editing host should be compared with the
found block element. Therefore, this patch makes it use `IsSplittableNode()`
instead of the comparing.
Depends on D123069
Differential Revision: https://phabricator.services.mozilla.com/D123070
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
Additionally, the given content may be in a nested editing host. Then, this
method may delete a nested-block editing host if it's empty. Therefore,
this patch get rid of `aEditingHost` and check same thing with
`HTMLEditUtils::IsRemovableFromParentNode()`.
Depends on D123066
Differential Revision: https://phabricator.services.mozilla.com/D123067
I have no idea how to test this, but I believe that checking non-editable
block element must be correct here because it checks whether the block is
empty or not for merging lines.
Depends on D123029
Differential Revision: https://phabricator.services.mozilla.com/D123066
The "state" is not limited by editing host nor editable nodes. Therefore, it
should keep scanning ancestor block element without checking editable state.
Differential Revision: https://phabricator.services.mozilla.com/D122948
For keeping current behavior, the options should be set to
`HTMLEditUtils::ClosestEditableBlockElement`, but it may cause returning
`nullptr` if the text node is in an inline editing host, and also cause
returning true from the method even when the text nodes are in different
inline editing hosts. Therefore, this patch uses
`HTMLEditUtils::ClosestEditableBlockElementOrInlineEditingHost` instead.
Differential Revision: https://phabricator.services.mozilla.com/D122947
This patch fixes a bug of setting background color of block parent. It should
not work with `contenteditable` in the comm-central applications, but we should
make it modify only editable block parent.
Differential Revision: https://phabricator.services.mozilla.com/D122945
With the new tests, I see the implementation is completely broken. It tries
to return block level element's background color, but it assumes that inline
elements are never parent of block elements. Therefore, this patch fixes the
bug too.
Note that this feature (retrieving only block level background color) is
Gecko specific and not available from the web. Therefore, it tests this change
with XPCOM interfaces directly.
Differential Revision: https://phabricator.services.mozilla.com/D122943
It does not check whether it meets a non-editable parent or not. Therefore,
it may cross another editing host boundary when `aContent` is in a nested
editing host.
So, this patch fixes some edge cases when editing hosts are nested and
scanning from inner editing host.
Differential Revision: https://phabricator.services.mozilla.com/D122940
There are a lot of ancestor scanners in `HTMLEditUtils`. This is good thing
for the performance, but it makes us hard to maintain. Therefore, we should
merge them as far as possible.
Differential Revision: https://phabricator.services.mozilla.com/D122939
If array size grows up, the array data may be reallocated. Therefore, after
getting a reference of an array item, we shouldn't modify array.
This makes them use raw pointer if they need to modify the array. Otherwise,
keep using the reference, but adds a stack class to detect the bug only in debug
build.
Differential Revision: https://phabricator.services.mozilla.com/D122920
This is just a simple mistake, but I'm not sure how this break the callers.
Therefore, I don't have a testcase for this.
Differential Revision: https://phabricator.services.mozilla.com/D122705
They clean up the tail of descendant block first. At this time, running script
may change one of or both of the left block element and the right block element.
In such situation, they should stop handling to join the blocks because of
unexpected case.
Depends on D122562
Differential Revision: https://phabricator.services.mozilla.com/D122565
It tries to delete a text node if it's invisible both selection start and
selection end. However, the selection range may be in a text node. If it's
an invisible, the node is deleted at first call of
`DeleteNodeIfInvisibleAndEditableTextNode()`, but it keep trying to remove
again with same method.
Differential Revision: https://phabricator.services.mozilla.com/D122561
The test causes different assertion count between platforms. As the comment in
crashtests.list, the difference is whether "removeList" command enabled check
is run at getting focus or not. Perhaps, the difference is caused by whether
HTML commands are initialized before or after executing "indent" command.
Differential Revision: https://phabricator.services.mozilla.com/D122546
`WSRunScanner::ScanNextVisibleNodeOrBlockBoundary()` is designed for scanning
editable content so that it's wrong to use it for scanning first editable leaf
node.
Differential Revision: https://phabricator.services.mozilla.com/D122479
It tries to replace a following NBSP with an ASCII white-space if there is.
However, it calls the scan method with start of the replacing range. Therefore,
the assertion in `GetInclusiveNextNBSPPointIfNeedToReplaceWithASCIIWhiteSpace()`
detects this bug.
Note that this occurs only when updating composition string because it's
called with non-collapsed range only for doing it. Otherwise, selected range
has already been deleted by `HTMLEditor::DeleteSelectionAsSubAction()`.
Unfortunately, I don't have how to make this bug appear. It seems that the
path does nothing in the wild because it tries to replace a first character
of composition string from an NBSP to a normal white-space, but it'll be
replaced with new composition string anyway. Therefore, this patch does not
have new tests.
Differential Revision: https://phabricator.services.mozilla.com/D122182
When getting suggestions from spellchecker's result, we use sync IPC
(`PRemoteSpellcheckEngine.CheckAndSuggest`). This is used by showing context
menu only on Gecko. So I think that we can remove this IPC if we add async API
to get spellchecker suggestions.
And in comm-central's code, `CheckCurrentWord` and `GetSuggestedWord` seems to
use on spellchecker dialog (content/dialogs/EdSpellCheck.js in mail and suite)
that runs on parent process. So c-c won't use this IPC method.
So I would like to add the promise version of getting spellchecker's
suggestion.
Differential Revision: https://phabricator.services.mozilla.com/D119936
This will make implementing the new behavior behind a pref
really straight-forward, and is generally nicer.
Depends on D121858
Differential Revision: https://phabricator.services.mozilla.com/D121705
It handles selection indexes really roughly since it does it only when
start index is same or greater then removing index. So, only end index
may be greater than the removing index, and if the start index is 0, it
sets `-1` to selection start.
I don't find any other issues around setting selection indexes. However,
for detecting wrong selection index management, this patch adds
`MOZ_DIAGNOSTIC_ASSERT` to every setter.
Differential Revision: https://phabricator.services.mozilla.com/D121592
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
The function tests whether `input` event whose `inputType` is `insertFromPaste`
or not and it waits the event until timeout. This sometimes fails in my
environment, and `input` event should be fired synchronously in this case.
Therefore, it should just fail if the event isn't fired.
Depends on D120424
Differential Revision: https://phabricator.services.mozilla.com/D120425