Last Comment Bug 725178 - get rid ensureAccessibleTree of common.js
: get rid ensureAccessibleTree of common.js
Status: RESOLVED FIXED
[good first bug][mentor=surkov.alexan...
: access
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla13
Assigned To: [retired]
:
Mentors:
Depends on:
Blocks: a11ytestdev
  Show dependency treegraph
 
Reported: 2012-02-07 17:23 PST by alexander :surkov
Modified: 2012-02-11 18:13 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Remove ensureAccessibleTree from common.js (948 bytes, patch)
2012-02-08 07:55 PST, [retired]
no flags Details | Diff | Splinter Review
patch v2 (2.91 KB, patch)
2012-02-08 11:16 PST, [retired]
surkov.alexander: review+
Details | Diff | Splinter Review

Description alexander :surkov 2012-02-07 17:23:12 PST
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 [retired] 2012-02-08 07:55:02 PST
Created attachment 595410 [details] [diff] [review]
Remove ensureAccessibleTree from common.js

patch v1
Comment 2 Marco Zehe (:MarcoZ) on PTO until August 15 2012-02-08 08:38:24 PST
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 [retired] 2012-02-08 08:40:21 PST
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 [retired] 2012-02-08 11:16:04 PST
Created attachment 595463 [details] [diff] [review]
patch v2

Removing function as well as callers
Comment 5 alexander :surkov 2012-02-09 02:11:36 PST
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) on PTO until August 15 2012-02-09 03:15:11 PST
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 2012-02-09 03:17:59 PST
(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 2012-02-10 19:25:04 PST
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 2012-02-11 18:13:27 PST
https://hg.mozilla.org/mozilla-central/rev/dfe18a0ece02

Note You need to log in before you can comment on or make changes to this bug.