Random failure on Linux mozilla-central qm-centos5-03 / browser_bug410900.js

RESOLVED FIXED in Firefox 3.1a1

Status

()

defect
RESOLVED FIXED
11 years ago
7 years ago

People

(Reporter: smaug, Assigned: rflint)

Tracking

({fixed1.9.0.4, intermittent-failure})

Trunk
Firefox 3.1a1
x86
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Reporter

Description

11 years ago
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/preferences/tests/browser_bug410900.js | handlersView is present
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/preferences/tests/browser_bug410900.js | App handler list populated

Using timeout to see if window is opened and the document for it is loaded is
error prone. Would be better if load event could be used.
Reporter

Comment 1

11 years ago
Posted patch something like this (obsolete) — Splinter Review
Assignee: nobody → Olli.Pettay
Status: NEW → ASSIGNED
Attachment #330401 - Flags: review?(rflint)
Posted patch Bulletproofing (obsolete) — Splinter Review
The problem with listening for load on the window is that the content of the richlistbox we're testing is built on a timeout after http://mxr.mozilla.org/mozilla-central/source/browser/components/preferences/applications.xul#57 is fired, so it still leaves us somewhat susceptible to starting the tests before things are actually ready.

This patch changes the test to observe a topic we fire off once we're sure the contents of the pane are ready for poking. I've ifdef'd the observer bits in applications.js since I don't think this topic would be genuinely useful outside of a testing context; especially when there are much better ways to go about manipulating the handler list.
Assignee: Olli.Pettay → rflint
Attachment #330401 - Attachment is obsolete: true
Attachment #330436 - Flags: review?(gavin.sharp)
Attachment #330401 - Flags: review?(rflint)
I think you might as well unconditionally fire the notification, it could potentially be useful for extensions wanting to extend this prefpane, it's pretty cheap, and test-only code in general is to be avoided.

I would also leave "runTest" separate from observe() for legibility (and have observe just pass the window in).
Posted patch v2Splinter Review
Attachment #330436 - Attachment is obsolete: true
Attachment #330437 - Flags: review?(gavin.sharp)
Attachment #330436 - Flags: review?(gavin.sharp)
Comment on attachment 330437 [details] [diff] [review]
v2

>diff --git a/browser/components/preferences/applications.js b/browser/components/preferences/applications.js

>+      // Notify observers the the UI is now ready

"that the"
Attachment #330437 - Flags: review?(gavin.sharp) → review+
http://hg.mozilla.org/index.cgi/mozilla-central/rev/f91119d7f946

Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1a1
Version: unspecified → Trunk
Comment on attachment 330437 [details] [diff] [review]
v2

I'd like to get this test fix onto 1.9.0, but it touches non-test code, so asking for approval. The actual impact of the non-test code is pretty small.
Attachment #330437 - Flags: approval1.9.0.3?
Depends on: 455258
Comment on attachment 330437 [details] [diff] [review]
v2

Approved for 1.9.0.4, a=dveditz for release-drivers
Attachment #330437 - Flags: approval1.9.0.4? → approval1.9.0.4+
mozilla/browser/components/preferences/applications.js 1.46
mozilla/browser/components/preferences/tests/browser_bug410900.js 1.6
Keywords: fixed1.9.0.4
Component: Testing → File Handling
Product: Core → Firefox
QA Contact: testing → file.handling
Target Milestone: mozilla1.9.1a1 → Firefox 3.1a1
Whiteboard: [orange]
Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.