Closed Bug 810268 Opened 12 years ago Closed 11 years ago

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

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: surkov, Assigned: surkov)

References

(Blocks 1 open bug)

Details

(Keywords: access)

Attachments

(1 file)

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
Attached 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: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: