TEST-UNEXPECTED-FAIL | /Users/cltbld/tasks/task_1547771879/build/tests/mozmill/folder-display/test-columns.js | test-columns.js::test_reset_to_inbox
TEST-UNEXPECTED-FAIL | /Users/cltbld/tasks/task_1547771879/build/tests/mozmill/folder-display/test-columns.js | test-columns.js::test_apply_to_folder_no_children
TEST-UNEXPECTED-FAIL | /Users/cltbld/tasks/task_1547771879/build/tests/mozmill/folder-display/test-columns.js | test-columns.js::test_apply_to_folder_and_children
TEST-UNEXPECTED-FAIL | /Users/cltbld/tasks/task_1547771879/build/tests/mozmill/folder-display/test-columns.js | test-columns.js::test_apply_to_folder_no_children_swapped
TEST-UNEXPECTED-FAIL | /Users/cltbld/tasks/task_1547771879/build/tests/mozmill/folder-display/test-columns.js | test-columns.js::test_apply_to_folder_and_children_swapped
TEST-UNEXPECTED-FAIL | /Users/cltbld/tasks/task_1547771879/build/tests/mozmill/folder-display/test-columns.js | test-columns.js::test_reset_columns_gloda_collection

M-C last good: 1bbec154fb73b0fa3756a9ed417815c87f
M-C first bad: 081c6ac45c5d2fb2d64465ec5d5f18771f

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1bbec154fb73b0fa3756a9ed417815c87f&tochange=081c6ac45c5d2fb2d64465ec5d5f18771f

https://taskcluster-artifacts.net/Iy9kycmbR5C8XRVtdoXLYQ/0/public/logs/live_backing.log
INFO - EXCEPTION: Popup never opened! id=, state=closed
INFO - at: utils.js line 396
INFO - TimeoutError utils.js:396 13
INFO - waitFor utils.js:452 11
INFO - _click_menus test-window-helpers.js:982 9
INFO - invoke_column_picker_option test-columns.js:361 3

Some stuff already broken in folder display, so I'll switch those off.

Since these tests are about columns, from bug 1507704.

https://searchfox.org/comm-central/source/mail/test/mozmill/folder-display/test-columns.js#357

Moment, I'll attach a likely patch.

Ouch, it's not just the test adjustment. We have https://searchfox.org/comm-central/source/mail/base/content/threadPaneColumnPicker.xml which now wouldn't work. The good news is we could just scrap that now and make a customzied built-in treecolpicker.

It's some hours of work to figure out properly though. I can look at it later today.

test-columns.js::test_reset_columns_gloda_collection is still failing, but other than that, this seems to work

For reference, this is the widget in the thread pane where you can select which columns to show.
The standard widget does not have an "Apply columns to..." menu item, which Thunderbird has, (re)implemented here.

(In reply to Arshad Khan [:arshad] from comment #7)

Comment on attachment 9037772 [details] [diff] [review]
bug1521016_colpicker.patch

Review of attachment 9037772 [details] [diff] [review]:

treecol-image is not showing.. Something is messing up.

Definitely showing for me. Can you recheck?

• ChromeUtils.import("resource://gre/modules/Services.jsm");

If I recall correctly, you prefer moving such imports to top of file? why is
that?

Pretty soon it will anyway become something like normal es imports. Those tend to be on top. Services is really global so there is no win trying to import it "conditionally" or late. It's always set. I'll move this to top too.

Re buildPopup and XXX - I intend to sumbit a patch for m-c to fix this method (the < 2 check) so it works for us too and we can get rid of this pointless override. The new code would look like the what i've included now.
I don't want to do any style changes in that patch, since they might not want that.

Refreshed, adjusted, and the mozmill test is now fine at least locally.
Sent to try just now: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=732fd86007024d5284979c65b84584ce5d63bb6f

I am getting errors like https://bugzilla.mozilla.org/show_bug.cgi?id=1515953 had for the elements that you have added. We should fix this first, wdys?

I've tested at the patch without looking at the code. I don't know what the treecol-image is but the column picker has certainly reappeared :-)

After enabling the tests again, four tests still fail. Note that three tests are disabled on Linux. The failing ones are:

SUMMARY-UNEXPECTED-FAIL | c:\mozilla-source\comm-central\comm\mail\test\mozmill\folder-display\test-columns.js | test-columns.js::test_apply_to_folder_and_children
EXCEPTION: Found visible column 'sizeCol' but was expecting 'tagsCol'!

SUMMARY-UNEXPECTED-FAIL | c:\mozilla-source\comm-central\comm\mail\test\mozmill\folder-display\test-columns.js | test-columns.js::test_apply_to_folder_no_children_swapped
EXCEPTION: Found visible column 'sizeCol' but was expecting 'undefined'!

SUMMARY-UNEXPECTED-FAIL | c:\mozilla-source\comm-central\comm\mail\test\mozmill\folder-display\test-columns.js | test-columns.js::test_apply_to_folder_and_children_swapped
EXCEPTION: Found visible column 'sizeCol' but was expecting 'undefined'!

SUMMARY-UNEXPECTED-FAIL | c:\mozilla-source\comm-central\comm\mail\test\mozmill\folder-display\test-columns.js | test-columns.js::test_reset_columns_gloda_collection
EXCEPTION: Found visible column 'accountCol' but was expecting 'locationCol'!

So not ready for prime time yet :-(

I would assume treecol-image missing is just due to bug 1515953 (which maybe is more prominent now).

I'll have to look more at reset, but I think it's basically due to different expectations: the toolkit widget will only reset column order, while ours used to reset state (order and which ones).

I would say we're alright with just resetting the column order and using the functionality toolkit already provides.

The test_reset_columns_gloda_collection test is somewhat nonsense now, but moving around the columns in the test requires more code which I don't have time to do atm.

I think we can solve the treecol-image not showing the icon on Mac issue later if that's still a problem.

(In reply to Arshad Khan [:arshad] from comment #16)

+var {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");

const can be used here?

Nope, the file is not a module so you'd get redeclaration errors.

Let's see what try says - https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=e74ac47f78992360edd22c4b477f952b32d17593

👎

Should be it, works at least locally.
Dunno why try doesn't want to cooperate atm.

Well, three tests are disabled on Linux:
Line 466: test_apply_to_folder_and_children.EXCLUDED_PLATFORMS = ['linux']; // See bug 1406717.
Line 513: test_apply_to_folder_no_children_swapped.EXCLUDED_PLATFORMS = ['linux']; // See bug 1406717.
Line 554: test_apply_to_folder_and_children_swapped.EXCLUDED_PLATFORMS = ['linux']; // See bug 1406717.

These three still fail in Windows. I'll land this now, but we still need to look into these failures.

(In reply to Magnus Melin [:mkmelin] from comment #21)

Dunno why try doesn't want to cooperate atm.

Because you're not at tip locally?

(In reply to Jorg K (GMT+1) from comment #23)

Because you're not at tip locally?

Indeed, that tree was not up to date. Too late for my brain to catch that apparently.

TB 66 beta:
https://hg.mozilla.org/releases/comm-beta/rev/86cb9d0c8982

I landed what we had after rebasing it due to the import changes that hit 67 but not 66. That also took care of the follow-up.

Magnus, can we please get the tests mentioned in comment #22 fixed.

Fix the test expectations, since reset now only resets the order, not to any defined state.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=1c9f70c6d6f9624a82885c68853c842b803813cc

Works fine at least locally, let's see if bug 1406717 is actually fixed

Great (if it works), thanks!

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/bdc549022762
adjust test expectations now that "reset" only resets order, not actual state. r=jorgk

TB 66 beta:
https://hg.mozilla.org/releases/comm-beta/rev/2d307b1bc4a3fb68e0b19f7b66c1bba2acb490f5