Closed Bug 1792387 Opened 2 years ago Closed 2 years ago

Land the patches for bug 1735608

Categories

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

Product:
Core
Component:
DOM: Editor
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
107 Branch
Tracking Flags:
Tracking Status
firefox107 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

Attachments

(12 files)

Bug 1792387 - part 1: Make `HTMLEditor` join/split node direction switchable by a pref r=m_kato!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1792387 - part 2-1: Make `HTMLEditor::DoJoinNodes()` adjust selection for both join nodes directions r=m_kato!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1792387 - part 2-2: Make `HTMLEditor::DoJoinNodes()` handle both join nodes directions r=m_kato!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1792387 - part 3-1: Make `HTMLEditor::DoSplitNode()` adjust selection for both split node directions r=m_kato!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1792387 - part 3-2: Make `HTMLEditor::DoSplitNode()` handle both split node directions r=m_kato!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1792387 - part 4: Make `JoinNodesTransaction` and `SplitNodeTransaction` support both join/split node directions r=m_kato!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1792387 - part 5: Make `HTMLEditor::SplitRangeOffFromBlock` and its caller refer `SplitNodeResult` at their post-processing r=m_kato!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1792387 - part 6: Make `HTMLEditor::InsertElementWithSplittingAncestorsWithTransaction` check edge case with next node of the split point r=m_kato!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1792387 - part 7: Make `editor/libeditor/test_bug449243.html` be aware of both split node directions r=m_kato!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1792387 - part 8: Make `HTMLEditor::InsertParagraphSeparatorAsSubAction` put caret into right paragraph r=m_kato!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1792387 - part 9: Make `widget/tests/test_composition_text_querycontent.xhtml` be aware of both split/join node directions r=m_kato!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1792387 - part 10: Make `editor/libeditor/tests/test_inline_style_cache.html` be aware of both split node directions r=m_kato!
48 bytes, text/x-phabricator-request
Details | Review

Although I still see 2 issues (one is bug 1792386 and/or bug 1789574, the other is odd failures of test_dragdrop.html) with the patches for bug 1735608 in my local repository, I think that it's time to land the patches.

One is, a newbie may touch HTMLEditor, in the case, my rebasing cost will be increased and rebasing with the other people's patches is riskier than doing it with my patches. And personally, the patches block some refactoring of the editor module of mine when I have some spare time.

Of course, the new behavior is disabled by default until we get QA after landing.

This patch add new path to adjust selection for the new join nodes direction.
So this does not change any behavior of the existing path.

Depends on D158097

This changes the existing path's behavior a bit. However, it should not affect
to web apps in the wild because it must be really rare case that web apps
inserting new nodes into the removing node while moving its children.

Depends on D158098

This patch adds new path to adjust selection for the new split node direction.
So this does not change any behavior of the existing path.

Depends on D158099

This does not change the existing path's behavior.

Depends on D158100

The reason why the changes of SplitNodeTransaction is smaller than
JoinNodesTransaction is, HTMLEditor::DoSplitNode does almost all things
instead.

Depends on D158101

The check assumes that the container of aPointToInsert is always right node.
However, its child is moved to new node if we switch the direction, so we should
make it check the edge case with SplitNodeResult::GetNextContent().

Depends on D158103

The test compares the node name of the split element and its previous sibling
element, so it should switch the scanning direction with the pref to switch
split node direction.

Note that the behavior is different from the other browsers in some cases.
Therefore, we cannot port this to WPT.

Depends on D158104

If splitting element is a list item element,
HandleInsertParagraphInListItemElement may just move it into a list element
if it's followed by the list element, or may create new paragraph element if
the list item element is empty and last list item element.

If splitting element is a heading element,
HandleInsertParagraphInHeadingElement may create new paragraph element if
splitting at end of the heading element and not followed by <br>.

Therefore, neither SplitNodeResult nor CreateElementResult is proper for
their result type, and the caller InsertParagraphSeparatorAsSubAction, just
wants to know the right handle element which should contain caret and a caret
point suggestion.

For solving these issues, this gives an alias to CreateElementResult, makes
them return the right paragraph even if it's not newly created nor split
element, and replaces blockElementToPutCaret with the returned element.

Depends on D158105

This means that the change has a risk of compatibility with IME, but from the
other point of view, we already behave differently from the other browsers
in native IME API handler level too.

Depends on D158106

Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/fdd1ba8cd07a
part 1: Make `HTMLEditor` join/split node direction switchable by a pref r=m_kato
https://hg.mozilla.org/integration/autoland/rev/fe6e3d412dd2
part 2-1: Make `HTMLEditor::DoJoinNodes()` adjust selection for both join nodes directions r=m_kato
https://hg.mozilla.org/integration/autoland/rev/0df250e3eccc
part 2-2: Make `HTMLEditor::DoJoinNodes()` handle both join nodes directions r=m_kato
https://hg.mozilla.org/integration/autoland/rev/de9f2339aeb2
part 3-1: Make `HTMLEditor::DoSplitNode()` adjust selection for both split node directions r=m_kato
https://hg.mozilla.org/integration/autoland/rev/953647a15a7d
part 3-2: Make `HTMLEditor::DoSplitNode()` handle both split node directions r=m_kato
https://hg.mozilla.org/integration/autoland/rev/9f24abab5fb2
part 4: Make `JoinNodesTransaction` and `SplitNodeTransaction` support both join/split node directions r=m_kato
https://hg.mozilla.org/integration/autoland/rev/8cc61e8fe7bc
part 5: Make `HTMLEditor::SplitRangeOffFromBlock` and its caller refer `SplitNodeResult` at their post-processing r=m_kato
https://hg.mozilla.org/integration/autoland/rev/c1f56015b1d5
part 6: Make `HTMLEditor::InsertElementWithSplittingAncestorsWithTransaction` check edge case with next node of the split point r=m_kato
https://hg.mozilla.org/integration/autoland/rev/db9d682774cf
part 7: Make `editor/libeditor/test_bug449243.html` be aware of both split node directions r=m_kato
https://hg.mozilla.org/integration/autoland/rev/acb7315965e5
part 8: Make `HTMLEditor::InsertParagraphSeparatorAsSubAction` put caret into right paragraph r=m_kato
https://hg.mozilla.org/integration/autoland/rev/6f315dbf947e
part 9: Make `widget/tests/test_composition_text_querycontent.xhtml` be aware of both split/join node directions r=m_kato
https://hg.mozilla.org/integration/autoland/rev/f4c48015c119
part 10: Make `editor/libeditor/tests/test_inline_style_cache.html` be aware of both split node directions r=m_kato
Regressions: 1793873
Regressions: 1808544
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: