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.
Assignee: nobody → Olli.Pettay
Status: NEW → ASSIGNED
Attachment #330401 - Flags: review?(rflint)
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.
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).
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+
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: approval126.96.36.199?
Comment on attachment 330437 [details] [diff] [review] v2 Approved for 188.8.131.52, a=dveditz for release-drivers
Attachment #330437 - Flags: approval184.108.40.206? → approval220.127.116.11+
mozilla/browser/components/preferences/applications.js 1.46 mozilla/browser/components/preferences/tests/browser_bug410900.js 1.6
Component: Testing → File Handling
Product: Core → Firefox
QA Contact: testing → file.handling
Target Milestone: mozilla1.9.1a1 → Firefox 3.1a1
You need to log in before you can comment on or make changes to this bug.