1530931 - "We must reach document accessible implementing text interface!" assertion in SelectionManager::ProcessSelectionChanged when direct child of shadow root gets focus

Status: RESOLVED FIXED

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

Description

[James Teh [:Jamie]]

We hit the "We must reach document accessible implementing text interface!" assertion in SelectionManager::ProcessSelectionChanged whenever something in shadow DOM gets focused:
https://searchfox.org/mozilla-central/source/accessible/base/SelectionManager.cpp#188

STR:

1. Open this test case:
   data:text/html,<div id="host"></div><script>let shadow = host.attachShadow({mode: "open"}); shadow.innerHTML = '<button>test</button>';</script>
2. Tab to the button.

Comment 1

[James Teh [:Jamie]]

Still trying to work out what's going on here. So far, I know the following:

1. The call to SelectionManager::ProcessSelectionChanged is triggered by DOM focus.
2. We get the common ancestor for the range (cntrNode), which is a document fragment (nodeName #document-fragment). Presumably, this is the shadow DOM's fragment. It has no parent node.
3. We call nsAccUtils::GetTextContainer, which calls DocAccessible::GetAccessibleOrContainer.
4. That calls GetFlattenedTreeParent() on the node from step 2, which returns null.

Comment 2

[James Teh [:Jamie]]

Actually, this only happens if a direct child of the shadow root gets focus, since that's when the common ancestor will be the shadow root document fragment. I have a fix; taking.

Comment 3

[James Teh [:Jamie]]

Attachment: Bug 1530931: Correctly handle retrieving a container accessible for a shadow root.

This can happen, for example, when GetAccessibleOrContainer is called within SelectionManager::ProcessSelectionChanged due to focusing a direct child of a shadow root.
In this case, the common ancestor is the shadow root itself.
Previously, we returned null in this case because GetFlattenedTreeParent doesn't work on the shadow root itself.
Now, we check if the given node is the shadow root, and if so, we use the shadow host instead.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/faa43bdd3347
Correctly handle retrieving a container accessible for a shadow root. r=eeejay

Comment 5

[Narcis Beleuzu [:NarcisB]]

Backed out for crashtest failures on RootAccessible::RemoveEventListeners.

Backout link: https://hg.mozilla.org/integration/autoland/rev/a423f146621147ee1b6d3b18629e950148e529c0
Push link: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Crunnable&selectedJob=235118129&revision=54057e1121b5459ec697bf480a46676d8fc1bff9
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=235112648&repo=autoland&lineNumber=1612

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1b6bf9395372
Correctly handle retrieving a container accessible for a shadow root. r=eeejay

Comment 7

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/1b6bf9395372

Comment 8

[Peter Van der Beken [:peterv]]

https://hg.mozilla.org/projects/ash/rev/1b6bf93953727decb8481cb58937ec3eefa5821a
Bug 1530931: Correctly handle retrieving a container accessible for a shadow root. r=eeejay