1617887 - dismantle mozmill.jsm and friends

Status: RESOLVED FIXED

(Thunderbird :: Testing Infrastructure, task)

Description

[Magnus Melin [:mkmelin]]

Inspired by the recent bustage, I notices mozmill.jsm contains mostly junk nowadays. Most of the stuff isn't used, and it even exports some things that do not exist :/
Time to clean out.

Comment 1

[Magnus Melin [:mkmelin]]

Attachment: bug1617887_dismantle_mozmil.part1.patch

Comment 2

[Magnus Melin [:mkmelin]]

Attachment: bug1617887.part2.patch

Comment 3

[Magnus Melin [:mkmelin]]

Attachment: bug1617887.part3.patch

Comment 4

[Magnus Melin [:mkmelin]]

Attachment: bug1617887.part4.patch

Comment 5

[Magnus Melin [:mkmelin]]

Attachment: bug1617887.part5.patch

Comment 6

[Magnus Melin [:mkmelin]]

Attachment: bug1617887.part6.patch

Comment 7

[Magnus Melin [:mkmelin]]

Attachment: bug1617887.part7.patch

Comment 8

[Magnus Melin [:mkmelin]]

Attachment: bug1617887.part8.patch

Comment 9

[Magnus Melin [:mkmelin]]

Attachment: bug1617887.part9.patch

Successful tryrun (have fixed the linting after) - https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=039ed6d33065f320334af488e647d4493bcfc484&selectedJob=290575559

It's almost entirely removing unused things.

Comment 10

[Khushil Mistry [:khushil324]]

Comment on attachment 9129399 [details] [diff] [review]
bug1617887_dismantle_mozmil.part1.patch

Review of attachment 9129399 [details] [diff] [review]:
-----------------------------------------------------------------

Everything else looks good. But we are using "platform" export here: https://searchfox.org/comm-central/source/mail/test/browser/shared-modules/frame.jsm#559
Is this of any use?

Comment 11

[Khushil Mistry [:khushil324]]

Comment on attachment 9129400 [details] [diff] [review]
bug1617887.part2.patch

Review of attachment 9129400 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. r=khushil. One question here. Can we replace all the controller i.e. mozmill.getMail3PaneController(); with mc? It will make things less confusing.

Comment 12

[Khushil Mistry [:khushil324]]

Comment on attachment 9129401 [details] [diff] [review]
bug1617887.part3.patch

Review of attachment 9129401 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. r=khushil.

Comment 13

[Khushil Mistry [:khushil324]]

Comment on attachment 9129402 [details] [diff] [review]
bug1617887.part4.patch

Review of attachment 9129402 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. r=khushil.

Comment 14

[Khushil Mistry [:khushil324]]

Comment on attachment 9129403 [details] [diff] [review]
bug1617887.part5.patch

Review of attachment 9129403 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. r=khushil

Comment 15

[Khushil Mistry [:khushil324]]

Comment on attachment 9129404 [details] [diff] [review]
bug1617887.part6.patch

Review of attachment 9129404 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. r=khushil

Comment 16

[Khushil Mistry [:khushil324]]

Comment on attachment 9129405 [details] [diff] [review]
bug1617887.part7.patch

Review of attachment 9129405 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. r=khushil

Comment 17

[Khushil Mistry [:khushil324]]

Comment on attachment 9129406 [details] [diff] [review]
bug1617887.part8.patch

Review of attachment 9129406 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. r=khushil

Comment 18

[Khushil Mistry [:khushil324]]

Comment on attachment 9129408 [details] [diff] [review]
bug1617887.part9.patch

Review of attachment 9129408 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. r=khushil

Comment 19

[Magnus Melin [:mkmelin]]

Attachment: bug1617887_dismantle_mozmil.part1.patch

Got rid of mozmill.platform calls. Also force_skip and EXCLUDED_PLATFORMS which we don't use in Mochitests.

Just sent to try: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=32f7b87d548d426ff36474f9a642913c2c19bdd9

Comment 20

[Khushil Mistry [:khushil324]]

Comment on attachment 9131446 [details] [diff] [review]
bug1617887_dismantle_mozmil.part1.patch

Review of attachment 9131446 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. r=khushil.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/bbcd9eee9226
part1 - remove unsed mozmill exports. r=khushil
https://hg.mozilla.org/comm-central/rev/d0a04e52dbd5
part2 - remove unnecessary getMail3PaneController. r=khushil
https://hg.mozilla.org/comm-central/rev/d5f2f34c0d01
part3 - stop exporting mozmill.controller. r=khushil
https://hg.mozilla.org/comm-central/rev/db70bd44f6e2
part4 - stop exporting mozmill.utils. r=khushil
https://hg.mozilla.org/comm-central/rev/fb950872cc0f
part5 - inline mozmill init.jsm. r=khushil
https://hg.mozilla.org/comm-central/rev/163fe0f39164
part6 - remove unused dom.jsm. r=khushil
https://hg.mozilla.org/comm-central/rev/89e4f92cad8f
part6 - inline strings.jsm usage. r=khushil
https://hg.mozilla.org/comm-central/rev/71e02e54a7f3
part8 - remove unused arrays.jsm. r=khushil
https://hg.mozilla.org/comm-central/rev/9979881cf01c
part9 - remove jum.jsm usage. r=khushil DONTBUILD

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/50fa3d8d13ff
part1 followup - assign module.mozmill late. rs=bustage-fix

Comment 23

[Magnus Melin [:mkmelin]]

Leaving open for some more removals.

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/22c665f600c7
part 1 follow-up - Fix tests that depend on isMac or isWindows. rs=bustage-fix

Comment 25

[Magnus Melin [:mkmelin]]

Attachment: bug1617887_rm_MessageHelpers.patch

Perhaps not strictly a mozmill one... but this one can also go.

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/9f5657fd0e4e
get rid of MessageHelpers.jsm. r=darktrojan

Comment 27

[Magnus Melin [:mkmelin]]

Attachment: bug1617887_rm_load_via_src_path.patch

This is a but yucky. I don't know why CI uses a different path than local runs. Anyway...

Comment 28

[Magnus Melin [:mkmelin]]

Attachment: bug1617887_less_osjsm.patch

Comment 29

[Magnus Melin [:mkmelin]]

Attachment: bug1617887_rm_runner.patch

Successful try with the this and the last two patches: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=f066f58f17f71a0414c3aaf5666cb58d505c2ad0

Comment 30

[Khushil Mistry [:khushil324]]

Comment on attachment 9145073 [details] [diff] [review]
bug1617887_less_osjsm.patch

Review of attachment 9145073 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. r=khushil.

Comment 31

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

Comment on attachment 9145072 [details] [diff] [review]
bug1617887_rm_load_via_src_path.patch

Review of attachment 9145072 [details] [diff] [review]:
-----------------------------------------------------------------

We could probably take this improvement further, but it's a good start. I think the use of the term "module" is misleading (these days, it probably made sense at the time) and should be replaced with something like "scope".

::: mail/test/browser/shared-modules/FolderDisplayHelpers.jsm
@@ +174,5 @@
>  var { MailUtils } = ChromeUtils.import("resource:///modules/MailUtils.jsm");
>  var { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
> +var { MessageGenerator, MessageScenarioFactory } = ChromeUtils.import(
> +  "resource://testing-common/mailnews/MessageGenerator.jsm"
> +);

It didn't occur to me when I first converted all of this stuff, but we should use ChromeUtils.defineModuleGetter instead of ChromeUtils.import within the helper modules, so that we don't load a lot of things we don't need for every test.

@@ +3383,4 @@
>  
>  /**
> + * Load a file in its own 'module' based on the effective location of the staged
> + * the FolderDisplayHelpers.jsm module

There's one too many "the"s in this sentence, and it's missing the full stop.

Comment 32

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

Comment on attachment 9145075 [details] [diff] [review]
bug1617887_rm_runner.patch

Review of attachment 9145075 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/test/browser/shared-modules/frame.jsm
@@ +40,5 @@
>  var arrayRemove = function(array, from, to) {
>    var rest = array.slice((to || from) + 1 || array.length);
>    array.length = from < 0 ? array.length + from : from;
>    return array.push.apply(array, rest);
>  };

It's probably about time we used .splice!

Comment 33

[Magnus Melin [:mkmelin]]

(In reply to Geoff Lankow (:darktrojan) from comment #31)

It didn't occur to me when I first converted all of this stuff, but we
should use ChromeUtils.defineModuleGetter instead of ChromeUtils.import
within the helper modules, so that we don't load a lot of things we don't
need for every test.

Will save this for later. There's a lot of things to clean up yet. Taking things out one at a time helps seeing what's left.

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/89648b88a0e3
Rework load_via_src_path. r=darktrojan
https://hg.mozilla.org/comm-central/rev/ad8b33dcd11e
remove unused (mozmill) os.jsm imports. r=khushil
https://hg.mozilla.org/comm-central/rev/6e0dee99d40a
remove Runner and other things from frame.jsm. r=darktrojan DONTBUILD

Comment 35

[Magnus Melin [:mkmelin]]

Attachment: bug1617887_rm_osjsm.patch

The mozmill os.jsm is now unused and can be removed.

Comment 36

[Khushil Mistry [:khushil324]]

Comment on attachment 9146734 [details] [diff] [review]
bug1617887_rm_osjsm.patch

Review of attachment 9146734 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. r=khushil

Comment 37

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/92b5978bce31
remove now unused mozmill os.jsm. r=khushil

Comment 38

[Magnus Melin [:mkmelin]]

Attachment: bug1617887_utils_export_less.patch

Small removal of exports - ok try at https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=c2da40145f09aa029b8d2f6afc2fc09c2b9b5dfc

Comment 39

[Khushil Mistry [:khushil324]]

Comment on attachment 9155742 [details] [diff] [review]
bug1617887_utils_export_less.patch

Review of attachment 9155742 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. r=khushil

::: mail/test/browser/shared-modules/controller.jsm
@@ -39,5 @@
>  // ***** END LICENSE BLOCK *****
>  
> -var EXPORTED_SYMBOLS = [
> -  "MozMillController",
> -  "MozMillAsyncTest",

MozMillAsyncTest related functions can be removed now.

Comment 40

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/c6b42eb37520
remove exports never used from utils.jsm. r=khushil DONTBUILD

Comment 41

[Magnus Melin [:mkmelin]]

Attachment: bug1617887_rm_MozMillAsyncTest.patch

Comment 42

[Khushil Mistry [:khushil324]]

Comment on attachment 9156517 [details] [diff] [review]
bug1617887_rm_MozMillAsyncTest.patch

Review of attachment 9156517 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me.

Comment 43

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/d204dd91ce6a
remove MozMillAsyncTest. r=khushil DONTBUILD

Comment 44

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/5b03f99338e2
followup to fix linting. rs=eslint DONTBUILD

Comment 45

[Magnus Melin [:mkmelin]]

Attachment: bug1617887_rm_controllerAdditions.patch

Another small removal. Ok try at https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=90bf33b687ee2bc5ef42a5869c03f87b46d2ff2f

Comment 46

[Khushil Mistry [:khushil324]]

Comment on attachment 9160284 [details] [diff] [review]
bug1617887_rm_controllerAdditions.patch

Look good to me. r=khushil.
We can also remove this comment: https://searchfox.org/comm-central/source/mail/test/browser/shared-modules/WindowHelpers.jsm#1740

Comment 47

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/ee5eb95ccf70
remove controllerAdditions and friends. r=khushil

Comment 48

[Magnus Melin [:mkmelin]]

Attachment: bug1617887_rm_controller_keypress.patch

Remove controller keypress

Comment 49

[Magnus Melin [:mkmelin]]

Attachment: bug1617887_rm_controller_junk.patch

Remove more old just from the controller.
Successful try for these two: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=774d559e0ab797999b6098cd5262988a65060e45

Comment 50

[Khushil Mistry [:khushil324]]

Comment on attachment 9182197 [details] [diff] [review]
bug1617887_rm_controller_junk.patch

Review of attachment 9182197 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. r=khushil

Comment 51

[Khushil Mistry [:khushil324]]

Comment on attachment 9182196 [details] [diff] [review]
bug1617887_rm_controller_keypress.patch

Review of attachment 9182196 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. r=khushil
comm/mail/test/browser/openpgp/browser_viewMessage.js Mochitest failure in the try-run. Any problem? This test is passing on the trunk on the local machine.

Comment 52

[Magnus Melin [:mkmelin]]

The mail/test/browser/openpgp/browser_viewMessage.js Mochitest failure appears to be a failure for artifact builds.
I got it for https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=d8742fec219a175fa4eb5f7e2a1bcf974c4a3290 too where these patches weren't applied

Comment 53

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/2a37c78166b8
stop using controller keypress, use EventUtils.synthesizeKey instead.  r=khushil
https://hg.mozilla.org/comm-central/rev/610cf5c11a00
remove from mozmill controller: renove assertPropertyNotExist, assertDOMProperty, assertNotDOMProperty, assertProperty, assertNotJSProperty, assertImageLoaded, mouseMove, dragToElement, Tabs. r=khushil DONTBUILD

Comment 54

[Magnus Melin [:mkmelin]]

Attachment: bug1617887_rm_mozmillcontroller_assertJSProperty.patch

Good try for the patches I'm going to attach: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=1b303a6bd9eb81e59bce41532daa8c22c4c00168&selectedTaskRun=WEskS8RaQXajGs45v6jitw.0

Comment 55

[Magnus Melin [:mkmelin]]

Attachment: bug1617887_rm_mozmillcontroller_assertChecked.patch

Comment 56

[Magnus Melin [:mkmelin]]

Attachment: bug1617887_rm_mozmillcontroller_assert.patch

Comment 57

[Magnus Melin [:mkmelin]]

Attachment: bug1617887_rm_mozmillcontroller_assertValue.patch

Comment 58

[Magnus Melin [:mkmelin]]

Attachment: bug1617887_rm_mozmillcontroller_assertNode.patch

Comment 59

[Magnus Melin [:mkmelin]]

Attachment: bug1617887_rm_mozmillcontroller_assertText.patch

Comment 60

[Magnus Melin [:mkmelin]]

Attachment: bug1617887_rm_mozmillcontroller_browserNav.patch

Comment 61

[Magnus Melin [:mkmelin]]

Attachment: bug1617887_rm_mozmillcontroller_keypress.patch

Comment 62

[Khushil Mistry [:khushil324]]

Comment on attachment 9182272 [details] [diff] [review]
bug1617887_rm_mozmillcontroller_assertJSProperty.patch

Review of attachment 9182272 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. r=khushil

Comment 63

[Khushil Mistry [:khushil324]]

Comment on attachment 9182273 [details] [diff] [review]
bug1617887_rm_mozmillcontroller_assertChecked.patch

Review of attachment 9182273 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. r=khushil

Comment 64

[Khushil Mistry [:khushil324]]

Comment on attachment 9182274 [details] [diff] [review]
bug1617887_rm_mozmillcontroller_assert.patch

Review of attachment 9182274 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. r=khushil

Comment 65

[Khushil Mistry [:khushil324]]

Comment on attachment 9182275 [details] [diff] [review]
bug1617887_rm_mozmillcontroller_assertValue.patch

Review of attachment 9182275 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. r=khushil

Comment 66

[Khushil Mistry [:khushil324]]

Comment on attachment 9182276 [details] [diff] [review]
bug1617887_rm_mozmillcontroller_assertNode.patch

Review of attachment 9182276 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. r=khushil

Comment 67

[Khushil Mistry [:khushil324]]

Comment on attachment 9182277 [details] [diff] [review]
bug1617887_rm_mozmillcontroller_assertText.patch

Review of attachment 9182277 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. r=khushil

Comment 68

[Khushil Mistry [:khushil324]]

Comment on attachment 9182278 [details] [diff] [review]
bug1617887_rm_mozmillcontroller_browserNav.patch

Review of attachment 9182278 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. r=khushil

Comment 69

[Khushil Mistry [:khushil324]]

Comment on attachment 9182279 [details] [diff] [review]
bug1617887_rm_mozmillcontroller_keypress.patch

Review of attachment 9182279 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. r=khushil

Comment 70

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/6a74db5212f9
remove last MozMillController.keypress usage. r=khushil
https://hg.mozilla.org/comm-central/rev/9fb691df9ec1
remove MozMillController.assertJSProperty. r=khushil
https://hg.mozilla.org/comm-central/rev/15b4d2bb4323
remove MozMillController.assertChecked and assertNotChecked and assertSelected. r=khushil
https://hg.mozilla.org/comm-central/rev/b3b13922a477
remove MozMillController.assert. r=khushil
https://hg.mozilla.org/comm-central/rev/88d60e6dfa79
remove MozMillController.assertValue. r=khushil
https://hg.mozilla.org/comm-central/rev/4c6b712bd109
remove MozMillController.assertNode. r=khushil
https://hg.mozilla.org/comm-central/rev/0fc810de9557
remove MozMillController.assertText. r=khushil
https://hg.mozilla.org/comm-central/rev/198a2372801e
remove MozMillController browser navigation functions, startUserShutdown, fireEvent, waitForImage, mouseDown, mouseOut, mouseOver, mouseUp, middleClick. r=khushil DONTBUILD

Comment 71

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/2818b57bf5df
follouwp - remove newline. rs=prettier DONTBUILD

Comment 72

[Magnus Melin [:mkmelin]]

Attachment: bug1617887_rm_controller.waitThenClick.patch

Successful try: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=9b6602f9ad1d62a40e148060918cf7e6b57492a1

Comment 73

[Khushil Mistry [:khushil324]]

Comment on attachment 9184055 [details] [diff] [review]
bug1617887_rm_controller.waitThenClick.patch

Review of attachment 9184055 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. r=khushil

Comment 74

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/7cde31af2652
remove mozmill controller.waitForEvents. r=khushil

Comment 75

[Magnus Melin [:mkmelin]]

Attachment: bug1617887_rm_controller.waitForEvents.patch

Accidentally pushed this instead of the patch reviewed. Going to leave it in since it's just removing unused code (but unfortunately not all, will have to push a followup.)

Comment 76

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/ae0046aeb61d
remove the rest of mozmillcontroller.waitForEvents. rs=me,eslint DONTBUILD

Comment 77

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/b0790ab438f5
remove mozmill controller.waitThenClick(). r=khushil DONTBUILD

Comment 78

[Magnus Melin [:mkmelin]]

Attachment: bug_1617887_remove_mozmill_controller_mouseevent_external_usage_r.patch

Successful try: https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=7ff84c47de8e885ed51bdfd1c76c04e1823cce13

This test was actually disabled everywhere due to the = vs == typo. Comments said it would be a windows only thing bug it works (and AFAICR, always has) on linux too.

Comment 79

[Magnus Melin [:mkmelin]]

Attachment: bug1617887_mozmill_rm_frame.patch

Noisy but successful try: https://treeherder.mozilla.org/jobs?repo=try-comm-central&selectedTaskRun=QDeWhmxNRzuhkGSS8NEnxw.0&revision=62080583d61cfc68c1015872fea12a121f8a0679

Comment 80

[Khushil Mistry [:khushil324]]

Comment on attachment 9184601 [details] [diff] [review]
bug1617887_rm_controller.waitForEvents.patch

Review of attachment 9184601 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. r=khushil

Comment 81

[Khushil Mistry [:khushil324]]

Comment on attachment 9185124 [details] [diff] [review]
bug1617887_mozmill_rm_frame.patch

Review of attachment 9185124 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. r=khushil

Comment 82

[Khushil Mistry [:khushil324]]

Comment on attachment 9185123 [details] [diff] [review]
bug_1617887_remove_mozmill_controller_mouseevent_external_usage_r.patch

Review of attachment 9185123 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. r=khushil

Comment 83

[Khushil Mistry [:khushil324]]

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

Created attachment 9185123 [details] [diff] [review]
bug_1617887_remove_mozmill_controller_mouseevent_external_usage_r.patch

Successful try: https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=7ff84c47de8e885ed51bdfd1c76c04e1823cce13

This test was actually disabled everywhere due to the = vs == typo. Comments said it would be a windows only thing bug it works (and AFAICR, always has) on linux too.

Here, the name of the patch and commit message are wrong.

Comment 84

[Magnus Melin [:mkmelin]]

I think it's basically correct (that was the motivation) but I'll update and clarify the commit message.

Comment 85

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/69dc70d94272
remove mozmill controller.mouseEvent external usage and make browser_autohideMenubar.js work again. r=khushil

Comment 86

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/39f724bf34d6
remove mozmill frame.jsm. r=khushil DONTBUILD

Comment 87

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

Attachment: Bug 1617887 - Remove unused code from mozmill's elementslib. r?mkmelin

This patch also:

• uses the name elib everywhere elementslib.jsm is imported, which should make future changes easier.
• updates the remaining elementslib types to ES classes, removing the dependency on utils.jsm.

Comment 88

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

Attachment: Bug 1617887 - Remove unused functions from mozmill's utils.jsm. r?mkmelin

I've managed to remove the uses of getChromeWindow and getWindows so that they can also be removed.

Depends on D100536

Comment 89

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/6b11bdfcd126
Remove unused code from mozmill's elementslib. r=mkmelin
https://hg.mozilla.org/comm-central/rev/2be2d9ed0917
Remove unused functions from mozmill's utils.jsm. r=mkmelin

Comment 90

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

Attachment: Bug 1617887 - Stop using Mozmill components in ItemEditingHelpers.jsm

Comment 91

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/e72943fbcfb1
remove obsolete comment. rs=comment-only

Comment 92

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/0668f84b4621
Stop using Mozmill components in ItemEditingHelpers.jsm r=mkmelin

Comment 93

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - remove unused TraceHelper.jsm. r=henry

Stictly speaking perhaps not mozmill, but I think it has its root there. Anyway, unused.

Comment 94

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/07a2385a8248
remove unused TraceHelper.jsm. r=henry

Comment 95

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

Attachment: Bug 1617887 - Remove unused event loggers from WindowHelpers.jsm. r=mkmelin

Most of this is very verbose mouse and keyboard event logging that I'm sure nobody uses. I've also reorganised some old-school logging functions to tidy up the loading of WindowHelpers.jsm.

Comment 96

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

Attachment: Bug 1617887 - Remove plan/wait_for_observable_event. r=mkmelin

More stuff we have tidier ways of doing.

Comment 97

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/672c0c3aa7b7
Remove unused event loggers from WindowHelpers.jsm. r=mkmelin
https://hg.mozilla.org/comm-central/rev/93dba17b7c0b
Remove plan/wait_for_observable_event. r=mkmelin

Comment 98

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - remove mozmill events.jsm. r=#thunderbird-reviewers

Comment 99

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/ac662f6bca32
remove mozmill events.jsm. r=lasana

Comment 100

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - use EventUtils instead of mc.click() in mail/. r=#thunderbird-reviewers

Comment 101

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/4998d4d1b4f8
use EventUtils instead of mc.click() in mail/. r=thunderbird-reviewers,rjl

Comment 102

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

This latest patch has introduced a number of perma-fails on Mac.

Comment 103

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - follow-up to mc.click() conversions (4998d4d1b4f8). r=#thunderbird-reviewers

comm/mail/test/browser/subscribe/browser_newsFilter.js and comm/mail/test/browser/folder-display/browser_rightClickMiddleClickMessages.js are problem cases. Going to leave them using clic() for now.

Successful try: https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=bbc0e67393c89eb941026f2dada1f593ca7b89ae

Comment 104

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/9ca018e4b904
follow-up to mc.click() conversions (revision 4998d4d1b4f8). rs=bustage-fix

Comment 105

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - Remove some unused parts of MozmillController annotating. r=leftmostcat

Comment 106

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - Remove more unused parts of MozmillController. r=leftmostcat

Depends on D175751

Comment 107

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - Remove controller.doubleClick. r=leftmostcat

grep -rEl "([a-zA-Z]+).doubleClick\(([a-zA-Z]+)\)" --exclude-dir=.hg --exclude-dir=suite --include="*.js" --exclude=".*" . | xargs xargs sed -i -E "s/([a-zA-Z]+).doubleClick\(([a-zA-Z]+)\)/EventUtils.synthesizeMouseAtCenter(\2, { clickCount: 2 }, \2.ownerGlobal)/g"

Depends on D175752

Comment 108

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - Remove most of controller.rightClick. r=leftmostcat

grep -rEl "([a-zA-Z]+).rightClick\(([a-zA-Z]+)\)" --exclude-dir=.hg --exclude-dir=suite --include="*.js" --exclude=".*" . | xargs xargs sed -i -E 's/([a-zA-Z]+).rightClick\(([a-zA-Z]+)\)/EventUtils.synthesizeMouseAtCenter(\2, { type: "contextmenu", button: 2 }, \2.ownerGlobal)/g'

Depends on D175753

Comment 109

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - Remove controller.check. r=leftmostcat

grep -rEl "mc.check\(([a-zA-Z]+), ([a-zA-Z]+)\)" --exclude-dir=.hg --exclude-dir=suite --include="*.js" --exclude=".*" . | xargs xargs sed -i -E 's/mc.check\(([a-zA-Z]+), ([a-zA-Z]+)\)/EventUtils.synthesizeMouseAtCenter(\1, { }, \1.ownerGlobal)/g'

Depends on D175754

Comment 110

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - Remove controller.radio. r=leftmostcat

Depends on D175755

Comment 111

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - Remove controller.click. r=leftmostcat

grep -rEl "([a-zA-Z]+).click\((.+)\);" --exclude-dir=.hg --exclude-dir=suite --include="*.js" --include="*.jsm" --exclude=".*" . | xargs xargs sed -i -E 's/([a-zA-Z]+).click\((.+)\);/EventUtils.synthesizeMouseAtCenter(\2, { }, \2.ownerGlobal)/g'

... and some manual fixups

There were some oddities, like with controller.click() you could click some menus that weren't open.

Depends on D175756

Comment 112

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - Remove controller.type. r=leftmostcat

grep -rEl "([a-zA-Z]+).type\((.+), (.+)\);" --exclude-dir=.hg --exclude-dir=suite --exclude-dir=chat --include="*.js" --include="*.jsm" --exclude=".*" . | xargs xargs sed -i -E 's/([a-zA-Z]+)\.type\((.+), (.+)\);/\2.focus();EventUtils.sendString(\3, \1.window)/g'

Depends on D175757

Comment 113

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - Remove controller Menu, and fix some problem cases from earlier. r=leftmostcat

Removes the fake menu.
Unfortunately the earlier patches in this series may not apply completely cleanly. Some of the fixups needed are done in this patch, but should ideally be part of some other. It got difficult tracking which exactly everthing should go...

Depends on D175758

Comment 114

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/48022c07e424
Remove some unused parts of MozmillController annotating. r=leftmostcat
https://hg.mozilla.org/comm-central/rev/80a89b307a08
Remove more unused parts of MozmillController. r=leftmostcat
https://hg.mozilla.org/comm-central/rev/a15b0142e4ae
Remove controller.doubleClick. r=leftmostcat
https://hg.mozilla.org/comm-central/rev/81ce931d7b14
Remove most of controller.rightClick. r=leftmostcat
https://hg.mozilla.org/comm-central/rev/6bad45fbacb2
Remove controller.check. r=leftmostcat
https://hg.mozilla.org/comm-central/rev/fcc322c9bd4f
Remove controller.radio. r=leftmostcat
https://hg.mozilla.org/comm-central/rev/0fa704b68e63
Remove controller.click. r=leftmostcat
https://hg.mozilla.org/comm-central/rev/25702cbfb421
Remove controller.type. r=leftmostcat
https://hg.mozilla.org/comm-central/rev/87bed05abb8f
Remove controller Menu, and fix some problem cases from earlier. r=leftmostcat

Comment 115

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - Replace controller.sleep(0) with async setTimeout. r=leftmostcat

Leaving the usage inside modules out for now.
Also removing some sleeps that were not needed. Turns out the issues with tests using insert image dialog were resolved when making this work async (the changes in WindowHelpers.jsm)

grep -rEl "([a-zA-Z1-9]+).sleep(0);" --exclude-dir=.hg --exclude-dir=suite --exclude-dir=chat --include=".js" --exclude="." . | xargs xargs sed -i -E 's#([a-zA-Z1-9]+).sleep(0);#await new Promise(resolve => setTimeout(resolve));#g'

grep -rEl "([a-zA-Z1-9]+).sleep();" --exclude-dir=.hg --exclude-dir=suite --exclude-dir=chat --include=".js" --exclude="." . | xargs xargs sed -i -E 's#([a-zA-Z1-9]+).sleep();#await new Promise(resolve => setTimeout(resolve));#g'

Comment 116

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/cc823633f9bf
Replace controller.sleep(0) with async setTimeout. r=leftmostcat

Comment 117

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - Remove the rest of controller.rightClick as well as exported sleep. r=leftmostcat

Comment 118

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - remove controller.threadTree augmentation. r=leftmostcat

Depends on D176131

Comment 119

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - remove controller.folderTree augmentation. r=leftmostcat

Depends on D176132

Comment 120

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - remove controller.tabmail augmentation. r=leftmostcat

Depends on D176133

Comment 121

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - remove controller.contentPane augmentation. r=leftmostcat

Depends on D176134

Comment 122

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - remove controller.folderDisplay augmentation. r=leftmostcat

Depends on D176135

Comment 123

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - remove controller.currentFolder augmentation. r=leftmostcat

Depends on D176136

Comment 124

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - remove controller.dbView augmentation. r=leftmostcat

Depends on D176137

Comment 125

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/dd10de79cb21
Remove the rest of controller.rightClick as well as exported sleep. r=leftmostcat
https://hg.mozilla.org/comm-central/rev/6be3748fbaaa
remove controller.threadTree augmentation. r=leftmostcat
https://hg.mozilla.org/comm-central/rev/e89bb8d3c3b6
remove controller.folderTree augmentation. r=leftmostcat
https://hg.mozilla.org/comm-central/rev/ad959870e478
remove controller.tabmail augmentation. r=leftmostcat
https://hg.mozilla.org/comm-central/rev/b4469318d2da
remove controller.contentPane augmentation. r=leftmostcat
https://hg.mozilla.org/comm-central/rev/80b3c4477848
remove controller.folderDisplay augmentation. r=leftmostcat
https://hg.mozilla.org/comm-central/rev/fcb96161d5fd
remove controller.currentFolder augmentation. r=leftmostcat
https://hg.mozilla.org/comm-central/rev/3b8e733cb6ec
remove controller.dbView augmentation. r=leftmostcat

Comment 126

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/5c9fa1a2444c
follow-up to e89bb8d3c3b6956f0b9d037b60384e21153f4916. rs=bustage-fix

Comment 127

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - remove mark_action and mark_failure. r=leftmostcat

This isn't really used much and where it's used it mostly for code that should be replaced. Never found it particularly useful.

Comment 128

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - Move click_menus_in_sequence, close_popup_sequence, click_appmenu_in_sequence, click_through_appmenu to exported methods instead of controller augmentations. r=leftmostcat

Depends on D176852

Comment 129

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - Remove controller.e() and now unused augment_controller. r=leftmostcat

Depends on D176853

Comment 130

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - Remove controller.sleep usage in .js files. r=leftmostcat

Depends on D176854

Comment 131

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - change controller.sleep to use utils.sleep directly in jsms. r=leftmostcat

So that controller itself can be removed completely, soon.

Depends on D176855

Comment 132

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/b089cbf3f15a
remove mark_action and mark_failure. r=leftmostcat
https://hg.mozilla.org/comm-central/rev/27c62778d6d9
Move click_menus_in_sequence, close_popup_sequence, click_appmenu_in_sequence, click_through_appmenu to exported methods instead of controller augmentations. r=leftmostcat
https://hg.mozilla.org/comm-central/rev/ef7471a6e6f7
Remove controller.e() and now unused augment_controller. r=leftmostcat
https://hg.mozilla.org/comm-central/rev/8612a93eebcb
Remove controller.sleep usage in .js files. r=leftmostcat
https://hg.mozilla.org/comm-central/rev/e868323ae8cc
change controller.sleep to use utils.sleep directly in jsms. r=leftmostcat

Comment 133

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - use waitFor() directly from utils instead of through controller. r=leftmostcat

Comment 134

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/a7d95c9e566e
use waitFor() directly from utils instead of through controller. r=leftmostcat

Comment 135

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - Remove controller.jsm usage outside of WindowHelpers.jsm. r=leftmostcat

Comment 136

[Magnus Melin [:mkmelin]]

Attachment: Bug 1617887 - Use async waiting in delete_mail_marked_as_junk. r=leftmostcat

I don't think I found the solution for bug 1833594, but this a minor improvement.
The test in question doesn't work with --headless since select_shift_click_row does the wrong thing with that apparently.

Comment 137

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/3f4c4d3eeae2
Remove controller.jsm usage outside of WindowHelpers.jsm. r=leftmostcat

Comment 138

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/f76e2db23bc5
Use async waiting in delete_mail_marked_as_junk. r=leftmostcat

Comment 139

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

I'm going to finish this off.

Comment 140

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

Attachment: Bug 1617887 - Remove tests for unsupported custom folder tree features. r=#thunderbird-reviewers

We no longer support these things and have no intention of doing so.

Comment 141

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

Attachment: Bug 1617887 - Remove dead and little-used code from test helper modules. r=#thunderbird-reviewers

Depends on D189728

Comment 142

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

Attachment: Bug 1617887 - Stop referring to `mc.window` in tests. r=#thunderbird-reviewers

mc.window is the same as window in a test's scope. That's not true in modules so they are unchanged.

Depends on D189729

Comment 143

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

Attachment: Bug 1617887 - Remove unused global variables in tests. r=#thunderbird-reviewers

This was done with the help of ESLint's no-unused-vars rule. Not all unused variables were removed as are pieces of tests that should be fixed.

Depends on D189730

Comment 144

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

Attachment: Bug 1617887 - Make the WindowHelpers module return windows instead of controllers. r=#thunderbird-reviewers

... and get rid of Mozmill controllers at last!

This works without changing the calling code as a window's .window property is the window itself.
Although the calls will be fixed in subsequent patches.

Depends on D189731

Comment 145

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

Attachment: Bug 1617887 - Drop `.window` where variables now refer to windows not controllers. r=#thunderbird-reviewers

Depends on D189732

Comment 146

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

Attachment: Bug 1617887 - Stop using `mc` in tests. r=#thunderbird-reviewers

mc is equal to window in the scope of test files.

Depends on D189733

Comment 147

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

Next up:

• stop referring to things as controllers now that they're actually windows
• replace utils.jsm, KeyboardHelper.jsm, MouseEventHelper and MockObjectHelper functions with something more modern
• replace WindowWatcher with something more modern
• change the helper module packaging so it no longer has mozmill in the URLs
• close this bug

Comment 148

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/0095225ba55c
Remove tests for unsupported custom folder tree features. r=mkmelin
https://hg.mozilla.org/comm-central/rev/0d9b5536c72e
Remove dead and little-used code from test helper modules. r=mkmelin
https://hg.mozilla.org/comm-central/rev/6e17d7f4b301
Stop referring to mc.window in tests. r=mkmelin
https://hg.mozilla.org/comm-central/rev/3029c347bcfb
Remove unused global variables in tests. r=mkmelin
https://hg.mozilla.org/comm-central/rev/bbb87cc8d9e3
Make the WindowHelpers module return windows instead of controllers. r=mkmelin
https://hg.mozilla.org/comm-central/rev/a68d94336da5
Drop .window where variables now refer to windows not controllers. r=mkmelin
https://hg.mozilla.org/comm-central/rev/701e15e070c4
Stop using mc in tests. r=mkmelin

Comment 149

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

Attachment: Bug 1617887 - Stop calling window objects "controller". r=#thunderbird-reviewers

These things are not controllers, as there's no such thing any more, so let's stop calling them controllers.

Comment 150

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

Attachment: Bug 1617887 - Use more modern ways to mock XPCOM objects. r=#thunderbird-reviewers

Toolkit has better ways of replacing objects with our own. We should use them.

Depends on D190426

Comment 151

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

Attachment: Bug 1617887 - Make the functions for opening compose windows async. r=#thunderbird-reviewers

Depends on D190427

Comment 152

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

Attachment: Bug 1617887 - Make the functions for opening and closing windows async. r=#thunderbird-reviewers

Mozmill had some complicated ways of tracking windows as they opened and closed that involve timers and hacking the event loop.
We can do much better with Promises, and BrowserTestUtils already provides the tools we need.

Depends on D190428

Comment 153

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

Attachment: Bug 1617887 - Remove the remnants of Mozmill's utils.jsm. r=#thunderbird-reviewers

Mozmill's way of waiting for things to happen involved timers and spinning the event loop until it did what was wanted.
We have Promises now.

In this patch I've converted all the remaining uses of utils.waitFor and utils.sleep to use async functions,
except those in FolderDisplayHelpers.jsm, because there's so much there I put it in another patch.

Depends on D190429

Comment 154

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

Attachment: Bug 1617887 - Remove Mozmill's utils.jsm from FolderDisplayHelpers.jsm. r=#thunderbird-reviewers

Depends on D190430

Comment 155

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

(In reply to Geoff Lankow (:darktrojan) from comment #147)

• replace utils.jsm, KeyboardHelper.jsm, MouseEventHelper and MockObjectHelper functions with something more modern

I've decided to leave KeyboardHelper and MouseEventHelper as they are for now.

• change the helper module packaging so it no longer has mozmill in the URLs

I've decided to leave this until we ESMify these modules, as it involves hundreds of changes that will only need to be changed again when we go to ESM. This can happen in another bug later.

Comment 156

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/7d2ea3fa72e5
Stop calling window objects "controller". r=aleca
https://hg.mozilla.org/comm-central/rev/b397d9602eb2
Use more modern ways to mock XPCOM objects. r=aleca
https://hg.mozilla.org/comm-central/rev/856ee2ef5b09
Make the functions for opening compose windows async. r=aleca
https://hg.mozilla.org/comm-central/rev/59ebb9249c1d
Make the functions for opening and closing windows async. r=aleca
https://hg.mozilla.org/comm-central/rev/020ca29b62e3
Remove the remnants of Mozmill's utils.jsm. r=aleca
https://hg.mozilla.org/comm-central/rev/73811aa2c47b
Remove Mozmill's utils.jsm from FolderDisplayHelpers.jsm. r=aleca

Comment 157

[Geoff Lankow (:darktrojan) [PTO, back 9 Feb]]

There's probably a few pieces here and there we can tidy up in other bugs, but I'm calling this done. RIP Mozmill.

Comment 158

[Magnus Melin [:mkmelin]]

Woo-hoo! \o/