AX: Consider TitleUIElementsAttribute (plural, not TitleUIElementAttribute) to support multiple IDREFs in aria-labelledby

[Alice Boxhall]

Bug 61995 has some discussion on the best way to expose aria-labelledby, with one suggestion being that where possible, it should be exposed via TitleUIElement.

I've hacked together a (very hacky) change to experiment with this.

[Alice Boxhall]

Created attachment 96685 [details]
Patch

[chris fleizach]

The problem with this is that TitleUIElementAttribute expects only one element, whereas aria-labelledby often returns many elements.

We really need a TitleUIElementsAttribute before we could make this change

[Alice Boxhall]

This is obviously just exploratory code; I wanted to see what would happen if we returned _something_ for TitleUIElement.

I'm not sure how frequent the case is where aria-labelledby returns many elements, but I thought it might be worth considering returning the sole value as a TitleUIElement in the case where there was only one.

However, I notice this still doesn't convince VoiceOver to make the parent DIV ("banana" in the test on bug 61995) show up in the accessibility hierarchy. Chris, do you know whether this is a bug? It seems that VoiceOver (or the Mac Accessibility framework?) will only place something in the accessibility hierarchy if it has a Description property.

[chris fleizach]

(In reply to comment #3)
> This is obviously just exploratory code; I wanted to see what would happen if we returned _something_ for TitleUIElement.
> 
> I'm not sure how frequent the case is where aria-labelledby returns many elements, but I thought it might be worth considering returning the sole value as a TitleUIElement in the case where there was only one.
> 
> However, I notice this still doesn't convince VoiceOver to make the parent DIV ("banana" in the test on bug 61995) show up in the accessibility hierarchy. Chris, do you know whether this is a bug? It seems that VoiceOver (or the Mac Accessibility framework?) will only place something in the accessibility hierarchy if it has a Description property.

The element that is being returned still needs to NOT be ignored. I suspect in this case the element is ignored, which is why it won't show up in the tree. you could assign a role="group" to the element to make it show up i suppose

[Alice Boxhall]

(In reply to comment #4)
> > However, I notice this still doesn't convince VoiceOver to make the parent DIV ("banana" in the test on bug 61995) show up in the accessibility hierarchy. Chris, do you know whether this is a bug? It seems that VoiceOver (or the Mac Accessibility framework?) will only place something in the accessibility hierarchy if it has a Description property.
> 
> The element that is being returned still needs to NOT be ignored. I suspect in this case the element is ignored, which is why it won't show up in the tree. you could assign a role="group" to the element to make it show up i suppose

Well, in the case on bug 61995, the DIV has a role of "article" and I have debugged through and checked that accessibilityIsIgnored is definitely returning FALSE. Note that in that case, also, there is no difference between the two DIVs other than one has an aria-label property and one has an aria-labelledby property. As aria-label is exposed as a Description, this DIV shows up in the accessibility hierarchy, but since aria-labelledby is (currently) exposed as a Title, that element does not show up.

[Eric Seidel (no email)]

Comment on attachment 96685 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=96685&action=review

This patch is clearly not ready for review.

> Source/WebCore/ChangeLog:8
> +        No new tests. (OOPS!)

This will cause the CQ to fail.  You should either replace this with a list of affected tests, or explain why testing is impossible/impractical.

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1356
> +//    const AtomicString& ariaLabelledBy = ariaLabeledByAttribute();
> +//    if (!ariaLabelledBy.isEmpty())
> +//        return ariaLabelledBy;

We don't commit commented out code.

> Source/WebCore/accessibility/mac/AccessibilityObjectWrapper.mm:1032
> +    else if (m_object->isWebArea()) {
>          objectAttributes = webAreaAttrs;
> +    }

Style?

[Alice Boxhall]

Sorry, I didn't intend to submit this for review. As explained earlier on the bug, this is just exploratory code.

Thanks for taking the time to look at it, though.

(In reply to comment #6)
> (From update of attachment 96685 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=96685&action=review
> 
> This patch is clearly not ready for review.
> 
> > Source/WebCore/ChangeLog:8
> > +        No new tests. (OOPS!)
> 
> This will cause the CQ to fail.  You should either replace this with a list of affected tests, or explain why testing is impossible/impractical.
> 
> > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1356
> > +//    const AtomicString& ariaLabelledBy = ariaLabeledByAttribute();
> > +//    if (!ariaLabelledBy.isEmpty())
> > +//        return ariaLabelledBy;
> 
> We don't commit commented out code.
> 
> > Source/WebCore/accessibility/mac/AccessibilityObjectWrapper.mm:1032
> > +    else if (m_object->isWebArea()) {
> >          objectAttributes = webAreaAttrs;
> > +    }
> 
> Style?

[James Craig]

<rdar://problem/15497749>

[Radar WebKit Bug Importer]

<rdar://problem/15710260>