AX: VoiceOver on macOS does not announce fieldset description from aria-describedby when focussing inputs

[Oliver Byford]

Created attachment 339403 [details]
HTML test case

TEST CASE:

Given the following HTML within a valid HTML5 document:

  <form>
    <fieldset aria-describedby="hint error">
      <legend>
        <h1>Have you changed your name?</h1>
      </legend>

      <span id="hint">This includes changing your last name or spelling your name differently.</span>
      <span id="error">Choose yes or no</span>

      <input name="changed_name" id="yes" type="radio" value="yes" >
      <label for="yes">Yes</label>

      <input name="changed_name" id="no" type="radio" value="no">
      <label for="no">No</label>

    </fieldset>
  </form>

EXPECTED BEHAVIOUR:

When focussing the first radio input ("Yes") in the fieldset, Voiceover should include the text content of any element(s) associated with the containing fieldset (using aria-describedby):

"Yes, radio button, 1 of 2, Have you changed your name? This includes changing your last name or spelling your name differently. Choose yes or no.

You are currently on a radio button, 1 of 2, inside of a group. To select this option press Control-Option-Space"


This is the behaviour currently exhibited by iOS 10/11, and would be consistent with other assistive technologies (as tested with JAWS 18, NVDA).


ACTUAL BEHAVIOUR:

When focussing the first radio input in a fieldset, Voiceover announces:

"Yes, radio button, 1 of 2, Have you changed your name?

You are currently on a radio button, 1 of 2, inside of a group. To select this option press Control-Option-Space"


As tested in Safari 11 and Safari Technology Preview 11.2 on macOS 10.13.4.

[Radar WebKit Bug Importer]

<rdar://problem/39939028>

[Nan Wang]

Created attachment 341621 [details]
patch

[chris fleizach]

Comment on attachment 341621 [details]
patch

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

> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1444
> +    else {

should this be an else if (isControl()

> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1445
> +        // For controls, use their fieldset parent's described-by text if avaialble.

available

> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1448
> +                return object.isFieldset() && !object.ariaDescribedByAttribute().isEmpty();

does it matter if we check whether ariaDescribedByAttribute is empty here? if we try to try to append an empty string nothing will happen anyway right? and then we'll save a call to ariaDescribedByAttribute?

[Nan Wang]

Comment on attachment 341621 [details]
patch

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

>> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1448
>> +                return object.isFieldset() && !object.ariaDescribedByAttribute().isEmpty();
> 
> does it matter if we check whether ariaDescribedByAttribute is empty here? if we try to try to append an empty string nothing will happen anyway right? and then we'll save a call to ariaDescribedByAttribute?

No I think we still need to check empty() in this case, since AccessibilityText doesn't check empty in construction and also baseAccessibilityHelpText doesn't check empty before return.

[Nan Wang]

Created attachment 341627 [details]
patch

updated from review

[EWS Watchlist]

Attachment 341627 [details] did not pass style-queue:


ERROR: Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1444:  Missing space before {  [whitespace/braces] [5]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Nan Wang]

Created attachment 341628 [details]
patch

update style

[WebKit Commit Bot]

Comment on attachment 341628 [details]
patch

Clearing flags on attachment: 341628

Committed r232331: <https://trac.webkit.org/changeset/232331>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.