1572317 - Relocated elements not pruned when container is hidden

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect, P1)

Description

[James Teh [:Jamie]]

Discovered this while trying to get aria-owns working properly on the address bar (bug 1567377).

STR:

1. Open this test case:
   data:text/html,<div role="combobox" aria-owns="listbox"></div><div id="container"><div id="listbox" role="listbox"></div></div><button onClick="container.hidden = true; listbox.innerHTML = `<div role='option'>1</div>`;">Break things</button><button onClick="container.hidden = false;">Break more things</button>
2. Observe that in the accessibility tree, there is an editcombobox containing a combobox list. The combobox list does not have the invisible state.
3. Press the "Break things" button.
4. In the accessibility tree, examine the children of the editcombobox.
   • Expected: There should be no children.
   • Actual: The combobox list is still there. It has the invisible state.
5. Press the "Break more things" button.
6. In the accessibility tree, examine the children of the editcombobox.
   • Observe: As expected, the combobox list is there and it does not have the invisible state.
7. In the accessibility tree, examine the children of the combobox list.
   • Expected: There should be a combobox option child.
   • Actual: There is a text leaf child.

This is not fixed by the patch for bug 1571616 (I applied and tested locally).

In PruneOrInsertSubtree, when we remove something (e.g. because its accessible no longer has a frame), we call ContentRemoved(acc) and return early. That removes the accessible and its descendant accessibles. However, relocated accessibles aren't descendant accessibles, even though they're descendant DOM nodes. I think we need to remove Accessibles for descendant DOM nodes here.

Can we just call ContentRemoved on the node instead of the Accessible? That will iterate through descendant DOM nodes.

Comment 1

[Dão Gottwald [:dao]]

Note that in html, node.hidden = whatever doesn't do anything.

Comment 2

[James Teh [:Jamie]]

It seems to, and MDN agrees:
https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/hidden

Comment 3

[Dão Gottwald [:dao]]

Oh, hmm, even after reading that I'm not sure what it does. In XUL (this includes the urlbar) hidden is just a shortcut for style.display="none".

Comment 4

[James Teh [:Jamie]]

Attachment: Bug 1572317: When removing an Accessible because it lost its frame, remove Accessibles for DOM descendants as well.

Removing an Accessible removes descendants in the a11y tree.
However, there may be DOM descendants which have been relocated elsewhere in the a11y tree.
Their DOM nodes are now hidden as well, so we need to remove those Accessibles too.
In addition to Accessibles remaining in the tree when they shouldn't, failing to remove relocated Accessibles caused problems later on when a relocated Accessible was shown with new descendants.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/09dfba0277d1
When removing an Accessible because it lost its frame, remove Accessibles for DOM descendants as well. r=eeejay

Comment 6

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/09dfba0277d1