1656782 - 2 Home calendars in new profile

Status: VERIFIED FIXED

(Calendar :: General, defect, P2)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Thunderbird 81.0a1 20200802100815 on Windows 8.1

If one creates a new Thunderbird profile and opens the Events tab, there are 2 disabled 'Home' calenders. There should be only one.

For background information why the calendar(s) are disabled see bug 1623152.

Comment 1

[Paul Morris [:pmorris]]

I just tried to reproduce this on Linux on current trunk (81.0a1), with a brand new profile, and there was only one 'Home' calendar.

Sebastian, are there any suspicious messages in the console? (When working on Thunderbird I have noticed occasional cases where two Home calendars would appear, but it usually occurs when there's an error happening that's throwing things off.)

Comment 2

[Paul Morris [:pmorris]]

Alex ran into this bug and reported this error in chat:

console.debug: "Checking for ical data"
JavaScript error: chrome://calendar/content/calendar-chrome-startup.js, line 479: TypeError: Node.replaceChild: Argument 2 is not an object.

Here's where it's failing (oldImage is not an object):

  let oldImage = todaypane.querySelector(".toolbarbutton-icon");
  todaypane.replaceChild(iconStack, oldImage);

Comment 3

[Paul Morris [:pmorris]]

Attachment: bug1656782-0.diff

This just avoids the error Alex saw. I don't know why the button icon isn't found, but when it's not, we can just continue without modifying the today pane button.

That said, this feels like a poor band-aid that doesn't get to the bottom of this. There's probably another problem that was causing the button icon to not be there. I tried throwing in this function and that didn't reproduce the double home calendar issue, so we haven't gotten to the bottom of this yet.

Comment 4

[Geoff Lankow (:darktrojan)]

My guess is the code is running before the toolbarbutton custom element has had a chance to add the icon. But I don't see how it leads to the original bug.

Comment 5

[Geoff Lankow (:darktrojan)]

Comment on attachment 9173374 [details] [diff] [review]
bug1656782-0.diff

This solves a problem but I don't see how it can be the one reported in this bug. In any case I think a better solution for a button with custom content would be to extend the button custom element.

Comment 6

[Wayne Mery (:wsmwk)]

Anyone else able to reproduce?

Comment 7

[John Bieling (:TbSync)]

On Windows 10 with TB78.2.2 I could reproduce it like so
1. start TB with option -p
2. create fresh profile with name "asd"
3. open TB with new profile
4. hit the X of the setup wizard
5. hit the X of the integration prompt
6. go to calendar

100% reproducable here. Anyone else?

Comment 8

[Arthur K. (he/him)]

Attachment: two-homes.png

Repros on 81.0b3 x64 using STR.

Comment 9

[WaltS48 [:walts48]]

I can reproduce using 78.2.2, 81.0b3 and today's build of Daily on Ubuntu 18.04 LTS. All with separate test profiles.

I probably did not notice that in rc testing, because instead of going to the Calendar tab to enable the Home calendar, I used Calendar from the Account Hub and created a new calendar on my computer. Using that method I only saw Home and Test in the Calendar List.

I also did not see it in yesterday's build of Daily with a test profile. That version updated without my knowledge and when I went to confirm it with another test profile saw two Home calendars in the Calendar List. Nice "Enable" button though!

Comment 10

[WaltS48 [:walts48]]

Upon further testing of all versions with new test profiles.

I only get the double calendars if I click the Close "X" of the account setup wizard.

I can not reproduce if I click the "Cancel" button.

For me on Linux, I don't see the integration prompt until after creating an account and I close and restart

Comment 11

[Paul Morris [:pmorris]]

Using the steps to reproduce (Thanks John!) I was able to fix this. In emailWizard.xhtml the window has identical onunload and onclose handlers that both call onWizardShutdown. Closing the dialog with the "X" button, caused both to be fired, which caused the loadCalendarComponent function to be called twice when it should only be called once per window. Apparently we only need onunload and not onclose. While at it, I also added a guard to prevent multiple calls to loadCalendarComponent. That isn't necessary to fix the bug, but seems prudent.

Try run, with moz-phab patch to follow:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=aeeadd992604c98dd1dd1238213b64e4d6bcfc4b

console.trace() calendar-chrome-startup.js:43:11
    loadCalendarComponent chrome://calendar/content/calendar-chrome-startup.js:43
    atStartupRestoreTabs chrome://messenger/content/msgMail3PaneWindow.js:1063
    loadStartFolder chrome://messenger/content/msgMail3PaneWindow.js:1164
    (Async: setTimeout handler)
    LoadPostAccountWizard chrome://messenger/content/msgMail3PaneWindow.js:919
    onWizardShutdown chrome://messenger/content/accountcreation/emailWizard.js:1996
    onunload chrome://messenger/content/accountcreation/emailWizard.xhtml:1

Comment 12

[Paul Morris [:pmorris]]

Attachment: Bug 1656782 - Prevent loading calendar more than once in a given window. r=darktrojan

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/1805540a69e4
Prevent loading calendar more than once in a given window. r=darktrojan

Comment 14

[Ping Chen [:rnons] (away)]

Comment on attachment 9176150 [details]
Bug 1656782 - Prevent loading calendar more than once in a given window. r=darktrojan

[Approval Request Comment]
Regression caused by (bug #):
User impact if declined: Two Home calendar, and JavaScript error: chrome://lightning/content/imip-bar.js, line 145: InternalError: too much recursion in some cases.
Testing completed (on c-c, etc.): Manually tested.
Risk to taking this patch (and alternatives if risky): Low.

Comment #7 can still be reproduced on TB 78.5.1.

About the too much recursion error, see https://searchfox.org/comm-central/rev/4a07865f0fd09191dbc094acc58f47a4b563606a/calendar/lightning/content/imip-bar.js#68-69
When the load event is triggered twice, HideMessageHeaderPane becomes a function that calls itself! I suspect this causes bug 1680061.

Comment 15

[Wayne Mery (:wsmwk)]

Comment on attachment 9176150 [details]
Bug 1656782 - Prevent loading calendar more than once in a given window. r=darktrojan

[Triage Comment]
Approved for esr78

Comment 16

[Rob Lemley [:rjl]]

Thunderbird 78.6.0:
https://hg.mozilla.org/releases/comm-esr78/rev/072086fa828e

Comment 17

[WaltS48 [:walts48]]

Verified in my testing of the 78.6.0 release candidate on Windows 10.