Closed Bug 1637856 Opened 5 years ago Closed 4 years ago

Make `WSRunScanner` stop storing editable text nodes at initialization, instead, make it look for text node when it's necessary

Categories

(Core :: DOM: Editor, task)

task

Tracking

()

RESOLVED FIXED
mozilla78
Tracking Status
firefox78 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

Attachments

(2 files)

WSRunScanner stores editable text nodes with WSRunScanner::mNodeArray first. However, some of them may not be accessed, and they need to be compared with given point to find a previous/next character from a DOM point whose container is not a text node but this is too expensive and makes the class complicated.

Instead, let's make it scans previous/next text nodes when it's requested. Then, WSRunScanner can treat wider range in the future without additional runtime cost.

Severity: -- → S3

WSRunObject::DeleteRange() removes only text nodes which are stored when
WSRunObject is created. Although it removes text nodes if it's removed,
this patch does not take care about it in the new method. The reason is
the following patch will remove mNodeArray and anyway DOM tree modifiers
can check whether they are in proper position before access if it's needed.

WSRunScanner scans around given point in GetWSNodes() at construction with
using HTMLEditUtils methods and caches editable text nodes between
mStartReasonContent and mEndReasonContent. However, it's used only by
GetNextCharPoint() and GetPreviousCharPoint(), and they shouldn't be
referred after changing the DOM tree. Therefore, we can scan it directly
only when it needs to scan.

The patch rewrites GetNextCharPoint() and GetPreviousCharPoint() without
mNodeArray and removes mNodeArray from its member. This may increase the
cost of scanning next/previous text node, but improves the scan performance
when it does not treat so wide range and they are called with a point whose
container is not a text node.

This patch unexpectedly changes the behavior of them, that causes the fix of
2 failures in insertlinebreak.html and insertparagraph.html. According to
debugger, previously GetNextCharPoint()inScanNextVisibleNodeOrBlockBoundaryFrom()called point at<br>element returned no next char, then,ScanNextVisibleNodeOrBlockBoundaryFrom()returned end point which is block boundary of<listing>element (it is legacy HTML2 element and treated as<pre>element internally). Therefore, the inserted<br>element was misunderstood as invisible<br>at end of a block and inserted another<br>` element for making it visible. However, the redesigned
one fixed this bug with searching correct text node. Therefore, I cannot
keep the buggy behavior in the new designed methods.

Depends on D75470

Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/3ead0007c8fb part 1: Move `WSRunObject::DeleteRange()` to `HTMLEditor` r=m_kato https://hg.mozilla.org/integration/autoland/rev/ac9f8166fa65 part 2: Get rid of `WSRunScanner::mNodeArray` r=m_kato
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla78
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: