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

RESOLVED FIXED in mozilla25

Status

()

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

People

(Reporter: surkov, Assigned: surkov)

Tracking

(Blocks: 1 bug, {access})

unspecified
mozilla25
access
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
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).
(Assignee)

Comment 1

5 years ago
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
(Assignee)

Comment 2

4 years ago
needed to make a11y test suite passing
Blocks: 887794
(Assignee)

Comment 3

4 years ago
Created attachment 773418 [details] [diff] [review]
patch
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+
(Assignee)

Comment 5

4 years ago
(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.
(Assignee)

Comment 6

4 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/12eedfb87ed6
https://hg.mozilla.org/mozilla-central/rev/12eedfb87ed6
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.