Page MenuHomePhabricator

Bug 1951518 - Make `HTMLEditor::DeleteRangesWithTransaction` stop deleting `Text` which has only a collapsible white-space r=m_kato!
ClosedPublic

Authored by masayuki on Mar 7 2025, 12:17 AM.
Referenced Files
Unknown Object (File)
Fri, Apr 10, 2:18 AM
Unknown Object (File)
Thu, Apr 9, 9:40 PM
Unknown Object (File)
Thu, Apr 9, 6:11 PM
Unknown Object (File)
Thu, Apr 9, 9:00 AM
Unknown Object (File)
Tue, Apr 7, 10:23 AM
Unknown Object (File)
Fri, Apr 3, 1:54 AM
Unknown Object (File)
Nov 21 2025, 8:56 AM
Unknown Object (File)
Oct 13 2025, 9:05 AM
Subscribers
None

Details

Summary

In bug 1925635, I moved the post processing after deleting ranges in
EditorBase::DeleteRangesWithTransaction into the HTMLEditor's override.
At this time, I makes it uses HTMLEditUtils::IsEmptyNode to check whether
the Text and its containers become empty. Therefore, if Ctrl+Backspace
deletes the only word in the Text which starts with a collapsible white-space
causes deleting the Text after deleting all visible characters in the Text.

Therefore, this makes check it with Text::TextDataLength too before searching
the most distant ancestor inline element which becomes empty.

However, it may cause leading invisible white-spaces of the only word visible
because nsFrameSelection computes the word range strictly in the Text.
Therefore, this patch also makes HTMLEditor::DeleteRangesWithTransaction and
AutoDeleteRangesHandler::ComputeRangesToDeleteRangesWithTransaction extend
each range to delete to include surrounding invisible white-spaces too.

Therefore, this patch adds the new method to AutoClonedRangeArray.

Then, I hit 4 existing bugs with new test failures.

One is HTMLEditUtils::LineRequiresPaddingLineBreakToBeVisible. It checks
whether the candidate point to insert a <br> is followed by a block boundary
first. Then, it checks whether the candidate point follows a collapsible
white-space or a block boundary. However, it uses
WSRunScanner::ScanPreviousVisibleNodeOrBlockBoundary() which ignores invisible
white-spaces. Therefore, it will ignore the new invisible white-space and
reaches preceding Text. Thus, it fails to put a <br> and makes the new
invisible white-space "fixed" as invisible.

Therefore, this patch rewrites the check with using
HTMLEditUtils::GetPreviousLeafContentOrPreviousBlockElement().

Next one is, HTMLEditor::DeleteEmptyInclusiveAncestorInlineElements()
returns end of deleted node if it's followed by a non-editable node, i.e.,
an element has contenteditable="false". Therefore, its caller inserts a
<br> to the end of the container when deleting preceding editable node of a
non-editable node.

Therefore, this patch removes the editable state check.

Next, AutoBlockElementsJoiner::HandleDeleteNonCollapsedRange may put a padding
<br> after the moved line, but it does not assume that the moved line does not
ends with a block boundary. This causes failing #46 and #48 tests in
text_bug772796.html. E.g., when pressing Delete in
<div>foo[]<div><span style="white-space:pre"><div>bar</div>baz</span>, we move
the second <div> as a child of the parent <span> to end of the first <div>
like <div>foo<span style="white-space:pre"><div>bar</div></span></div>....
Without the change, it starts to put unnecessary <br> after the <span>`
because of the bug fix in HTMLEditUtils::LineRequiresPaddingLineBreakToBeVisible
above. This result is completely odd from the users point of view (looks like
just move caret), but we should avoid to put the unnecessary <br>.

Finally, we'll fail an assertion when putting caret at the last of
AutoBlockElementsJoiner::DeleteContentInRange because it forgets to flush
the tracking range before using it. This appeared by the changes above.

Therefore, this patch fixes this bug too.

Diff Detail

Event Timeline

phab-bot changed the visibility from "Custom Policy" to "Public (No Login Required)".
phab-bot changed the edit policy from "Custom Policy" to "Restricted Project (Project)".
phab-bot removed a project: secure-revision.

This needs to work before shipping the new white-space normalizer. Therefore, this should be fixed before enabling it in the nightly channel.

masayuki edited the summary of this revision. (Show Details)
This revision is now accepted and ready to land.Mar 17 2025, 10:20 AM