Bug 189379 – AX: The title of an input contained in a label should participate in the accessible name calculation

• Component: Accessibility
• Status: RESOLVED INVALID

[Joanmarie Diggs]

Given this test case:

  <label for="test">foo <input id="test" type="button" name="test" title="bar"> baz</label>

The accessible name of the input should be "foo bar baz". Currently it is "foo baz" because we are not falling back on the title attribute of inputs which lack displayed text.

[Radar WebKit Bug Importer]

<rdar://problem/44200580>

[Joanmarie Diggs]

Created attachment 349085 [details]
Patch

[EWS Watchlist]

Comment on attachment 349085 [details]
Patch

Attachment 349085 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/9120037

New failing tests:
accessibility/label-with-input-with-title.html

[EWS Watchlist]

Created attachment 349096 [details]
Archive of layout-test-results from ews204 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews204  Port: win-future  Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit

[Joanmarie Diggs]

Created attachment 349098 [details]
Patch

[Nan Wang]

Comment on attachment 349098 [details]
Patch

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

> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1788
> +        if (childText.isEmpty() && is<HTMLInputElement>(child->node()))

Do we want to consider elements other than inputs?

> LayoutTests/platform/mac/accessibility/label-with-input-with-title-expected.txt:9
> +	AXTitleUIElement: non-null

Can you try to see if the name calculation is included in the stringValue() on mac platform?

[EWS Watchlist]

Comment on attachment 349098 [details]
Patch

Attachment 349098 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/9121458

New failing tests:
accessibility/label-with-input-with-title.html

[EWS Watchlist]

Created attachment 349106 [details]
Archive of layout-test-results from ews201 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews201  Port: win-future  Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit

[EWS Watchlist]

Comment on attachment 349098 [details]
Patch

Attachment 349098 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/9122159

New failing tests:
accessibility/label-with-input-with-title.html

[EWS Watchlist]

Created attachment 349108 [details]
Archive of layout-test-results from ews200 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews200  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Joanmarie Diggs]

(In reply to Nan Wang from comment #6)
> Comment on attachment 349098 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=349098&action=review
> 
> > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1788
> > +        if (childText.isEmpty() && is<HTMLInputElement>(child->node()))
> 
> Do we want to consider elements other than inputs?

Maybe.

What I'm doing at the moment is debugging and fixing AccName spec test failures so that AccName will be able to successfully exit CR and become a proposed recommendation. As I continue doing so, I may stumble across other instances where we're failing to include the title attribute of non-input elements. In which case the answer to your question will become "yes" and I'll submit a patch for that.

If you have other elements you'd like me to consider, I'm happy to do so -- though we should probably ensure that jibes with what's in AccName and HTML-AAM and file issues against those specs if it doesn't.
 
> > LayoutTests/platform/mac/accessibility/label-with-input-with-title-expected.txt:9
> > +	AXTitleUIElement: non-null
> 
> Can you try to see if the name calculation is included in the stringValue()
> on mac platform?

I'm not seeing it. Speaking of which: It was my assumption that in this instance, VoiceOver would build up the label to be presented from the descendants of the AXTitleUIElement, but VoiceOver is not presenting the "bar" when each widget in the test case gets focus. So.... Where should this be exposed so that VoiceOver says "foo bar baz" when presenting the label of the focused widget?

[Joanmarie Diggs]

Created attachment 349164 [details]
trying to make win EWS happy

[EWS Watchlist]

Comment on attachment 349164 [details]
trying to make win EWS happy

Attachment 349164 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/9131414

New failing tests:
accessibility/label-with-input-with-title.html

[EWS Watchlist]

Created attachment 349181 [details]
Archive of layout-test-results from ews202 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews202  Port: win-future  Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit

[Joanmarie Diggs]

Created attachment 349192 [details]
trying to make win EWS happy

[Joanmarie Diggs]

Comment on attachment 349192 [details]
trying to make win EWS happy

Hey Chris.

If my answer to my earlier question is easy, then I'll do a new patch here to fix both of our platforms. Otherwise, please review.

For convenience, this is the question I'm wondering about:

> It was my assumption that in this instance, VoiceOver would build
> up the label to be presented from the descendants of the AXTitleUIElement,
> but VoiceOver is not presenting the "bar" when each widget in the test case
> gets focus. So.... Where should this be exposed so that VoiceOver says
> "foo bar baz" when presenting the label of the focused widget?

Thanks!

[chris fleizach]

(In reply to Joanmarie Diggs (irc: joanie) from comment #16)
> Comment on attachment 349192 [details]
> trying to make win EWS happy
> 
> Hey Chris.
> 
> If my answer to my earlier question is easy, then I'll do a new patch here
> to fix both of our platforms. Otherwise, please review.
> 
> For convenience, this is the question I'm wondering about:
> 
> > It was my assumption that in this instance, VoiceOver would build
> > up the label to be presented from the descendants of the AXTitleUIElement,
> > but VoiceOver is not presenting the "bar" when each widget in the test case
> > gets focus. So.... Where should this be exposed so that VoiceOver says
> > "foo bar baz" when presenting the label of the focused widget?

Yea I think in this case the title UI element is exposed and VO calculates the name based on all the elements. So I guess this is correct, but I admit it's a bit of a weird discrepancy.


> 
> Thanks!

[Nan Wang]

Comment on attachment 349098 [details]
Patch

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

>>> LayoutTests/platform/mac/accessibility/label-with-input-with-title-expected.txt:9
>>> +	AXTitleUIElement: non-null
>> 
>> Can you try to see if the name calculation is included in the stringValue() on mac platform?
> 
> I'm not seeing it. Speaking of which: It was my assumption that in this instance, VoiceOver would build up the label to be presented from the descendants of the AXTitleUIElement, but VoiceOver is not presenting the "bar" when each widget in the test case gets focus. So.... Where should this be exposed so that VoiceOver says "foo bar baz" when presenting the label of the focused widget?

I think there's a containsOnlyStaticText() in AccessibilityLabel. Maybe we should add this case in that check?

[Nan Wang]

Comment on attachment 349098 [details]
Patch

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

>>>> LayoutTests/platform/mac/accessibility/label-with-input-with-title-expected.txt:9
>>>> +	AXTitleUIElement: non-null
>>> 
>>> Can you try to see if the name calculation is included in the stringValue() on mac platform?
>> 
>> I'm not seeing it. Speaking of which: It was my assumption that in this instance, VoiceOver would build up the label to be presented from the descendants of the AXTitleUIElement, but VoiceOver is not presenting the "bar" when each widget in the test case gets focus. So.... Where should this be exposed so that VoiceOver says "foo bar baz" when presenting the label of the focused widget?
> 
> I think there's a containsOnlyStaticText() in AccessibilityLabel. Maybe we should add this case in that check?

Anyway I think you are right that things are exposed in AXTitleUIElement

[Joanmarie Diggs]

Hey Chris and Nan.

Thanks for the info! I looked into it some more today. Responses inline below.

(In reply to chris fleizach from comment #17)
 
> Yea I think in this case the title UI element is exposed

Correct.

> and VO calculates the name based on all the elements.

Ok, but VO is not fully succeeding. The descendants of the AXTitleUIElement (an AccessibilityLabel) are:

* "foo" (text)
* "bar" (the title attribute value of an input with type button or checkbox)
* "baz" (text)

VO's calculation is "foo baz".

CANDIDATE SOLUTION 1: In this particular case, "bar" is exposed as the AXHelp value of the input. So VO *could* present it without our making any further changes in WebKit, if you change VO to fall back on AXHelp.

(In reply to Nan Wang from comment #18)
 
> I think there's a containsOnlyStaticText() in AccessibilityLabel. Maybe we
> should add this case in that check?

Do you mean modify containsOnlyStaticText()? If so, I don't think that's the right thing to do. In the test cases in question, the AccessibilityLabel does NOT contain only static text. It contains widgets. As a result containsOnlyStaticText() returns false -- I would argue that is the correct return value and that this case is already in that check.

Or were you instead suggesting we add a call to containsOnlyStaticText() in AccessibilityRenderObject::exposesTitleUIElement()? i.e. if the AccessibilityLabel does NOT contain only static text, it doesn't expose a title UI element? If so, that works for me. We already treat labels which contain unrelated controls as not being a valid title UI element.

CANDIDATE SOLUTION 2: Complicated labels should not be treated a valid title UI element; instead their calculated value should be exposed via AXTitle.

As an experiment, I tried seeing what would happen if the calculated result included an AXTitle value but the AXTitleUIElement continued to be exposed. The answer, which I'm guessing you already knew, is that VO presents both. I'm also guessing that it is by design and thus modifying VO to ignore AXTitleUIElement when there is an AXTitle is not a solution. But if I'm wrong about that, please let me know.

So.... Do you want the fix in VO or in WebKit. If the latter, are you cool with CANDIDATE SOLUTION 2?

[chris fleizach]

(In reply to Joanmarie Diggs (irc: joanie) from comment #20)
> Hey Chris and Nan.
> 
> Thanks for the info! I looked into it some more today. Responses inline
> below.
> 
> (In reply to chris fleizach from comment #17)
>  
> > Yea I think in this case the title UI element is exposed
> 
> Correct.
> 
> > and VO calculates the name based on all the elements.
> 
> Ok, but VO is not fully succeeding. The descendants of the AXTitleUIElement
> (an AccessibilityLabel) are:
> 
> * "foo" (text)
> * "bar" (the title attribute value of an input with type button or checkbox)
> * "baz" (text)
> 
> VO's calculation is "foo baz".
> 
> CANDIDATE SOLUTION 1: In this particular case, "bar" is exposed as the
> AXHelp value of the input. So VO *could* present it without our making any
> further changes in WebKit, if you change VO to fall back on AXHelp.
> 

Should title be exposed as AXHelp here? it seems that if this is the name calculation algorithm, title is no longer considered "help" text but part of the label...

> (In reply to Nan Wang from comment #18)
>  
> > I think there's a containsOnlyStaticText() in AccessibilityLabel. Maybe we
> > should add this case in that check?
> 
> Do you mean modify containsOnlyStaticText()? If so, I don't think that's the
> right thing to do. In the test cases in question, the AccessibilityLabel
> does NOT contain only static text. It contains widgets. As a result
> containsOnlyStaticText() returns false -- I would argue that is the correct
> return value and that this case is already in that check.
> 
> Or were you instead suggesting we add a call to containsOnlyStaticText() in
> AccessibilityRenderObject::exposesTitleUIElement()? i.e. if the
> AccessibilityLabel does NOT contain only static text, it doesn't expose a
> title UI element? If so, that works for me. We already treat labels which
> contain unrelated controls as not being a valid title UI element.
> 
> CANDIDATE SOLUTION 2: Complicated labels should not be treated a valid title
> UI element; instead their calculated value should be exposed via AXTitle.
> 
> As an experiment, I tried seeing what would happen if the calculated result
> included an AXTitle value but the AXTitleUIElement continued to be exposed.
> The answer, which I'm guessing you already knew, is that VO presents both.
> I'm also guessing that it is by design and thus modifying VO to ignore
> AXTitleUIElement when there is an AXTitle is not a solution. But if I'm
> wrong about that, please let me know.
> 
> So.... Do you want the fix in VO or in WebKit. If the latter, are you cool
> with CANDIDATE SOLUTION 2?

[Joanmarie Diggs]

(In reply to chris fleizach from comment #21)

> Should title be exposed as AXHelp here? it seems that if this is the name
> calculation algorithm, title is no longer considered "help" text but part of
> the label...

Good question. I just asked it: https://github.com/w3c/html-aam/issues/148

Executive summary is that if you read the HTML AAM literally, I believe it should indeed be exposed as AXHelp here. Whether or not that's what is correct or intended is another matter.

[Joanmarie Diggs]

On a different note, I gave CANDIDATE SOLUTION 2 a try. In macOS it fixes (in my mind) the test case in this bug, namely AXTitle is "foo bar baz" and AXTitleUIElement is null. But it does break 5 tests, all of which expect an AXTitleUIElement which no longer exists:

* double-title.html
* label-elements-exposed-as-title-ui-elements.html
* radio-button-title-label.html
* secure-textfield-title-ui.html
* title-ui-element-correctness.html

You happen to have an opinion on if this solution is correct and we just need to update the tests, or if this is the wrong approach?

[chris fleizach]

(In reply to Joanmarie Diggs (irc: joanie) from comment #23)
> On a different note, I gave CANDIDATE SOLUTION 2 a try. In macOS it fixes
> (in my mind) the test case in this bug, namely AXTitle is "foo bar baz" and
> AXTitleUIElement is null. But it does break 5 tests, all of which expect an
> AXTitleUIElement which no longer exists:
> 
> * double-title.html
> * label-elements-exposed-as-title-ui-elements.html
> * radio-button-title-label.html
> * secure-textfield-title-ui.html
> * title-ui-element-correctness.html
> 
> You happen to have an opinion on if this solution is correct and we just
> need to update the tests, or if this is the wrong approach?

Let me ask around here

[Joanmarie Diggs]

I'm going to close this as invalid for now. The ARIA Working Group's test expectations state that the "foo bar baz" is the right result. And based on AccName step 2E, it is. But AccName step 2D seems to preempt that calculation from occurring.

Sorry for the noise!

[chris fleizach]

(In reply to chris fleizach from comment #24)
> (In reply to Joanmarie Diggs (irc: joanie) from comment #23)
> > On a different note, I gave CANDIDATE SOLUTION 2 a try. In macOS it fixes
> > (in my mind) the test case in this bug, namely AXTitle is "foo bar baz" and
> > AXTitleUIElement is null. But it does break 5 tests, all of which expect an
> > AXTitleUIElement which no longer exists:
> > 
> > * double-title.html
> > * label-elements-exposed-as-title-ui-elements.html
> > * radio-button-title-label.html
> > * secure-textfield-title-ui.html
> > * title-ui-element-correctness.html
> > 
> > You happen to have an opinion on if this solution is correct and we just
> > need to update the tests, or if this is the wrong approach?
> 
> Let me ask around here

It seems like it might be ok to collapse title elements into one label on the Mac
The next time we have this issue might be worth checking out