Closed Bug 1939065 Opened 1 month ago Closed 21 days ago

Optimize DOM mutation caused by `HTMLEditor::DoSplitNode` and `HTMLEditor::DoJoinNodes`

Categories

(Core :: DOM: Editor, task)

task

Tracking

()

RESOLVED FIXED
136 Branch
Tracking Status
firefox136 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

Attachments

(1 file)

HTMLEditor::DoSplitNode does:

  1. Insert a new right node
  2. Move content in left node into the right node

HTMLEditor::DoJoinNodes does:

  1. Move content of the right node to the left node
  2. Delete the left node

So, all things are done when connected. Therefore IMEContentObserver uses its cache and it's harder to debug. Additionally, temporarily the touching range becomes wider then the result at joining nodes. That could cause IME gets received longer text change range. Therefore, they should do:

HTMLEditor::DoSplitNode:

  1. Move content in left node to the orphan new right node (then, only removing content is handled by IMEContentObserver)
  2. Insert the right node (then, IMEContentObserver can compute the offset and length only once)

HTMLEditor::DoJoinNodes:

  1. Delete the right node (then, all moving content is treated as removed by IMEContentObserver, i.e., it needs to compute offset/length once)
  2. Move the non-connected right node children into the left node (then, IMEContentObserver can cache all moving nodes and compute offset/length once)

HTMLEditor::DoSplitNode does:

  1. Insert a new right node
  2. Move content in left node into the right node

HTMLEditor::DoJoinNodes does:

  1. Move content of the right node to the left node
  2. Delete the left node

So, all things are done when connected. Therefore IMEContentObserver uses its
cache and it's harder to debug. Additionally, temporarily the touching range
becomes wider then the result at joining nodes. That could cause IME gets
received longer text change range. Therefore, they should do:

HTMLEditor::DoSplitNode:

  1. Move content in left node to the orphan new right node (then, only removing
    content is handled by IMEContentObserver)
  2. Insert the right node (then, IMEContentObserver can compute the offset and
    length only once)

HTMLEditor::DoJoinNodes:

  1. Delete the right node (then, all moving content is treated as removed by
    IMEContentObserver, i.e., it needs to compute offset/length once)
  2. Move the non-connected right node children into the left node (then,
    IMEContentObserver can cache all moving nodes and compute offset/length once)
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/ef3aed257e91 Optimize `HTMLEditor::DoSplitNode` and `HTMLEditor::DoJoinNodes` for `IMEContentObserver` r=m_kato

Backed out for causing mochitests failures in test_composition_text_querycontent.xhtml.

  • Backout link
  • Push with failures
  • Failure Log
  • Failure line: TEST-UNEXPECTED-FAIL | widget/tests/test_composition_text_querycontent.xhtml | runIMEContentObserverTest with contenteditable (defaultParagraphSeparator is br): deleting child nodes (partially #2) with pressing Delete key should cause text change notification whose offset is 1 - got 2, expected 1
Flags: needinfo?(masayuki)

Hmm, odd. I didn't see the failure when I was working on this bug...

Flags: needinfo?(masayuki)

The patch also fails in test_text_change_notifications_when_inserting_text_containing_line_breaks.html which will be updated in bug 1940653 which needs to be fixed in the beta channel. Therefore, I'll update the patch again after landing the patch for bug 1940653.

Depends on: 1940653
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/cae85f5a2d1c Optimize `HTMLEditor::DoSplitNode` and `HTMLEditor::DoJoinNodes` for `IMEContentObserver` r=m_kato
Status: ASSIGNED → RESOLVED
Closed: 21 days ago
Resolution: --- → FIXED
Target Milestone: --- → 136 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: