ember.js icon indicating copy to clipboard operation
ember.js copied to clipboard

[Bug] Issues with Select rendering and JAWS

Open pzuraq opened this issue 5 years ago • 6 comments

🐞 Describe the Bug

We've gotten reports that the JAWS is unable to correctly read minimal select element created with Ember. Specifically, when an option is selected, the selected value is not read out by the screen reader. The workaround at the moment is to use aria-selected, but this seems problematic. Either JAWS has a bug, or Ember does, and we need to figure out which.

🔬 Minimal Reproduction

{{!-- This example works --}}
<select>
  <option>Alabama</option>
  <option selected>California</option>
  <option>Oregon</option>
</select>

{{!-- This example does not work --}}
<select>
  {{#each values as |value|}}
    <option selected={{eq value this.selectedValue}}></option>
  {{/each}}
</select>

Codepen with the first example: https://codepen.io/pzuraq/pen/oNxGvWz

😕 Actual Behavior

The screen reader does not read out the option label.

🤔 Expected Behavior

The screen reader should read out the option label.

🌍 Environment

  • Ember: Latest
  • Node.js/npm: -
  • OS: -
  • Browser: -

➕ Additional Context

We've verified that the Ember version of the select is properly updating the properties that represent the selected state, and other A11y tools including the A11y tree in Chrome work properly here. The next step is to start reducing the differences between these two examples to figure out what exactly is causing the issue.

Noted differences:

  1. selected is set the first time as an attribute on the native example. It remains set as the select is updated. By contrast, the Ember example always sets the property, so no option will have the attribute set at any time. We should see if removing the attribute in the native example causes any difference in behavior.
  2. The Ember example will create its DOM dynamically, using document.createElement and manually appending it, whereas the native version does not. We should try to create a select using vanilla JS to see if that impacts behavior at all.

pzuraq avatar Sep 01 '20 18:09 pzuraq

cc @syu15

pzuraq avatar Sep 01 '20 18:09 pzuraq

I think next steps here would be to do manual DOM element building for this snippet:

<select>
  <option>Alabama</option>
  <option selected>California</option>
  <option>Oregon</option>
</select>

Something like:

const selectEl = document.createElement('select');

const option1 = document.createElement('option');
option1.appendChild(document.createTextNode('Alabama'));

const option2 = document.createElement('option');
option2.appendChild(document.createTextNode('California'));
option2.selected = true;

const option3 = document.createElement('option');
option3.appendChild(document.createTextNode('Oregon'));

selectEl.appendChild(option1);
selectEl.appendChild(option2);
selectEl.appendChild(option3);
document.body.appendChild(selectEl);

And then test JAWS/NVDA to see if they read the correct output. I've made https://codepen.io/rwjblue/pen/NWNaqWy for this snippet in case that makes it easier to test.

rwjblue avatar Sep 01 '20 21:09 rwjblue

I was also able to reproduce the issue in Firefox and NVDA.

MelSumner avatar Sep 03 '20 22:09 MelSumner

So I dug into this a bit more, and was unable to reproduce the issue in Chrome with NVDA, but was able to with Firefox. The issue appears to be coming from the fact that we append multiple text nodes to the <option> element. The screen reader only reads the first text node in these cases.

Plain JS reproduction: https://codepen.io/pzuraq/pen/PoNLmor

Ember-Twiddle reproduction: https://ember-twiddle.com/097b5e7a6d20c5dee88d65149f7b28ee

I opened up a bug report with Firefox because this seems to be an issue specific to that browser so far, but if anyone can reproduce with other screen readers and other browsers please add to this issue!

https://bugzilla.mozilla.org/show_bug.cgi?id=1667494

pzuraq avatar Sep 25 '20 23:09 pzuraq

Hi, I came across this bug today. I was caught by surprise, because template syntax (using {{eq}} helper) didn't just work. Could we push for a fix so that developers can write semantic HTML more easily?

In Ember 3.28.6 (tested on Chrome 98.0), the following template didn't set the selected attribute correctly:

{{!-- this.options is an array of strings, @value is string or undefined --}}

{{#each this.options as |opt|}}
  <option
    selected={{eq opt @value}}
    value={{opt}}
  >
    {{opt}}
  </option>
{{/each}}

I ended up using an if-else statement:

{{#each this.options as |opt|}}
  {{#if (eq opt @value)}}
    <option
      selected
      value={{opt}}
    >
      {{opt}}
    </option>

  {{else}}
    <option value={{opt}}>
      {{opt}}
    </option>

  {{/if}}
{{/each}}

As an aside, I didn't write the <option> tag as shown above, but used a yielded component:

{{!-- Some reusable option component --}}

<option
  disabled={{@isDisabled}}
  value={{@value}}
  ...attributes
>
  {{@label}}
</option>

ijlee2 avatar Feb 22 '22 15:02 ijlee2

@locks on Discord

I can't reproduce with <select> and <option>, the problem might be in ...attributes instead

ijlee2 avatar Feb 22 '22 16:02 ijlee2

I just came across this, trying to set selected as an attribute using a simple template like <option selected={{this.isSelected}}>{{yield}}</option>.

I haven't tried what a screenreader does, but even when ignoring a11y for a second, then selected should nevertheless be set as an attribute and not a property I believe? Like in a SSR/FastBoot setup, you want a selected option to actually render as selected even before JS kicks in, right?

By contrast, the Ember example always sets the property, so no option will have the attribute set at any time.

I don't know what the heuristics is that Ember applies in determining the attribute vs. property question, does anyone here? At least it seems wrong here IMHO.

For now I ended up using the workaround that @ijlee2 suggested, but that's not really nice. Also now it does way more DOM manipulation when the selected option changes, as it destroys and creates new <option> elements for all the options, rather than doing a simple setAttribute('selected', ...). 😕

simonihmig avatar Mar 02 '23 08:03 simonihmig