Make accessibility use the flattened tree a bit more consistently.

RESOLVED FIXED in Firefox 59

Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: emilio, Assigned: emilio)

Tracking

unspecified
mozilla59
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox59 fixed)

Details

Attachments

(1 attachment)

I believe this is what is causing the assertion failure in bug 1398981, though I haven't managed to repro it locally.
Comment hidden (mozreview-request)

Comment 2

2 years ago
mozreview-review
Comment on attachment 8939612 [details]
Bug 1427825: Make accessibility use the flattened tree more consistently.

https://reviewboard.mozilla.org/r/209920/#review215506

r=me, looks like the very right thing, thank you for fixing it!

::: accessible/generic/DocAccessible.cpp:1256
(Diff revision 1)
>    if (!aNode || !aNode->GetComposedDoc())
>      return nullptr;
>  
> -  nsINode* currNode = aNode;
> -  Accessible* accessible = nullptr;
> -  while (!(accessible = GetAccessible(currNode))) {
> +  for (nsINode* currNode = aNode; currNode;
> +       currNode = currNode->GetFlattenedTreeParentNode()) {
> +    if (Accessible* accessible = GetAccessible(currNode)) {

shouldn't you enclose (Accessible* accessible = ) into double parentheses, since compilers don't usually like assignement inside if statement.
Attachment #8939612 - Flags: review?(surkov.alexander) → review+
Assignee

Comment 3

2 years ago
mozreview-review
Comment on attachment 8939612 [details]
Bug 1427825: Make accessibility use the flattened tree more consistently.

https://reviewboard.mozilla.org/r/209920/#review215580

::: accessible/generic/DocAccessible.cpp:1256
(Diff revision 1)
>    if (!aNode || !aNode->GetComposedDoc())
>      return nullptr;
>  
> -  nsINode* currNode = aNode;
> -  Accessible* accessible = nullptr;
> -  while (!(accessible = GetAccessible(currNode))) {
> +  for (nsINode* currNode = aNode; currNode;
> +       currNode = currNode->GetFlattenedTreeParentNode()) {
> +    if (Accessible* accessible = GetAccessible(currNode)) {

I'm pretty sure this pattern is fine if you include the type declaration in the `if`. We use it in a bunch of places and I don't know what would the compiler warn about.

Comment 4

2 years ago
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/b0a210594814
Make accessibility use the flattened tree more consistently. r=surkov

Comment 5

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b0a210594814
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.