1427825 - Make accessibility use the flattened tree a bit more consistently.

Status: RESOLVED FIXED

(Core :: Disability Access APIs, enhancement)

Description

[Emilio Cobos Álvarez [:emilio]]

I believe this is what is causing the assertion failure in bug 1398981, though I haven't managed to repro it locally.

Comment 1

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1427825: Make accessibility use the flattened tree more consistently.

Review commit: https://reviewboard.mozilla.org/r/209920/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/209920/

Comment 2

[alexander :surkov (:asurkov)]

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.

Comment 3

[Emilio Cobos Álvarez [:emilio]]

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

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b0a210594814
Make accessibility use the flattened tree more consistently. r=surkov

Comment 5

[Noemi Erli[:noemi_erli]]

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