1056803 - Add an object attribute changed event interface.

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Yura Zenevich [:yzen] (please use "needinfo")]

Currently object attribute changed accessibility event is a generic accessibility event. This means it's difficult to determine which attribute actually changed. 

We need to add a nsiAccessibleObjectAttributeChangedEvent interface that will have additional information about which attribute has changed such as attribute name.

Comment 1

[Yura Zenevich [:yzen] (please use "needinfo")]

Attachment: 1056803 patch v1

Comment 2

[Trevor Saunders (:tbsaunde)]

Comment on attachment 8476689 [details] [diff] [review]
1056803 patch v1

>+AccObjectAttrChangedEvent::
>+  AccObjectAttrChangedEvent(Accessible* aAccessible,
>+                            const nsAString& aAttributeName)
>+  : AccEvent(::nsIAccessibleEvent::EVENT_OBJECT_ATTRIBUTE_CHANGED, aAccessible)
>+  , mAttributeName(aAttributeName)

inline it.

>+++ b/accessible/base/AccEvent.h
>+class AccObjectAttrChangedEvent: public AccEvent
>+{
>+public:
>+  AccObjectAttrChangedEvent(Accessible* aAccessible,
>+                            const nsAString& aAttributeName);

use nsIAtom*

>+  virtual ~AccObjectAttrChangedEvent() { };

should be protected.


>+++ b/accessible/interfaces/nsIAccessibleObjectAttributeChangedEvent.idl
>@@ -0,0 +1,18 @@
>+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-

add a vim mode line too.

>+++ b/accessible/tests/mochitest/events.js
>@@ -1417,16 +1417,33 @@ function openCombobox(aComboboxID)
>   }
>
>   this.getID = function openCombobox_getID()
>   {
>     return "open combobox " + prettyName(aComboboxID);
>   }
> }
>
>+function updateAttribute(aID, aAttr, aValue) {

brace on next line.

I guess there's no real reason to move this stuff around you don't use it elsewhere.

>+  this.invoke = function updateAttribute_invoke() {

same and elsewhere.

Comment 3

[Yura Zenevich [:yzen] (please use "needinfo")]

Attachment: 1056803 patch v2

Updated the patch based on comments. Thanks!

Comment 4

[Yura Zenevich [:yzen] (please use "needinfo")]

Attachment: 1056803 patch v3

Updated the patch to return nsIAtom, as you requested, Trevor. Hopefully looks good now. Thanks

Comment 5

[Trevor Saunders (:tbsaunde)]

Comment on attachment 8479587 [details] [diff] [review]
1056803 patch v3

>+class AccObjectAttrChangedEvent: public AccEvent
>+{
>+protected:
>+  virtual ~AccObjectAttrChangedEvent() { }

actually it can be private since nothing inherits from this.

>+
>+private:
>+  nsRefPtr<nsIAtom> mAttribute;

nsCOMPtr<nsIAtom> stupid custom aria attributes that prevent us from using nsIAtom*

>+function objAttrChangedChecker(aID, aAttr)
>+{
>+
>+  this.__proto__ = new invokerChecker(EVENT_OBJECT_ATTRIBUTE_CHANGED, aID);

extra blank line

>+
>+  this.check = function objAttrChangedChecker_check(aEvent)
>+  {
>+    var event = null;
>+    try {
>+      var event = aEvent.QueryInterface(
>+        nsIAccessibleObjectAttributeChangedEvent);
>+    } catch (e) {
>+      ok(false, "Object attribute changed event was expected");
>+    }
>+
>+    if (!event) {
>+      return;
>+    }
>+
>+    is(event.changedAttribute.toString(), aAttr,
>+      "Wrong attribute name of the object attribute changed event.");
>+  };
>+
>+  this.match = function objAttrChangedChecker_match(aEvent)
>+  {
>+    if (aEvent instanceof nsIAccessibleObjectAttributeChangedEvent) {
>+      var scEvent = aEvent.QueryInterface(
>+        nsIAccessibleObjectAttributeChangedEvent);
>+      return (aEvent.accessible == getAccessible(this.target)) &&
>+        (scEvent.changedAttribute.toString() == aAttr);
>+    }
>+    return false;
>+  };
>+}
>+
>+/**
>  * State change checker.
>  */
> function stateChangeChecker(aState, aIsExtraState, aIsEnabled,
>                             aTargetOrFunc, aTargetFuncArg, aIsAsync,
>                             aSkipCurrentStateCheck)
> {
>   this.__proto__ = new invokerChecker(EVENT_STATE_CHANGE, aTargetOrFunc,
>                                       aTargetFuncArg, aIsAsync);
>diff --git a/accessible/tests/mochitest/events/test_aria_objattr.html b/accessible/tests/mochitest/events/test_aria_objattr.html
>index b335a47..1410a53 100644
>--- a/accessible/tests/mochitest/events/test_aria_objattr.html
>+++ b/accessible/tests/mochitest/events/test_aria_objattr.html
>@@ -15,35 +15,34 @@

Comment 6

[Yura Zenevich [:yzen] (please use "needinfo")]

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=b68778d3aa16

Comment 7

[Yura Zenevich [:yzen] (please use "needinfo")]

alternatively: https://tbpl.mozilla.org/?tree=Try&rev=b68778d3aa16

Comment 8

[Yura Zenevich [:yzen] (please use "needinfo")]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ad8901b59cd0

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ad8901b59cd0