1972539 - crash [@ mozilla::a11y::TextRange::CommonParent ]

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Magnus Melin [:mkmelin]]

bp-740a6384-3e5d-423e-bcc0-d85940250612
bp-34a7efd3-ca7a-4f9c-8c43-92caf0250612

p1 is null - https://searchfox.org/mozilla-central/rev/fdb34ddfe30bd54aba991feb72b1476c77938e46/accessible/base/TextRange.cpp#358

Comment 1

[Morgan Reschenberg [:morgan]]

FYI seems like the two crash reports above are thunderbird specific, and windows as well

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1957314

:Jamie, since you are the author of the regressor, bug 1957314, could you take a look?

For more information, please visit BugBot documentation.

Comment 3

[James Teh [:Jamie] (away returning 14 Apr)]

Chill, bot. I'm on it. :)

Comment 4

[James Teh [:Jamie] (away returning 14 Apr)]

Attachment: Bug 1972539 part 1: Remove HyperTextAccessible::SelectionCount and SelectionBoundsAt.

These are implemented in HyperTextAccessibleBase.
Having local and remote use the same implementation makes things simpler, more consistent, easier to test and easier to reason about.
This is also necessary in order to properly test the fix in the next patch.

This breaks the rare edge case of selecting only empty containers for LocalAccessible.
This never worked for RemoteAccessible anyway, which is the majority of content now.
Bug 1973154 has been filed for this.

Comment 5

[James Teh [:Jamie] (away returning 14 Apr)]

Attachment: Bug 1972539: Fix handling of empty containers in TextRange::Crop.

It is possible for a DOM selection to start or end within an empty container.
In this case, we have offset 0 in an empty HyperTextAccessible.
This caused two problems.

First, DocAccessibleParent::SelectionRanges previously treated a range as invalid (and thus skipped it) if the offset was >= the character count in the container.
While this usually makes sense, it is problematic for empty containers.
To fix this, always treat offset 0 as valid.

Second, GetChildAtOffset returns null when called on an empty container with offset 0, since there's no child for it to return.
Previously, TextRange::Crop wasn't handling this, resulting in a crash.
To fix this, just use the start/end container itself if GetChildAtOffset returns null.

Comment 6

[James Teh [:Jamie] (away returning 14 Apr)]

https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=138241

Comment 7

[Phabricator Automation]

Comment on attachment 9495911 [details]
Bug 1972539 part 1: Remove HyperTextAccessible::SelectionCount and SelectionBoundsAt.

Revision D254466 was moved to bug 1946645. Setting attachment 9495911 [details] to obsolete.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/539086292504
https://hg.mozilla.org/integration/autoland/rev/db26641de452
Fix handling of empty containers in TextRange::Crop. r=morgan

Comment 9

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/db26641de452

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:Jamie, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox141 to wontfix.

For more information, please visit BugBot documentation.