1026125 - ShadowRoot nodes don't get accessible objects

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Eitan Isaacson [:eeejay]]

This seems to be a regression by the changes introduced in bug 992521.

Comment 1

[Eitan Isaacson [:eeejay]]

Attachment: Check for current document accross shadow root when creating accessible.

I think this is all we need. I'm going to create a mochitest, and then submit for review.

Comment 2

[alexander :surkov (:asurkov)]

I'm curious why IsInDoc() doesn't work for them since it works for everything else (native anonymous content, XBL). Also curious if this one correlates to bug 997752.

Comment 3

[William Chen [:wchen]]

XBL actually does get set in doc, so IsInDoc() would return true for XBL anonymous content. It makes a little more sense for XBL to be marked in document because you can walk the parent chain of elements in anonymous content and end up in the document. Walking the parent chain of elements in the shadow DOM would terminate at the ShadowRoot.

Comment 4

[Eitan Isaacson [:eeejay]]

Attachment: Check for current document accross shadow root when creating accessible.

Comment 5

[alexander :surkov (:asurkov)]

(In reply to William Chen [:wchen] from comment #3)
> XBL actually does get set in doc, so IsInDoc() would return true for XBL
> anonymous content. It makes a little more sense for XBL to be marked in
> document because you can walk the parent chain of elements in anonymous
> content and end up in the document. Walking the parent chain of elements in
> the shadow DOM would terminate at the ShadowRoot.

nsINode::GetParent() doesn't work to cross from anonymous to explicit content but you can do GetFlattenParent() sure. Do you say that there's no way to find explicit parent (insertion point) having shadow DOM node? If no then probably shadow content is not much different from XBL content.

Anyway, can we have something nicer than GetCrossShadowCurrentDoc check to see whether the node is attached to the document?

Comment 6

[alexander :surkov (:asurkov)]

Comment on attachment 8441489 [details] [diff] [review]
Check for current document accross shadow root when creating accessible.

Review of attachment 8441489 [details] [diff] [review]:
-----------------------------------------------------------------

test looks good, I'm not happy though with GetCrossShadowCurrentDoc call as replacement to the check whether node is in document but somebody from content/dom team should help here.

Comment 7

[Trevor Saunders (:tbsaunde)]

> Anyway, can we have something nicer than GetCrossShadowCurrentDoc check to
> see whether the node is attached to the document?

other than being slightly slower in the common case of not shadow dom what's wrong with it?

Comment 8

[alexander :surkov (:asurkov)]

name, I would be happy having IsInDoc working here, at least it would be consistent with XBL and native anon content

Comment 9

[William Chen [:wchen]]

Attachment: Parent of anonymous content

(In reply to alexander :surkov from comment #5)
> nsINode::GetParent() doesn't work to cross from anonymous to explicit
> content but you can do GetFlattenParent() sure. Do you say that there's no
> way to find explicit parent (insertion point) having shadow DOM node? If no
> then probably shadow content is not much different from XBL content.
GetParent() will walk out of anonymous content, see the attached example. The problem with putting shadow content in document is that we have some code that expects that if IsInDoc() is true, we can walk the parent chain to reach the document node. This is true with anonymous content, but not true with nodes in the shadow DOM.
 
> Anyway, can we have something nicer than GetCrossShadowCurrentDoc check to
> see whether the node is attached to the document?
For now, that's probably the best we have, bug 1026047 will likely add a similar equivalent for IsInDoc().

Comment 10

[alexander :surkov (:asurkov)]

(In reply to William Chen [:wchen] from comment #9)

> GetParent() will walk out of anonymous content, see the attached example.

maybe DOM node has some trick for this, I referred to bug 653881.

> The problem with putting shadow content in document is that we have some
> code that expects that if IsInDoc() is true, we can walk the parent chain to
> reach the document node.

I see, that easy can be a problem for a11y as well since our code relies on working GetParent(). It'd be good to have tests to check tree mutations, in case if we have tree update problems
 
> > Anyway, can we have something nicer than GetCrossShadowCurrentDoc check to
> > see whether the node is attached to the document?
> For now, that's probably the best we have, bug 1026047 will likely add a
> similar equivalent for IsInDoc().

ok

Comment 11

[Eitan Isaacson [:eeejay]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/47d6ee269692

We could follow up with a change once we have a working IsInDoc equivalent for shadow dom.

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Bug 1026047 is the meta bug to sort out the whole IsInDoc/GetCurrentDoc handling.

Comment 13

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/47d6ee269692