1498588 - Select all not working when Lightning enabled version 63 beta 1 released October 11th, 2018

Status: RESOLVED FIXED

(Calendar :: General, defect)

Description

[dennism2005]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3497.100 Safari/537.36

Steps to reproduce:

Attempted to use Ctrl-A to select all messages in my Inbox.  When it did not work I tried Edit/Select/All which didn't work either.  I got the same results with other folders.


Actual results:

After Thunderbird 63 beta 1 released October 11th, 2018 was installed Ctrl-A or Edit/Select/All does not select all messages in a folder unless the Lightning extension is disabled.  When I noticed the problem I ran Thunderbird with add-ons disabled and everything worked fine.  By disabling add-ons one at a time I found Lightning was causing it.


Expected results:

All messages in the folder should have been selected with Ctrl-A or Edit/Select/All.

Comment 1

[Jorg K (CEST = GMT+2)]

I can reproduce this on trunk. Thanks for reporting.

Comment 2

[Geoff Lankow (:darktrojan)]

Oh boy, command controllers! My favourite how-is-that-supposed-to-work mystery. I have two ways of fixing this, one is a hack, and the other I prefer but I'm not sure if it breaks any number of unrelated things.

Comment 3

[Geoff Lankow (:darktrojan)]

Attachment: 1498588-cmd-selectall-1.diff

I think this is right for all commands handled by the calendar controller. I had a green Try run with this patch, but IMO that doesn't prove it's fault-free. Alternatively we could just hard-wire the specific case for cmd_selectAll.

Comment 4

[Philipp Kewisch [:Fallen][☁️📆]]

Comment on attachment 9017369 [details] [diff] [review]
1498588-cmd-selectall-1.diff

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

Not a fan of hard-wiring, if this improves the situation lets give it a try.

::: calendar/base/content/calendar-common-sets.js
@@ +788,5 @@
>      // we leave the current thread in order to re-enter the message loop.
>  
>      let tbController = top.DefaultController;
>      if (tbController) {
> +        calendarController.defaultController = document.getElementById("tabmail");

If we are using tabmail always here, might as well get rid of the tbController variable.

Comment 5

[Geoff Lankow (:darktrojan)]

Attachment: 1498588-cmd-selectall-2.diff

Good, that gets rid of the setTimeout too. I never did like that.

Comment 6

[Jorg K (CEST = GMT+2)]

Comment on attachment 9018082 [details] [diff] [review]
1498588-cmd-selectall-2.diff

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

::: calendar/base/content/calendar-common-sets.js
@@ +784,5 @@
>      // gets installed by thunderbird. Since we get called pretty early
>      // during startup we need to install the function below as a callback
>      // that periodically checks when the original thunderbird controller
>      // gets alive. Please note that setTimeout with a value of 0 means that
>      // we leave the current thread in order to re-enter the message loop.

Can you adjust this comment as well, please.

Comment 7

[Geoff Lankow (:darktrojan)]

Attachment: 1498588-cmd-selectall-3.diff

Yeah, oops, that comment's a load of garbage now.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/855ce13ec8f9
Pass commands on to the tabmail controller if calendar is not the active tab. r=philipp DONTBUILD

Comment 11

[Jorg K (CEST = GMT+2)]

Comment on attachment 9018176 [details] [diff] [review]
1498588-cmd-selectall-3.diff

Too late for 63/6.5 beta now.

Comment 12

[dennism2005]

64.0b1 allows select all but the Archive and Delete buttons are dimmed but do work.

Comment 13

[Geoff Lankow (:darktrojan)]

Yes we know, thank you, that is bug 1501101.