Closed
Bug 788389
Opened 12 years ago
Closed 11 years ago
fire statechange event whenever checked state is changed not depending on focused state
Categories
(Core :: Disability Access APIs, defect)
Core
Disability Access APIs
Tracking
()
RESOLVED
FIXED
mozilla25
People
(Reporter: surkov, Assigned: surkov)
References
(Blocks 1 open bug)
Details
(Keywords: access)
Attachments
(1 file)
(deleted),
patch
|
tbsaunde
:
review+
|
Details | Diff | Splinter Review |
We were asked to fire statechange event whenever checked state is changed, for example, if the author does checkbox.checked = true. Historically we fired the event when the widget was focused. We started to do exceptions on this way, for example, for ARIA attributes like bug 467143.
Comment 1•12 years ago
|
||
Fwiw, NVDA needs this too. I just never thought of it before. :)
Assignee | ||
Comment 2•12 years ago
|
||
(In reply to James Teh [:Jamie] from comment #1)
> Fwiw, NVDA needs this too. I just never thought of it before. :)
ok, cool :)
Assignee | ||
Comment 3•12 years ago
|
||
Instead of listening the CheckboxStateChange DOM event we need to listen content state change for HTML and checked attribute change for XUL.
Consequences
1) this approach allows to fix bug 477997
2) we can start to keep cached accessible states for example mStates (too fat for one cached state though)
3) we can provide HasNativeState(aState) method optimized to work on mFlags like
a) if (mStates & aState) return true; if (states::FOCUSABLE & aState) return NativeInteractiveState() & aState; etc.
Trev, does it look good?
Comment 4•12 years ago
|
||
(In reply to alexander :surkov from comment #3)
> Instead of listening the CheckboxStateChange DOM event we need to listen
> content state change for HTML and checked attribute change for XUL.
Well, I'm basically always for getting rid of usage of DOM events :)
> Consequences
> 1) this approach allows to fix bug 477997
> 2) we can start to keep cached accessible states for example mStates (too
> fat for one cached state though)
I'm not sure that needs to be part of this, but if we were to do it I think we could probably cache a couple other already, but that would need to be checked.
> 3) we can provide HasNativeState(aState) method optimized to work on mFlags
> like
> a) if (mStates & aState) return true; if (states::FOCUSABLE & aState) return
> NativeInteractiveState() & aState; etc.
That could be nice, but if the state is cached as off we don't take advantage of the cache which seems like something we should do.
> Trev, does it look good?
I'm not thrilled by the thought of making every Accessible 8 bytes larger, but its probably worth it, and maybe we can be clever and not increase size that much.
Assignee | ||
Comment 5•12 years ago
|
||
(In reply to Trevor Saunders (:tbsaunde) from comment #4)
> > 2) we can start to keep cached accessible states for example mStates (too
> > fat for one cached state though)
>
> I'm not sure that needs to be part of this, but if we were to do it I think
> we could probably cache a couple other already, but that would need to be
> checked.
I think the idea was to keep checkable as part of mStates (we could do that in mFlags though) because we need to know at least for XUL when checked is valid attribute and when it's not.
> I'm not thrilled by the thought of making every Accessible 8 bytes larger,
> but its probably worth it, and maybe we can be clever and not increase size
> that much.
for example?
Assignee | ||
Comment 6•11 years ago
|
||
I've been asked again about this (so adding dependency on bug 2013q3a11y). I think to fix the bug HTML checkbox and radios only (leaving the idea from comment #3 out of board for now). So XUL elements and other controls will be still focus dependent.
Blocks: 2013q3a11y
Assignee | ||
Comment 7•11 years ago
|
||
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #779323 -
Flags: review?(trev.saunders)
Comment 8•11 years ago
|
||
Comment on attachment 779323 [details] [diff] [review]
patch
>-
>- // radiogroup in prefWindow is exposed as a list,
>- // and panebutton is exposed as XULListitem in A11y.
>- // XULListitemAccessible::GetStateInternal uses STATE_SELECTED in this case,
>- // so we need to check states::SELECTED also.
> bool isEnabled = (state & (states::CHECKED | states::SELECTED)) != 0;
>
>- nsRefPtr<AccEvent> accEvent =
>- new AccStateChangeEvent(accessible, states::CHECKED, isEnabled);
>- nsEventShell::FireEvent(accEvent);
>+ if (accessible->NeedsDOMUIEvent()) {
why can't we just bail out at the start if dom ui events aren't needed?
var gQueue = null;
>
> // var gA11yEventDumpID = "eventdump"; // debug stuff
>- //gA11yEventDumpToConsole = true; // debug stuff
>+ gA11yEventDumpToConsole = true; // debug stuff
keep commented
Attachment #779323 -
Flags: review?(trev.saunders) → review+
Assignee | ||
Comment 9•11 years ago
|
||
(In reply to Trevor Saunders (:tbsaunde) from comment #8)
> Comment on attachment 779323 [details] [diff] [review]
> patch
>
> >-
> >- // radiogroup in prefWindow is exposed as a list,
> >- // and panebutton is exposed as XULListitem in A11y.
> >- // XULListitemAccessible::GetStateInternal uses STATE_SELECTED in this case,
> >- // so we need to check states::SELECTED also.
> > bool isEnabled = (state & (states::CHECKED | states::SELECTED)) != 0;
> >
> >- nsRefPtr<AccEvent> accEvent =
> >- new AccStateChangeEvent(accessible, states::CHECKED, isEnabled);
> >- nsEventShell::FireEvent(accEvent);
> >+ if (accessible->NeedsDOMUIEvent()) {
>
> why can't we just bail out at the start if dom ui events aren't needed?
in case of RadioStateChange it's still needed to fire focus events
Assignee | ||
Comment 10•11 years ago
|
||
Flags: in-testsuite+
Comment 11•11 years ago
|
||
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.
Description
•