Disable a11y tests that put <tabbrowser> in random XUL documents and expect it to work

RESOLVED FIXED in mozilla12

Status

()

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

People

(Reporter: dao, Assigned: dao)

Tracking

(Blocks: 1 bug)

Trunk
mozilla12
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
<tabbrowser> was removed from toolkit a long time ago. It's expected to work in browser.xul and navigator.xul, respectively, together with the associated scripts. It's not supposed to work in random other contexts.

People wasted enough time on these tests. This needs to stop. In case somebody cares about them, they need to be rewritten as browser-chrome tests or something.
(Assignee)

Comment 1

5 years ago
Created attachment 586386 [details] [diff] [review]
patch
Assignee: nobody → dao
Status: NEW → ASSIGNED
I would request that these be disabled, but kept in the files so we can still properly refer to them when these tests get rewritten. Since these test important parts of accessibility code, we need to have the right sense of direction here on how to embed a web page that we can test on. CCing David and Alex.
I think we need to create a suite of browser chrome accessibility tests. Marco do you want to drive this?

(I'm fine with disabling our naughty tests, as long as we're active on an alternative)
(Assignee)

Comment 4

5 years ago
Created attachment 588848 [details] [diff] [review]
patch v2
Attachment #586386 - Attachment is obsolete: true
Attachment #588848 - Flags: review?(marco.zehe)
Comment on attachment 588848 [details] [diff] [review]
patch v2

r=me with this change, thanks!

Do you have a good suggestion how we can best test these UI pieces in the future? We need these tests obviously, and we need the environment that these tests need as a prerequisite, and which we thought xul:tabbrowser was there for.
Attachment #588848 - Flags: review?(marco.zehe) → review+
(Assignee)

Comment 6

5 years ago
https://developer.mozilla.org/en/Browser_chrome_tests is what we usually use for testing the browser front-end.
(Assignee)

Comment 7

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/dc636e517089
Target Milestone: --- → mozilla12
https://hg.mozilla.org/mozilla-central/rev/dc636e517089
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Comment 9

5 years ago
Comment on attachment 588848 [details] [diff] [review]
patch v2

Guys, I'm surprised you disable tests without keeping exact alternative in your mind. If the reason is that people waste time to keep these tests working then I wonder why we weren't said about that earlier and we are faced to the fact "people wasted enough time".

Please work out the solution and do not disable them, just fix. Especially when the fix is nearly trivial (see events/test_focus_browserui.xul as example).

r- until we *have* to disable them
Attachment #588848 - Flags: review-
(Assignee)

Comment 10

5 years ago
Ok, I'm telling you now, the tabbrowser binding isn't supposed to work in random contexts. This is a simple fact. We regularly add dependencies on browser.js & Co. and get bitten by these bogus tests every time. Keeping them as they are should not be considered an option. I also don't think it's reasonable to expect random browser devs to fix your tests.

Comment 11

5 years ago
I realize these tests misuse XUL and that brings problem to developers. But I wonder why you disable them rather than say us there's a problem and ask whether we can fix it in reasonable time. If tomorrow layout, toolkit or whatever else regress us then we'll spend hours for regression finding, debugging, fixing and etc and even worth we can ship release with regression because of these disabled tests.

Comment 12

5 years ago
ping, guys, any ongoing actions to fix that?
Blocks: 417472
(Assignee)

Comment 13

5 years ago
I'm not sure what exactly you're asking. Somebody needs to rewrite the tests. I don't feel that I'm responsible for that...

Comment 14

5 years ago
(In reply to Dão Gottwald [:dao] from comment #13)
> I'm not sure what exactly you're asking. Somebody needs to rewrite the
> tests. I don't feel that I'm responsible for that...

You don't since you're not familiar with a11y tests infrastructure I think. I hoped Marco can drive that since he approved tests disabling.

At least we need to have a bug and (ideally) get an assignee for that.
(Assignee)

Updated

5 years ago
Blocks: 719754

Comment 15

5 years ago
Comment on attachment 588848 [details] [diff] [review]
patch v2

>-          if (window.TabsOnTop)
>-            TabsOnTop.syncUI();
>-
>-          if (window.TabsInTitlebar)
>-            TabsInTitlebar.allowedBy("tabs-visible", visible);
>+          TabsOnTop.syncUI();
>+
>+          TabsInTitlebar.allowedBy("tabs-visible", visible);

Has this come from elsewhere?
(Assignee)

Comment 16

5 years ago
(In reply to John P Baker from comment #15)
> Comment on attachment 588848 [details] [diff] [review]
> patch v2
> 
> >-          if (window.TabsOnTop)
> >-            TabsOnTop.syncUI();
> >-
> >-          if (window.TabsInTitlebar)
> >-            TabsInTitlebar.allowedBy("tabs-visible", visible);
> >+          TabsOnTop.syncUI();
> >+
> >+          TabsInTitlebar.allowedBy("tabs-visible", visible);
> 
> Has this come from elsewhere?

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