Closed
Bug 1213402
Opened 9 years ago
Closed 9 years ago
No ATK/AT-SPI2 object:property-change:accessible-value events for web widgets in e10s
Categories
(Core :: Disability Access APIs, defect)
Tracking
()
RESOLVED
FIXED
mozilla45
People
(Reporter: jdiggs, Assigned: tbsaunde)
References
(Blocks 1 open bug)
Details
Attachments
(4 files)
458 bytes,
text/x-python
|
Details | |
18.59 KB,
patch
|
davidb
:
review+
|
Details | Diff | Splinter Review |
1.09 KB,
patch
|
davidb
:
review+
|
Details | Diff | Splinter Review |
17.50 KB,
patch
|
davidb
:
review+
|
Details | Diff | Splinter Review |
Steps to reproduce: 1. View http://files.paciellogroup.com/blogmisc/samples/aria/slider/ 2. Run the attached pyatspi accessible-event listener in a terminal 3. Change the value of the sliders on the page Expected results: object:property-change:accessible-value events would be emitted each time the slider value is changed. Actual results: Things work as expected if e10s is not enabled; if e10s is enabled, the events are absent. Note: It seems like the value interface itself is working. You can, for instance, query the value interface of any of the sliders. It's just the events which are missing. (And it's the events which tell Orca that the value interface should be queried and the results presented to the user.)
Blocks: e10sa11y2
tracking-e10s:
--- → +
Assignee | ||
Comment 1•9 years ago
|
||
Arguably these are different things, and it will be easier to proxy events for atk this way because atk only wants the numeric value changes.
Attachment #8682146 -
Flags: review?(dbolter)
Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8682148 -
Flags: review?(dbolter)
Comment 3•9 years ago
|
||
Comment on attachment 8682148 [details] [diff] [review] Fire numeric value change events for proxies Review of attachment 8682148 [details] [diff] [review]: ----------------------------------------------------------------- ::: accessible/atk/AccessibleWrap.cpp @@ +1476,5 @@ > // A hack using state change showing events as alert events. > atk_object_notify_state_change(wrapper, ATK_STATE_SHOWING, true); > break; > + case nsIAccessibleEvent::EVENT_VALUE_CHANGE: > + g_object_notify((GObject*)wrapper, "accessible-value"); Why do you need the cast?
Assignee | ||
Comment 4•9 years ago
|
||
(In reply to David Bolter [:davidb] from comment #3) > Comment on attachment 8682148 [details] [diff] [review] > Fire numeric value change events for proxies > > Review of attachment 8682148 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: accessible/atk/AccessibleWrap.cpp > @@ +1476,5 @@ > > // A hack using state change showing events as alert events. > > atk_object_notify_state_change(wrapper, ATK_STATE_SHOWING, true); > > break; > > + case nsIAccessibleEvent::EVENT_VALUE_CHANGE: > > + g_object_notify((GObject*)wrapper, "accessible-value"); > > Why do you need the cast? because Glib is silly and tries to reimplement C++ in C and so you need to cast to upcast.
Updated•9 years ago
|
Assignee: nobody → tbsaunde+mozbugs
Comment 5•9 years ago
|
||
Comment on attachment 8682146 [details] [diff] [review] separate value change events into text value changes and numeric value changes Review of attachment 8682146 [details] [diff] [review]: ----------------------------------------------------------------- ok ::: accessible/base/nsAccessibilityService.h @@ +308,5 @@ > "location change", // EVENT_LOCATION_CHANGE > "name changed", // EVENT_NAME_CHANGE > "description change", // EVENT_DESCRIPTION_CHANGE > "value change", // EVENT_VALUE_CHANGE > + "text value change", // EVENT_TEXT_VALUE_CHANGE nit: Comment indented too far (5 spaces). ::: accessible/generic/RootAccessible.cpp @@ +453,5 @@ > else if (accessible->NeedsDOMUIEvent() && > eventType.EqualsLiteral("ValueChange")) { > + uint32_t event = accessible->HasNumericValue() > + ? nsIAccessibleEvent::EVENT_VALUE_CHANGE > + : nsIAccessibleEvent::EVENT_TEXT_VALUE_CHANGE; Hmm I guess we're ok never firing an EVENT_TEXT_VALUE_CHANGE if eHasNumericValue is set... ::: accessible/interfaces/nsIAccessibleEvent.idl @@ +414,5 @@ > > /** > + * An object's text Value has changed. > + */ > + const unsigned long EVENT_TEXT_VALUE_CHANGE = 0x0057; I guess we're ok getting out of order here...
Attachment #8682146 -
Flags: review?(dbolter) → review+
Updated•9 years ago
|
Attachment #8682148 -
Flags: review?(dbolter) → review+
Assignee | ||
Comment 6•9 years ago
|
||
(In reply to David Bolter [:davidb] from comment #5) > Comment on attachment 8682146 [details] [diff] [review] > separate value change events into text value changes and numeric value > changes > > Review of attachment 8682146 [details] [diff] [review]: > ----------------------------------------------------------------- > > ok > > ::: accessible/base/nsAccessibilityService.h > @@ +308,5 @@ > > "location change", // EVENT_LOCATION_CHANGE > > "name changed", // EVENT_NAME_CHANGE > > "description change", // EVENT_DESCRIPTION_CHANGE > > "value change", // EVENT_VALUE_CHANGE > > + "text value change", // EVENT_TEXT_VALUE_CHANGE > > nit: Comment indented too far (5 spaces). > > ::: accessible/generic/RootAccessible.cpp > @@ +453,5 @@ > > else if (accessible->NeedsDOMUIEvent() && > > eventType.EqualsLiteral("ValueChange")) { > > + uint32_t event = accessible->HasNumericValue() > > + ? nsIAccessibleEvent::EVENT_VALUE_CHANGE > > + : nsIAccessibleEvent::EVENT_TEXT_VALUE_CHANGE; > > Hmm I guess we're ok never firing an EVENT_TEXT_VALUE_CHANGE if > eHasNumericValue is set... > > ::: accessible/interfaces/nsIAccessibleEvent.idl > @@ +414,5 @@ > > > > /** > > + * An object's text Value has changed. > > + */ > > + const unsigned long EVENT_TEXT_VALUE_CHANGE = 0x0057; > > I guess we're ok getting out of order here... no, that's totally bogus. This needs to line up with the various arrays this patch also updates.
Assignee | ||
Comment 7•9 years ago
|
||
Arguably these are different things, and it will be easier to proxy events for atk this way because atk only wants the numeric value changes.
Attachment #8685579 -
Flags: review?(dbolter)
Comment 8•9 years ago
|
||
Comment on attachment 8685579 [details] [diff] [review] separate value change events into text value changes and numeric value changes Review of attachment 8685579 [details] [diff] [review]: ----------------------------------------------------------------- Order looks good, just an indent nit. ::: accessible/base/nsAccessibilityService.h @@ +384,5 @@ > "hypertext changed", // EVENT_HYPERTEXT_CHANGED > "hypertext links count changed", // EVENT_HYPERTEXT_NLINKS_CHANGED > "object attribute changed", // EVENT_OBJECT_ATTRIBUTE_CHANGED > "virtual cursor changed" // EVENT_VIRTUALCURSOR_CHANGED > + "text value change", // EVENT_TEXT_VALUE_CHANGE nit: comment indent is still too deep.
Attachment #8685579 -
Flags: review?(dbolter) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/72f55d458dd6 https://hg.mozilla.org/integration/mozilla-inbound/rev/5a3d821ef31b
Comment 12•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/72f55d458dd6 https://hg.mozilla.org/mozilla-central/rev/5a3d821ef31b https://hg.mozilla.org/mozilla-central/rev/e1e714d10b5e https://hg.mozilla.org/mozilla-central/rev/b40b6c44fe97
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox45:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in
before you can comment on or make changes to this bug.
Description
•