Looks like bug 1206133 changed popup states here: http://hg.mozilla.org/mozilla-central/rev/bca4534616ad#l5.9 This causes Mozmill failures here: https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=907ba80124fe9638e436f824e7f4aa11cb5198b5&selectedJob=48979 INFO - SUMMARY-UNEXPECTED-FAIL | test-message-header.js | test-message-header.js::test_address_book_switch_disabled_on_contact_in_mailing_list INFO - EXCEPTION: a != b: 'showing' != 'open'. INFO - at: test-folder-display-helpers.js line 2858 INFO - assert_true test-folder-display-helpers.js:2858 11 INFO - assert_equals test-folder-display-helpers.js:2845 3 INFO - test_address_book_switch_disabled_on_contact_in_mailing_list test-message-header.js:464 3
Created attachment 8796608 [details] [diff] [review] 1306630.patch OK, what happens is that instead of "open" we now get "showing". In total there are four states: "closed", "open", "showing" and "hidden". https://dxr.mozilla.org/comm-central/rev/9baec74b3db1bf005c66ae2f50bafbdb02c3be38/mozilla/layout/xul/PopupBoxObject.cpp#223 Let's be a little more flexible and cater for "open" and "showing" as being open. With this patch the test passes on current trunk.
Where is the "positioning" state? Or is that just an event? Also, why should we consider "showing" as being open, in this particular test only? We often get popups stuck in "showing" state on Linux for infinite length of time and the test fails. But those problems are intermittent so we attribute them to either mozmill, or some Linux bug in Gtk/XUL backend or something. So why is this test special?
Kirk, bug 1206133 broke one of our test. We now get the panel state returned as "showing" and not as "open" as it used to be. Most likely this is due to: http://hg.mozilla.org/mozilla-central/rev/bca4534616ad#l5.12 Is this expected?
Bug 1206133 changed panel behavior. Panels used to open synchronously; now they open asynchronously. This means that tests that do things like this: |panel.open(); assert(panel.state == "open");| now fail. The fix for this is typically to wait for the popupshown event.
Created attachment 8796679 [details] [diff] [review] 1306630-take2.patch
Comment on attachment 8796679 [details] [diff] [review] 1306630-take2.patch Review of attachment 8796679 [details] [diff] [review]: ----------------------------------------------------------------- Yes, this seems more correct. Please remove the now redundant assert_equals()s after the waitFor()s (3 occurrences in the patch).
Created attachment 8796751 [details] [diff] [review] 1306630-take2.patch Addressed review comments and removed those three lines.
https://hg.mozilla.org/comm-central/rev/ca4b376232c7 (pushed with DONTBUILD since we're waiting for bug 456053 to fix its test failures.)