Land the patches for bug 1735608
Categories
(Core :: DOM: Editor, task, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox107 | --- | fixed |
People
(Reporter: masayuki, Assigned: masayuki)
References
(Regressed 1 open bug)
Details
Attachments
(12 files)
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
Bug 1792387 - part 2-2: Make `HTMLEditor::DoJoinNodes()` handle both join nodes directions r=m_kato!
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
Bug 1792387 - part 3-2: Make `HTMLEditor::DoSplitNode()` handle both split node directions r=m_kato!
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details |
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.
Assignee | ||
Comment 1•2 years ago
|
||
Depends on D157575
Assignee | ||
Comment 2•2 years ago
|
||
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
Assignee | ||
Comment 3•2 years ago
|
||
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
Assignee | ||
Comment 4•2 years ago
|
||
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
Assignee | ||
Comment 5•2 years ago
|
||
This does not change the existing path's behavior.
Depends on D158100
Assignee | ||
Comment 6•2 years ago
|
||
The reason why the changes of SplitNodeTransaction
is smaller than
JoinNodesTransaction
is, HTMLEditor::DoSplitNode
does almost all things
instead.
Depends on D158101
Assignee | ||
Comment 7•2 years ago
|
||
Depends on D158102
Assignee | ||
Comment 8•2 years ago
|
||
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
Assignee | ||
Comment 9•2 years ago
|
||
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
Assignee | ||
Comment 10•2 years ago
|
||
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
Assignee | ||
Comment 11•2 years ago
|
||
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
Assignee | ||
Comment 12•2 years ago
|
||
Depends on D158107
Comment 13•2 years ago
|
||
Comment 14•2 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/fdd1ba8cd07a
https://hg.mozilla.org/mozilla-central/rev/fe6e3d412dd2
https://hg.mozilla.org/mozilla-central/rev/0df250e3eccc
https://hg.mozilla.org/mozilla-central/rev/de9f2339aeb2
https://hg.mozilla.org/mozilla-central/rev/953647a15a7d
https://hg.mozilla.org/mozilla-central/rev/9f24abab5fb2
https://hg.mozilla.org/mozilla-central/rev/8cc61e8fe7bc
https://hg.mozilla.org/mozilla-central/rev/c1f56015b1d5
https://hg.mozilla.org/mozilla-central/rev/db9d682774cf
https://hg.mozilla.org/mozilla-central/rev/acb7315965e5
https://hg.mozilla.org/mozilla-central/rev/6f315dbf947e
https://hg.mozilla.org/mozilla-central/rev/f4c48015c119
Description
•