1457746 - TEST-UNEXPECTED-FAIL | /builds/worker/workspace/build/tests/mozmill/folder-display/test-columns.js | test-columns.js::test_reset_to_inbox and two more

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Jorg K (CEST = GMT+2)]

TEST-UNEXPECTED-FAIL | /builds/worker/workspace/build/tests/mozmill/folder-display/test-columns.js | test-columns.js::test_reset_to_inbox
TEST-UNEXPECTED-FAIL | /builds/worker/workspace/build/tests/mozmill/folder-display/test-columns.js | test-columns.js::test_apply_to_folder_no_children
TEST-UNEXPECTED-FAIL | /builds/worker/workspace/build/tests/mozmill/folder-display/test-columns.js | test-columns.js::test_reset_columns_gloda_collection


M-C last good: 8b2c1fc3d6c348f053fe33a478fa3b1ddb
M-C first bad: 08f68e2c892cadc4035ecbfbf3529f32d4
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8b2c1fc3d6c348f053fe33a478fa3b1ddb&tochange=08f68e2c892cadc4035ecbfbf3529f32d4

Log
https://taskcluster-artifacts.net/QikRAwawS9eGH3rAy3pbyw/0/public/logs/live_backing.log
says:
INFO -  SUMMARY-UNEXPECTED-FAIL | test-columns.js | test-columns.js::test_reset_to_inbox
INFO -    EXCEPTION: Popup never opened! id=, state=closed
INFO -      at: utils.js line 406
INFO -         TimeoutError utils.js:406 13
INFO -         waitFor utils.js:444 11
INFO -         _click_menus test-window-helpers.js:965 9
INFO -         invoke_column_picker_option test-columns.js:359 3

And there is:
INFO -  JavaScript error: chrome://messenger/content/threadPaneColumnPicker.xml, line 96: TypeError: popup.showPopup is not a function

Same error for the other subtests. Most likely due to bug 1446961:
https://hg.mozilla.org/mozilla-central/rev/5659ad69e145#l1.12

Aceman, one for you.

Comment 1

[Jorg K (CEST = GMT+2)]

Looks like there are at least two:
https://dxr.mozilla.org/comm-central/search?q=showPopup

mail/base/content/threadPaneColumnPicker.xml and chat/content/convbrowser.xml (ignoring im/).

Comment 2

[:aceman]

Attachment: 1457746.patch

Try run: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=49cf13eb35d767bdbde1194612eb599a16410c12

Comment 3

[Jorg K (CEST = GMT+2)]

Thanks. I'd love to approve and land this, but sadly I can't make any sense of dom/webidl/PopupBoxObject.webidl.

You must have derived the new parameters for openPopup() somehow, so rather me doing the work again and deriving what is necessary, can you please explain. Maybe some comments would be useful. Also, do we need to test this manually?

Comment 4

[:aceman]

I took the replacement values from https://hg.mozilla.org/mozilla-central/rev/e872787876a5 (e.g. https://hg.mozilla.org/mozilla-central/rev/e872787876a5#l17.12). And I also manually tested the popup from the column picker widget in the thread pane.

Comment 5

[Jorg K (CEST = GMT+2)]

OK, for the thread pane you're doing:
-          popup.showPopup(this, -1, -1, "popup", "bottomright", "topright");
+          popup.openPopup(this, "after_end");
which is exactly

https://hg.mozilla.org/mozilla-central/rev/e872787876a5#l17.12
-            popup.showPopup(this, -1, -1, "popup", "bottomright", "topright");
+            popup.openPopup(this, "after_end");

I have no idea what "after_end" or any of the others in dom/webidl/PopupBoxObject.webidl mean, but I assume M-C know what they're doing.

The other one is:
-            this._autoScrollPopup.showPopup(document.documentElement,
                                             event.screenX,
-                                            event.screenY,
-                                            "popup", null, null);

+            this._autoScrollPopup.openPopup(null, "after_start",
                                             event.screenX,
+                                            event.screenY);
which is what M-C does at:
https://hg.mozilla.org/mozilla-central/rev/e872787876a5#l16.12

OK, again, it makes zero sense to me. I'll stick an r+ onto it :-(

Comment 6

[Jorg K (CEST = GMT+2)]

Comment on attachment 8971858 [details] [diff] [review]
1457746.patch

r+ as per the previous comment.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/1b769dba70e1
port bug 1446961: migrate popupBoxObject.showPopup to openPopup. r=jorgk

Comment 8

[Jorg K (CEST = GMT+2)]

Neil, could you kindly explain the changes at
https://hg.mozilla.org/mozilla-central/rev/e872787876a5#l17.12 and
https://hg.mozilla.org/mozilla-central/rev/e872787876a5#l16.12

We copied them verbatim: https://hg.mozilla.org/comm-central/rev/1b769dba70e1

Patrick, could you tell us what the chat hunk is about and how to test it. Or please test it for us.

Comment 9

[Neil Deakin]

The autoscroll popup code is wrong -- see bug 1457763 and should be using openPopupAtScreen as I had forgotten than the obsolete showPopup had overloaded screen posiitoning.

The tree change just makes the popup aligned on its upper-right corner to the lower right corner of the button. Ideally, we'd remove the call to openPopup entirely and just change display="xul:menu" so it behaves like a menu, but that is a different bug.

Comment 10

[Jorg K (CEST = GMT+2)]

Aceman, can you please do a follow-up here. Bug 1457763 does:
-            this._autoScrollPopup.openPopup(null, "after_start", popupX, popupY);
+            this._autoScrollPopup.openPopupAtScreen(popupX, popupY);

So we need to replace this hunk:
https://hg.mozilla.org/comm-central/rev/1b769dba70e1#l1.13

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/58ef06d5ad9b
Follow-up: Fix convbrowser.xml as per bug 1457763 (suggestion by Neil Deakin). r=me DONTBUILD CLOSED TREE

Comment 12

[Jorg K (CEST = GMT+2)]

I've done it myself, I'd still like to know how to test it.