1740847 - Make methods in `WhiteSpaceVisibilityKeeper` track DOM points in smaller block as far as possible

Status: RESOLVED FIXED

(Core :: DOM: Editor, enhancement, P3)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I found some points in the class do not track cached DOM points/ranges for each DOM change. So they could cause deleting unexpected characters or inserting new character to unexpected position.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1740847 - part 1: Track split point at replacing collapsible white-spaces r=m_kato!

It pointToSplit should be tracked at replacing text, but I have no idea how
to test this because it replaces the text after the split point.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1740847 - part 2: Make `WhiteSpaceVisibilityKeeper::InsertBRElement()` track insert position in smaller block r=m_kato!

And also invisible white-space range and replacing white-space position should
also be tracked too.

Depends on D131036

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1740847 - part 3: Make `WhiteSpaceVisibilityKeeper::ReplaceText()` track insert position in smaller block r=m_kato!

Similar to the previous patch, this patch also make it track invisible
white-space ranges and clear outdated things.

And this makes it cache some information instead of tracking some changes
because of performance reason.

Depends on D131037

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/addb07ae8de0
part 1: Track split point at replacing collapsible white-spaces r=m_kato

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7f1d3fd39a93
part 2: Make `WhiteSpaceVisibilityKeeper::InsertBRElement()` track insert position in smaller block r=m_kato

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/17076719bbf2
part 3: Make `WhiteSpaceVisibilityKeeper::ReplaceText()` track insert position in smaller block r=m_kato

Comment 7

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/addb07ae8de0
https://hg.mozilla.org/mozilla-central/rev/7f1d3fd39a93
https://hg.mozilla.org/mozilla-central/rev/17076719bbf2

Comment 8

[Ryan VanderMeulen [:RyanVM]]

We've gotten some reports of this fixing other crashes in the wild. What do you think about nominating this for ESR uplift, Masayuki?

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #8)

We've gotten some reports of this fixing other crashes in the wild. What do you think about nominating this for ESR uplift, Masayuki?

Sure, but we need to port the part 3 patch.
https://treeherder.mozilla.org/jobs?repo=try&selectedTaskRun=Y5DR197hSfSc7EKSWflxKw.0&revision=1201346db134cacfae33874a53bb14c64dd3cb26

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1740847 - part 3: Make `WhiteSpaceVisibilityKeeper::ReplaceText()` track insert position in smaller block (for ESR 91) r=m_kato!

Due to the change of bug 1727844 and bug 1724650, only "part 3" cannot be
grafted cleanly.

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 9250476 [details]
Bug 1740847 - part 1: Track split point at replacing collapsible white-spaces r=m_kato!

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: These patches fix some random crashes in the wild.
• User impact if declined: Users will see crash in some editor apps.
• Fix Landed on Version: 96
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): The patches touch a lot of lines, but each line's change is simple. And the fix has already been tested by a lot of testers.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9250476 [details]
Bug 1740847 - part 1: Track split point at replacing collapsible white-spaces r=m_kato!

Approved for 91.5esr. Thanks for attaching the rebased patch.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/38ff66d6c922
https://hg.mozilla.org/releases/mozilla-esr91/rev/255cad36ba4d
https://hg.mozilla.org/releases/mozilla-esr91/rev/ed25aa724fba