Closed Bug 725565 Opened 13 years ago Closed 13 years ago

update DOMi since nsIAccessNode interface was removed

Categories

(Other Applications :: DOM Inspector, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: surkov, Assigned: surkov)

References

Details

Attachments

(1 file, 1 obsolete file)

nsIAccessNode was removed in bug 672507, DOMi should be updated
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #595652 - Flags: review?(neil)
comment:
QI nsIAccessible interface, used for compatibility with Gecko versions prior
 * to Gecko13.

was updated to:

QI nsIAccessNode interface if any, used for compatibility with Gecko versions
 * prior to Gecko13.
Comment on attachment 595652 [details] [diff] [review]
patch

> const gAccInterfaces =
> [
>   Ci.nsIAccessible,
>   Ci.nsIAccessibleDocument,
>   Ci.nsIAccessibleEditableText,
>   Ci.nsIAccessibleHyperLink,
>   Ci.nsIAccessibleHyperText,
>   Ci.nsIAccessibleImage,
>   Ci.nsIAccessibleSelectable,
>   Ci.nsIAccessibleTable,
>   Ci.nsIAccessibleTableCell,
>   Ci.nsIAccessibleText,
>-  Ci.nsIAccessibleValue,
>-  Ci.nsIAccessNode,
>+  Ci.nsIAccessibleValue
> ];
In this case, I think it would look better if you pushed Ci.nsIAccessNode on to the end of the array if it exists (same for evalJSDialog.js).

>+  if ("nsIAccessNode" in Ci) {
>+    return XPCU.QI(aAccessible, Ci.nsIAccessNode);
>+  }
>+  return aAccessible;
Nit: use a ?: expression for these. r=me with that fixed.

>diff --git a/resources/content/viewers/accessibleProps/accessiblePropViewerMgr.js b/resources/content/viewers/accessibleProps/accessiblePropViewerMgr.js
...
>+  var accessNode = QIAccessNode(this.getAccessible(aRow));
...
>diff --git a/resources/content/viewers/accessibleProps/accessibleProps.js b/resources/content/viewers/accessibleProps/accessibleProps.js
>+function QIAccessNode(aAccessible)
This looks confusing, but I guess it's OK.

>-  var accessNode = this.mTargets.queryElementAt(aRow, nsIAccessNode);
Another possibility:
const nsIAccessNode = "nsIAccessNode" in Components.interfaces ?
    Components.interfaces.nsIAccessNode : nsIAccessible;
Or even:
const nsIAccessNode = Components.interfaces.nsIAccessNode || nsIAccessible;
Attachment #595652 - Flags: review?(neil) → review+
Attached patch patch2Splinter Review
Neil, please quick check if your comments were addressed
Attachment #595652 - Attachment is obsolete: true
Attachment #596049 - Flags: review?(neil)
Comment on attachment 596049 [details] [diff] [review]
patch2

>diff --git a/resources/content/viewers/accessibleRelations/accessibleRelations.js b/resources/content/viewers/accessibleRelations/accessibleRelations.js
...
>+function QIAccessNode(aAccessible)
Not used, because of the "new" nsIAccessNode const. (You might want to move the comment to the const though.)

>diff --git a/resources/content/viewers/accessibleTree/evalJSDialog.js b/resources/content/viewers/accessibleTree/evalJSDialog.js
...
>+function QIAccessNode(aAccessible)
Not used, because you push Ci.nsIAccessNode into gAccInterfaces.

r=me with those removed.
Attachment #596049 - Flags: review?(neil) → review+
landed with Neil's comments addressed http://hg.mozilla.org/dom-inspector/rev/0bc5a6d7a31a
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: