Closed Bug 1644313 Opened 4 years ago Closed 4 years ago

Split `HTMLEditor::HandleDeleteAroundCollapsedSelection()` to range computation part and modifying DOM tree part (only for the case collapsed in a text node)

Categories

(Core :: DOM: Editor, task, P2)

task

Tracking

()

RESOLVED FIXED
81 Branch
Tracking Status
firefox81 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

Attachments

(6 files)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
No description provided.
Severity: -- → S3
Priority: -- → P2
Status: NEW → ASSIGNED
Summary: Split `HTMLEditor::HandleDeleteAroundCollapsedSelection()` to range computation part and modifying DOM tree part → Split `HTMLEditor::HandleDeleteAroundCollapsedSelection()` to range computation part and modifying DOM tree part (only for the case collapsed in a text node)

InputEvent.getTargetRange() should have actual delete range. It means that
the range should contain the invisible leading/trailing white-spaces which
need to be removed for keep them invisible, but should not contain the range
of normalizing white-space sequence because they are not target of edit action
indicated by InputEvent.inputType.

So, we can use new path which uses the new white-space normalizer for
computing the value of InputEvent.getTargetRanges() because difference of
white-space normalizer shouldn't affect the deleting ranges (although, some
existing path calls DeleteNodeIfInvisibleAndEditableTextNode() later so that
the new method, ComputeRangeInTextNodesContainingInvisibleWhiteSpaces(), does
not exactly same thing, but the result shouldn't become different in usual
cases). This new path can test with some WPTs under editing/other.

This patch creates new backspace/delete key handler when caret is at next
to a white-space as HTMLEditor::HandleDeleteTextAroundCollapsedSelection()
and creates helper methods of WSRunScanner to treat invisible leading and
trailing white-spaces.

Note that new failures are caused by the difference whether adjacent white-space
sequence at deletion is normalized or not in edge cases. They will be fixed
by the part.5.

Depends on D84943

For the following patches, it's nicer that the API has optional behavior which
removes new empty text node or empty parent elements because directly removing
parent element can reduce the number of transactions and avoid storing text
with DeleteTextTransaction().

Depends on D84944

When it calls DeleteTextAndNormalizeSurroundingWhiteSpaces(), we need to
remove empty text nodes and newly empty inline ancestors. Currently, empty
nodes are removed by HTMLEditor::OnEndHandlingTopLevelEditSubActionInternal():
https://searchfox.org/mozilla-central/rev/25d491b7924493b5d4bedc07841a1cd92f271100/editor/libeditor/HTMLEditSubActionHandler.cpp#510
But we should stop using this in this case for reducing transactions.

Depends on D84945

The difference between HTMLEditor::HandleDeleteCollapsedSelectionAtTextNode()
and HTMLEditor::HandleDeleteCollapsedSelectionAtWhiteSpace() is whether
it treats surrogate pair or not. And it's easy to handle it in WSRunScanner.
Therefore, they can handle all deletion in a text node.

Some new failures are caused by that we normalize preceding white-spaces of
deletion range but Blink does not to it. These failures will be fixed by
part 5.

Depends on D84946

Blink normalizes preceding white-spaces of deleting range only when the range is
NOT remove last character or there are no following text nodes. If removing
last character at first text node which is followed by another text node, Blink
normalize white-spaces at start of the following text node.

This behavior seems odd, but we should follow the Blink's behavior for better
compatibility.

Note that new test failures in
white-spaces-after-execCommand-forwarddelete.tentative.html is caused by
that forward deletion is handled by existing path instead of the new handler.
Therefore, it's no problem for now.

Additionally, this change causes a mochitest
(editor/libeditor/tests/test_bug1425997.html) and a crash test
(editor/libeditor/crashtests/1424450.html) becoming oranges since their
DOM tree becomes different after applying this patch. The oranges will
be fixed by the following patches.

Depends on D84947

The previous patch changed target of white-space normalization. That caused
editor/libeditor/tests/test_bug1425997.html starting to failed with new
exception. What happens in the test is, selection is collapsed into comment
node (I think that this behavior is also a bug though) after second
document.execCommand("delete"), and then, failed to delete anything
and throws the exception. For fixing this issue, the most simple approach is
make HTMLEditor::HandleDeleteAroundCollapsedSelection() stop handling
deletion when caret position is not editable. So, unfortunately, this
patch changes traditional path too.

Depends on D84948

Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/59762a664470
part 1: Create new path to handle backspace or (forward) delete with the new normalizer r=m_kato
https://hg.mozilla.org/integration/autoland/rev/08ce5e7aabc9
part 2: Make `HTMLEditor::DeleteTextAndTextNodeWithTransaction()` remove empty text node or most distant empty inline ancestor optionally r=m_kato
https://hg.mozilla.org/integration/autoland/rev/d7dd84a27d30
part 3: Make `HTMLEditor::HandleDeleteTextAroundCollapsedSelection()` call `HTMLEditor::DeleteTextAndNormalizeSurroundingWhiteSpaces()` with `TreatEmptyTextNodes::RemoveAllEmptyInlineAncestors` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/c32c0cc3964d
part 4: Make `WSRunScanner::GetRangeInTextNodesTo*From()` available with non-white-space characters r=m_kato
https://hg.mozilla.org/integration/autoland/rev/2fd3fc20dd99
part 5: Make `ExtendRangeToDeleteWithNormalizingWhiteSpaces()` not normalize preceding white-spaces when deleting range all characters in first text node after start to delete r=m_kato
https://hg.mozilla.org/integration/autoland/rev/f92770ba1a83
part 6: Make `HTMLEditor::HandleDeleteAroundCollapsedSelection()` cancel the deletion if caret position is not editable r=m_kato
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/24806 for changes under testing/web-platform/tests
Upstream PR merged by moz-wptsync-bot
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: