Closed
Bug 715857
Opened 13 years ago
Closed 13 years ago
Disable a11y tests that put <tabbrowser> in random XUL documents and expect it to work
Categories
(Core :: Disability Access APIs, defect)
Core
Disability Access APIs
Tracking
()
RESOLVED
FIXED
mozilla12
People
(Reporter: dao, Assigned: dao)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
MarcoZ
:
review+
surkov
:
review-
|
Details | Diff | Splinter Review |
<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•13 years ago
|
||
Assignee: nobody → dao
Status: NEW → ASSIGNED
Comment 2•13 years ago
|
||
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•13 years ago
|
||
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•13 years ago
|
||
Attachment #586386 -
Attachment is obsolete: true
Attachment #588848 -
Flags: review?(marco.zehe)
Comment 5•13 years ago
|
||
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•13 years ago
|
||
https://developer.mozilla.org/en/Browser_chrome_tests is what we usually use for testing the browser front-end.
Assignee | ||
Comment 7•13 years ago
|
||
Target Milestone: --- → mozilla12
Comment 8•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment 9•13 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•13 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•13 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.
Assignee | ||
Comment 13•13 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•13 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.
Comment 15•13 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•13 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.
Description
•