get rid ensureAccessibleTree of common.js

RESOLVED FIXED in mozilla13

Status

()

Core
Disability Access APIs
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: surkov, Assigned: Tobbi)

Tracking

(Blocks: 1 bug, {access})

unspecified
mozilla13
access
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][mentor=surkov.alexander@gmail.com][lang=js])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
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).
(Assignee)

Comment 1

6 years ago
Created attachment 595410 [details] [diff] [review]
Remove ensureAccessibleTree from common.js

patch v1
Assignee: nobody → tobbi.bugs
Status: NEW → ASSIGNED
Attachment #595410 - Flags: review?(surkov.alexander)

Comment 2

6 years ago
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.
(Assignee)

Comment 3

6 years ago
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.
(Assignee)

Comment 4

6 years ago
Created attachment 595463 [details] [diff] [review]
patch v2

Removing function as well as callers
Attachment #595410 - Attachment is obsolete: true
Attachment #595410 - Flags: review?(surkov.alexander)
Attachment #595463 - Flags: review?(surkov.alexander)
(Reporter)

Comment 5

5 years ago
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)
Attachment #595463 - Flags: review?(surkov.alexander) → review+

Comment 6

5 years ago
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.
(Reporter)

Comment 7

5 years ago
(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.
(Reporter)

Comment 8

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

Comment 9

5 years ago
https://hg.mozilla.org/mozilla-central/rev/dfe18a0ece02
Target Milestone: --- → mozilla13
(Reporter)

Updated

5 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.