725178 - get rid ensureAccessibleTree of common.js

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

tree is created automatically and no way to force its creation so ensureAccessibleTree makes nothing (http://mxr.mozilla.org/mozilla-central/source/accessible/tests/mochitest/common.js).

Comment 1

[Tobias (:Tobbi) Markus]

Attachment: Remove ensureAccessibleTree from common.js

patch v1

Comment 2

[Marco Zehe (:MarcoZ)]

Tobias, thanks for taking this!

I looked over your patch, and saw that you didn't take into account yet the instances where this function is actually being called from test files. Grep tells me there are at least three of them that rely on the behavior here. They obviously need to be looked at and adjusted accordingly.

Comment 3

[Tobias (:Tobbi) Markus]

I'll get right on it. I relied on the function only being called in itself. Obviously, I was mistaken. Will use MXR to search through the tree.

Comment 4

[Tobias (:Tobbi) Markus]

Attachment: patch v2

Removing function as well as callers

Comment 5

[alexander :surkov (:asurkov)]

Comment on attachment 595463 [details] [diff] [review]
patch v2

Review of attachment 595463 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with comment fixed

::: accessible/tests/mochitest/events/test_text.html
@@ -51,4 @@
>        this.invoke = function removeChildSpan_invoke()
>        {
>          // remove HTML span, a first child of the node
> -        ensureAccessibleTree(this.DOMNode);

it appears you didn't removed all instances (see removeChildren function)

Comment 6

[Marco Zehe (:MarcoZ)]

Comment on attachment 595463 [details] [diff] [review]
patch v2

>diff --git a/accessible/tests/mochitest/events/test_tree.xul b/accessible/tests/mochitest/events/test_tree.xul
>--- a/accessible/tests/mochitest/events/test_tree.xul
>+++ b/accessible/tests/mochitest/events/test_tree.xul
>@@ -129,11 +129,6 @@
>         gTreeBox.view = gView;
>       }
> 
>-      this.finalCheck = function setTreeView_finalCheck(aEvent)
>-      {
>-        ensureAccessibleTree(gTree);
>-      }
>-

So this gets removed without adding something that actually *does* a real check on the result of this test? I don't think this is right.

Comment 7

[alexander :surkov (:asurkov)]

(In reply to Marco Zehe (:MarcoZ) from comment #6)
> Comment on attachment 595463 [details] [diff] [review]
> patch v2
> 
> >diff --git a/accessible/tests/mochitest/events/test_tree.xul b/accessible/tests/mochitest/events/test_tree.xul
> >--- a/accessible/tests/mochitest/events/test_tree.xul
> >+++ b/accessible/tests/mochitest/events/test_tree.xul
> >@@ -129,11 +129,6 @@
> >         gTreeBox.view = gView;
> >       }
> > 
> >-      this.finalCheck = function setTreeView_finalCheck(aEvent)
> >-      {
> >-        ensureAccessibleTree(gTree);
> >-      }
> >-
> 
> So this gets removed without adding something that actually *does* a real
> check on the result of this test? I don't think this is right.

don't get confused by 'finalCheck' name, this invoker loads tree view, nothing else.

Comment 8

[alexander :surkov (:asurkov)]

landed with minor fix in events/test_tree.xul to make mochitest pass https://hg.mozilla.org/integration/mozilla-inbound/rev/dfe18a0ece02

Comment 9

[alexander :surkov (:asurkov)]

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