there's no way to know unselected item when selection in single selection was changed

RESOLVED FIXED in mozilla25

Status

()

defect
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: surkov, Assigned: surkov)

Tracking

(Blocks 1 bug, {access})

unspecified
mozilla25
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

spun off the bug 804040 comment #4:

"> 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."

It seems everybody ok to fire selection change events additionally (they are excess but there's no nice way). (Btw, ATK wants them too).
sorry, weird name changing.
Summary: ARIA tabs aren't subject of selection events → there's no way to know unselected item when selection in single selection was changed
needed to make a11y test suite passing
Blocks: 2013q3a11y
Posted patch patchSplinter Review
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #773418 - Flags: review?(trev.saunders)
Comment on attachment 773418 [details] [diff] [review]
patch

>diff --git a/accessible/src/base/EventQueue.cpp b/accessible/src/base/EventQueue.cpp
>--- a/accessible/src/base/EventQueue.cpp
>+++ b/accessible/src/base/EventQueue.cpp
>@@ -298,17 +298,17 @@ EventQueue::CoalesceSelChangeEvents(AccS
>       aTailEvent->mPackedEvent = aThisEvent;
>       return;
>     }
> 
>     if (aThisEvent->mSelChangeType == AccSelChangeEvent::eSelectionAdd &&
>         aTailEvent->mSelChangeType == AccSelChangeEvent::eSelectionRemove) {
>       aTailEvent->mEventRule = AccEvent::eDoNotEmit;
>       aThisEvent->mEventType = nsIAccessibleEvent::EVENT_SELECTION;
>-      aThisEvent->mPackedEvent = aThisEvent;
>+      aThisEvent->mPackedEvent = aTailEvent;

sort of unrelated though it seems correct.

>+   */
>+  static void FireEvent(mozilla::a11y::Accessible* aTarget, uint64_t aState,
>+                        bool aIsEnabled, bool aIsFromUserInput)

I'd prefer you just inlined this function or if you really want to keep it a function make it a file static function in EventQueue.cpp

>+  {
>+    nsRefPtr<mozilla::a11y::AccStateChangeEvent> stateChangeEvent =
>+      new mozilla::a11y::AccStateChangeEvent(aTarget, aState, aIsEnabled,
>+                                             static_cast<mozilla::a11y::EIsFromUserInput>(aIsFromUserInput));

to be safe I think you need to do ? or something but I'm pretty sure casting a bool to an enum isn't required to work correctly by the spec.
Attachment #773418 - Flags: review?(trev.saunders) → review+
(In reply to Trevor Saunders (:tbsaunde) from comment #4)
> Comment on attachment 773418 [details] [diff] [review]
> patch
> 
> >diff --git a/accessible/src/base/EventQueue.cpp b/accessible/src/base/EventQueue.cpp
> >--- a/accessible/src/base/EventQueue.cpp

> >     if (aThisEvent->mSelChangeType == AccSelChangeEvent::eSelectionAdd &&
> >         aTailEvent->mSelChangeType == AccSelChangeEvent::eSelectionRemove) {
> >       aTailEvent->mEventRule = AccEvent::eDoNotEmit;
> >       aThisEvent->mEventType = nsIAccessibleEvent::EVENT_SELECTION;
> >-      aThisEvent->mPackedEvent = aThisEvent;
> >+      aThisEvent->mPackedEvent = aTailEvent;
> 
> sort of unrelated though it seems correct.

it is related, otherwise I can't unwrap selection change event and fire state change events properly

> >+   */
> >+  static void FireEvent(mozilla::a11y::Accessible* aTarget, uint64_t aState,
> >+                        bool aIsEnabled, bool aIsFromUserInput)
> 
> I'd prefer you just inlined this function 

you mean no function, just copy/paste? It looked cumbersome that's why I went with inline fuction

> or if you really want to keep it a
> function make it a file static function in EventQueue.cpp

EventShell.h seems looking ok for this propose since it has basic FireEvent already

> >+  {
> >+    nsRefPtr<mozilla::a11y::AccStateChangeEvent> stateChangeEvent =
> >+      new mozilla::a11y::AccStateChangeEvent(aTarget, aState, aIsEnabled,
> >+                                             static_cast<mozilla::a11y::EIsFromUserInput>(aIsFromUserInput));
> 
> to be safe I think you need to do ? or something but I'm pretty sure casting
> a bool to an enum isn't required to work correctly by the spec.

oh, I will fix it.
https://hg.mozilla.org/mozilla-central/rev/12eedfb87ed6
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.