Selection event not fired when selection of ARIA tab changes

RESOLVED FIXED in mozilla19

Status

()

Core
Disability Access APIs
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Jamie, Assigned: surkov)

Tracking

(Blocks: 1 bug)

Trunk
mozilla19
x86_64
Windows 7
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [davidb +1])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Created attachment 673744 [details]
Test case.

Str:
1. Open the attached test case.
2. Press the "Select tab" button.
Expected: A state change event should be fired on the tab.
Actual: No state change event is fired.
Impact: Screen readers such as NVDA that maintain their own representation of the document are not notified when tab selection changes and thus report this incorrectly to the user.
(Assignee)

Comment 1

5 years ago
Shouldn't we fire selection event instead? We do that for XUL tabs and we don't fire state change events for them.
(Assignee)

Updated

5 years ago
Blocks: 343213
(Reporter)

Comment 2

5 years ago
(In reply to alexander :surkov from comment #1)
> Shouldn't we fire selection event instead?
My apologies. Yes, it should be a selection event. The only problem is that this wouldn't signal to AT when a given tab is unselected. Hmm.
(Reporter)

Updated

5 years ago
Summary: State change event not fired when selection of ARIA tab changes → Selection event not fired when selection of ARIA tab changes
(Assignee)

Comment 3

5 years ago
(In reply to James Teh [:Jamie] from comment #2)
> (In reply to alexander :surkov from comment #1)
> > Shouldn't we fire selection event instead?
> My apologies. Yes, it should be a selection event. The only problem is that
> this wouldn't signal to AT when a given tab is unselected. Hmm.

selection event is fired when single item is selected (all others are unselected) within a container, otherwise we fire selection_add/remove or within events.

Why do you need to know an item was unselected?

Btw, aria implementation guide is unclear:

5.8.1. State and Property Change Events (http://www.w3.org/WAI/PF/aria-implementation/#mapping_events_state-change):

aria-selected (state):
EVENT_OBJECT_SELECTIONADD
EVENT_OBJECT_SELECTIONREMOVE

5.8.3. Selection (http://www.w3.org/WAI/PF/aria-implementation/#mapping_events_selection):

Toggle aria-selected:
EVENT_OBJECT_SELECTIONADD/EVENT_OBJECT_SELECTIONREMOVE on the current container + EVENT_OBJECT_STATECHANGE on the item

Selection follows focus:
EVENT_OBJECT_SELECTION then EVENT_OBJECT_STATECHANGE on newly focused item, but arrange events so state change does not occur on focused item, to avoid extra selection change announcements

Select or deselect many items at once:
EVENT_OBJECT_SELECTIONWITHIN is all that is necessary. The state change events MAY be trimmed out for performance.
(Reporter)

Comment 4

5 years ago
(In reply to alexander :surkov from comment #3)
> Why do you need to know an item was unselected?
So that NVDA's representation can be updated such that selected is no longer reported for the tab that was just unselected. This is why I originally thought state change, but selection does make more sense. Ug.
(Assignee)

Comment 5

5 years ago
I'm fine to follow any reasonable idea. Btw, ARIA impl guide says to fire selection and state change events both. Does make sense for you? Do you know what does IE/others do?
(Reporter)

Comment 6

5 years ago
(In reply to alexander :surkov from comment #5)
> I'm fine to follow any reasonable idea. Btw, ARIA impl guide says to fire
> selection and state change events both. Does make sense for you?
Ideally, we need an event to be fired when selected and an event to be fired when unselected. I guess selection makes sense for selection and state change for unselection, though that's obviously asymmetric. It just seems silly to fire two events for a single action.

> Do you know
> what does IE/others do?
I'm not sure. For IE, due to the major deficiencies in its accessibility implementation, we have to watch the DOM directly using a different API, so we don't really take much interest in its MSAA events. :)
(In reply to James Teh [:Jamie] from comment #6)
> (In reply to alexander :surkov from comment #5)
> > I'm fine to follow any reasonable idea. Btw, ARIA impl guide says to fire
> > selection and state change events both. Does make sense for you?
> Ideally, we need an event to be fired when selected and an event to be fired
> when unselected. I guess selection makes sense for selection and state
> change for unselection, though that's obviously asymmetric. It just seems
> silly to fire two events for a single action.

I would agree there's some dupplication, but I think firing both selection and selected state change events would be correct.  Otherwise someone who only tracks states might not know one has changed.
(Assignee)

Comment 8

5 years ago
Currently we fire events (in case of Firefox tabs):

    EVENT_OBJECT_SELECTION when single item was selected, event is targeted to item.

    EVENT_OBJECT_SELECTIONADD and EVENT_OBJECT_SELECTIONREMOVE when item(s) was unselected and/or items were selected, events are targeted to changed items.

    EVENT_OBJECT_SELECTIONWITHIN when selection within control was changed much, for example, when all items were selected. In this case screen readers can announce that selection was changed rather than announce every changed item. Event target is control.

if we fire state_change events then they dupe selectionadd/selectionremove events. Perhaps it's ok if they are used for different proposes like selection/selectionadd/selectionremove to announce the change, state change events to update the buffer.

Anyway, it's not in sync with ARIA spec what puts us into interesting state.

David, is ARIA on your plate?
(In reply to alexander :surkov from comment #8)

> David, is ARIA on your plate?

Yes I can take this one. (I need a bigger plate)

Are we all in agreement we should fire a state change event for selection? (I've put my vote in the whiteboard)
Whiteboard: [david +1]

Updated

5 years ago
Whiteboard: [david +1] → [davidb +1]
(Reporter)

Comment 10

5 years ago
FWIW, Chrome fires selectionAdd on a tab which gets selected and selectionRemove on a tab which gets unselected. Technically, this is a bit strange, since tab controls are a single selection control, but it does solve the problem without duplicate events.
(Assignee)

Comment 11

5 years ago
Jamie, in that case it seems AT should learn how to differentiate following cases on single selection list:
1) selection were moved: selection remove + selection add events
2) selection was lost: selection remove event

(case of listbox)
(Reporter)

Comment 12

5 years ago
(In reply to alexander :surkov from comment #11)
> Jamie, in that case it seems AT should learn how to differentiate following
> cases on single selection list:
> 1) selection were moved: selection remove + selection add events
> 2) selection was lost: selection remove event
NVDA should handle that fine, but some might argue it seems odd for single selection list boxes. Maybe we should just add the stateChange as suggested elsewhere. Either way, we probably need to check/test with other ATs.
(Assignee)

Comment 13

5 years ago
so we deal with two problems here:
1) ARIA tabs aren't subject of selection events
2) there's no way to know unselected item for single selection container
(Assignee)

Comment 14

5 years ago
I filed bug 810268 for 2nd item, let's keep this bug for 1st issue
(Assignee)

Comment 15

5 years ago
Created attachment 680056 [details] [diff] [review]
wip

Trev, I'm thinking to combine
1) AccTypes.h and protected Accessible::AccessibleTypes
2) make mFlags 64bit unit
3) introduce public Accessible::IsOfType(unit64_t aAccTypes) and protected SetAccType() which take into account shift of StateFlags and ChildrenFlags.

How does it sound?
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
(Assignee)

Comment 16

5 years ago
Created attachment 680058 [details] [diff] [review]
patch

we can take this and file follow up based on comment #15 to make accessible types nicer.
Attachment #680056 - Attachment is obsolete: true
Attachment #680058 - Flags: review?(trev.saunders)
Comment on attachment 680058 [details] [diff] [review]
patch

do you really need both the friend thing and public in Accessible.h for the enum>
Attachment #680058 - Flags: review?(trev.saunders) → review+
(Assignee)

Comment 18

5 years ago
(In reply to Trevor Saunders (:tbsaunde) from comment #17)
> Comment on attachment 680058 [details] [diff] [review]
> patch
> 
> do you really need both the friend thing and public in Accessible.h for the
> enum>

no, I removed it already (artifacts, sorry). thank you for the catch

how do you feel about comment #15?
sorry I forgot to answer that, I'm certainly fine with everything other than making mFlags 64 bits.  I'll live with that if we need to, but of course I'd rather not.  Note the mRoleMapEntry poitner is pretty low data for a pointer size object maybe we should make that an index into the array and stuff that in the mFlags blob.

I'd actually sort of like to use packed bit field stuff to avoid the nasty shifting alltogether so we'd have like

uint32_t mTypeFlags : 20;
uint32_t mChildFlags : 2;
uint32_t mRoleMap : 5;
or something the one worrying thing about that is we could make accessibles bigger and not really notice, maybe we can add static asserts?

The other thing I started pondering was creating some seperate mTypeInfo struct for accessibles like mNodeInfo for nodes.
(Assignee)

Comment 20

5 years ago
filed bug 810572 on this
https://hg.mozilla.org/mozilla-central/rev/09c4b0d21581
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
You need to log in before you can comment on or make changes to this bug.