Skip to content

Gecko vbuf: Render the selected item in list boxes and trees.#9166

Merged
michaelDCurran merged 3 commits into
nvaccess:masterfrom
jcsteh:geckoVbufRenderSelectedItem
Jan 23, 2019
Merged

Gecko vbuf: Render the selected item in list boxes and trees.#9166
michaelDCurran merged 3 commits into
nvaccess:masterfrom
jcsteh:geckoVbufRenderSelectedItem

Conversation

@jcsteh
Copy link
Copy Markdown
Contributor

@jcsteh jcsteh commented Jan 17, 2019

Link to issue number:

Fixes #3573. Fixes #9157.

Summary of the issue:

  1. When an ARIA list box or tree isn't focusable but its items are (as permitted by the ARIA spec), it is currently impossible to focus the list box/tree in browse mode.
  2. In browse mode, the selected item in a list box or tree is not reported. The user has to use the report focus command or similar to query for this information.

Description of how this pull request fixes the issue:

In the Gecko vbuf backend, IAccessible::accSelection is used to retrieve the selected item in list boxes and trees. This is then rendered into the buffer. If there is no selected item, the first chlid is rendered.

This required changing speech.getControlFieldSpeech so that the content of lists and trees is spoken after the control field info for REASON_FOCUS. Otherwise, moving to a list box or tree with quick navigation would speak the selected item first.

List item also had to be added to the roles which always enable pass through, as this change means that a list item can now be under the cursor when enter is pressed. (Read only list items are still excluded by an earlier check.)

This is currently disabled for HTML select size>1 controls in Chrome. These list items get the focusable state but setting focus programmatically does nothing. Therefore, we don't want to render these in Chrome because a user wouldn't be able to focus these list boxes in browse mode if we did.

Testing performed:

Test cases:

  • HTML select, initially selected item should be "b":
    data:text/html,<select size="2"><option>a</option><option selected>b</option></select>
  • ARIA tree, selected item should be "b":
    data:text/html,<div role="tree"><div id="a" role="treeitem">a</div><div id="b" role="treeitem" aria-selected="true" tabindex="0">b</div></div><button onClick="a.setAttribute('aria-selected', 'true'); b.setAttribute('aria-selected', 'false');">Select a</button>
    Use the "Select a" button to change the selection.

In Firefox, for each test case, verified the initial selection is reported in the buffer. Verified that when the selection is changed, the buffer is updated.
In Chrome, only the tree test case can be tested, since this is disabled for HTML select in Chrome.

Known issues with pull request:

Disabled for HTML select in Chrome due to a Chrome bug.

Change log entry:

New Features:

- In Mozilla Firefox and Google Chrome, browse mode now reports the selected item in list boxes and trees.
 - This works in Firefox 66 and later.
 - This does not work for certain list boxes (HTML select controls) in Chrome.

Bug Fixes:
- In Mozilla Firefox and Google Chrome, switching to focus mode now works correctly for certain list boxes and trees (where the list box/tree is not itself focusable but its items are) . (#3573, #9157)

If there is no selected item, render the first child.
This allows the user to see the selected item without having to switch to focus mode.
It also allows focus to move into list boxes and trees where the container itself isn't focusable but the selected item is, as permitted by the ARIA spec.

This required changing speech.getControlFieldSpeech so that the content of lists and trees is spoken after the control field info for REASON_FOCUS.
Otherwise, moving to a list box or tree with quick navigation would speak the selected item first.
List item also had to be added to the roles which always enable pass through, as this change means that a list item can now be under the cursor when enter is pressed. (Read only list items are still excluded by an earlier check.)

This is currently disabled for HTML select size>1 controls in Chrome.
These list items get the focusable state but setting focus programmatically does nothing.
Therefore, we don't want to render these in Chrome because a user wouldn't be able to focus these list boxes in browse mode if we did.
@jcsteh jcsteh requested a review from michaelDCurran January 17, 2019 00:57
@michaelDCurran
Copy link
Copy Markdown
Member

Although I can confirm that the selected item in a list is being rendered, I cannot get NVDA to render the first child if nothing is selected.
From the quick bit of debugging I did, it looks as though in Firefox Nightly 66, code gets all the way past successfully calling IEnumVARIANT->Next, but item 0 is not a VT_DISPATCH.
My guess would be that Next is returning S_FALSE as although there was no actual error, the element count emitted was smaller than the requested number.
And therefore item 0's vt would most likely be left as VT_EMPTY.

@jcsteh
Copy link
Copy Markdown
Contributor Author

jcsteh commented Jan 21, 2019

Although I can confirm that the selected item in a list is being rendered, I cannot get NVDA to render the first child if nothing is selected.
From the quick bit of debugging I did, it looks as though in Firefox Nightly 66, code gets all the way past successfully calling IEnumVARIANT->Next, but item 0 is not a VT_DISPATCH.

Ug. My bad.

I could fix this here, but IMO, this is a bug in Firefox. Looking at the docs for accSelection, we really shouldn't be returning an IEnumVARIANT when there is no selection; it specifically says VT_UNKNOWN is only for "multiple child objects". Instead, we should return VT_EMPTY. I've fixed that now in Mozilla bug 1521438. While I was at it, I changed it to do VT_DISPATCH for a single selected item, since it's more correct strictly speaking and more efficient as well. Note that Chrome already behaves correctly in this respect.

With that patch, test cases and expected outcomes:

No selection, NVDA should report "a":
data:text/html,<div role="listbox"><div role="option">a</div><div role="option">b</div></div>

Single selection, NVDA should report "b":
data:text/html,<div role="listbox"><div role="option">a</div><div role="option" aria-selected="true">b</div></div>

Multiple selection, NVDA should report "a":
data:text/html,<div role="listbox"><div role="option" aria-selected="true">a</div><div role="option" aria-selected="true">b</div></div>

@MarcoZehe
Copy link
Copy Markdown
Contributor

FYI, I just reviewed and landed this bug, should be in the next or second to next nightly build.

@jcsteh
Copy link
Copy Markdown
Contributor Author

jcsteh commented Jan 22, 2019

@michaelDCurran, the Firefox bug fix is in Nightly now, so the first item should get rendered when there's no selection as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants