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
Summary: Port bug 1206133 to Thunderbird → Port bug 1206133 to Thunderbird - TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/message-header/test-message-header.js
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.
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).
Attachment #8796679 - Flags: review?(acelists) → review+
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.)
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 52.0
You need to log in before you can comment on or make changes to this bug.