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

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Dão Gottwald [:dao]]

<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.

Comment 1

[Dão Gottwald [:dao]]

Attachment: patch

Comment 2

[Marco Zehe (:MarcoZ)]

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.

Comment 3

[David Bolter [:davidb] (NeedInfo me for attention)]

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)

Comment 4

[Dão Gottwald [:dao]]

Attachment: patch v2

Comment 5

[Marco Zehe (:MarcoZ)]

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.

Comment 6

[Dão Gottwald [:dao]]

https://developer.mozilla.org/en/Browser_chrome_tests is what we usually use for testing the browser front-end.

Comment 7

[Dão Gottwald [:dao]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/dc636e517089

Comment 8

[Justin Wood (:Callek)]

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

Comment 9

[alexander :surkov (:asurkov)]

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

Comment 10

[Dão Gottwald [:dao]]

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

[alexander :surkov (:asurkov)]

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

[alexander :surkov (:asurkov)]

ping, guys, any ongoing actions to fix that?

Comment 13

[Dão Gottwald [:dao]]

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

[alexander :surkov (:asurkov)]

(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.

Comment 15

[John P Baker]

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?

Comment 16

[Dão Gottwald [:dao]]

(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