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

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

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

Comment 1

[alexander :surkov (:asurkov)]

sorry, weird name changing.

Comment 2

[alexander :surkov (:asurkov)]

needed to make a11y test suite passing

Comment 3

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 4

[Trevor Saunders (:tbsaunde)]

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.

Comment 5

[alexander :surkov (:asurkov)]

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

Comment 6

[alexander :surkov (:asurkov)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/12eedfb87ed6

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/12eedfb87ed6