Closed Bug 715857 Opened 9 years ago Closed 9 years ago
Disable a11y tests that put <tabbrowser> in random XUL documents and expect it to work
<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: 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)
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+
https://developer.mozilla.org/en/Browser_chrome_tests is what we usually use for testing the browser front-end.
Target Milestone: --- → mozilla12
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
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-
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.
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.
ping, guys, any ongoing actions to fix that?
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...
(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 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?
(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.