1664016 - Recent changes broke the blocking-master-password-startup fix

Status: RESOLVED FIXED

(Calendar :: General, defect, P1)

Description

[Kai Engert (:KaiE:)]

In Jan 2020 with bug 1610390 we had added code, that works around the ugly bug 177175.

A couple of days ago the problem started to appear again.

At the time the secondary prompts are open, I see the following messages on the console:
JavaScript error: resource:///modules/calendar/utils/calAuthUtils.jsm, line 305: TypeError: self.mWindow is null

That code in asyncPromptAuth() doesn't check that self.mWindow may be null. And apparently it's reached multiple times in succession, triggering multiple password prompts.

My local esr78 branch build from August 5 still worked (only one prompt), and my local build from August 10 is broken (multiple prompts).

I have fix that seems to work.

Comment 1

[Kai Engert (:KaiE:)]

Attachment: Bug 1664016 - Avoid exception "self.mWindow is null" in calAuthUtils. r=pmorris

Comment 2

[Magnus Melin [:mkmelin]]

Bug 1663430 may be caused by this.

Comment 3

[Kai Engert (:KaiE:)]

My testing says this bug is a side effect of recently commited bug 1652885.

Comment 4

[Kai Engert (:KaiE:)]

Paul, Philipp, the change from bug 1652885 causes this regression.

Apparently it causes the calendar authentication to be brought up at a very early time, and inside calAuthUtils self.mWindow is null - but asyncPromptAuth() assumes that it's non-null.

Can you see a problem with the attached suggested fix to this bug?

Comment 5

[Wayne Mery (:wsmwk)]

I consider solving this to be a blocker to eventually setting 68-78 update rate to a large number. (but perhaps not a blocker to shipping 78.2.2)

Comment 6

[Paul Morris [:pmorris]]

I've taken a look at this. I looked at the changes in Philipp's patch for bug 1652885 and I don't understand how they made calendar authorization happen earlier, before there is a window to work with. (I don't doubt it, but I don't see how yet.)

In calAuthUtils's asyncPromptAuth, just below the change in Kai's patch, there is code that handles if the window is not ready yet, by attaching a listener to the window to do something when it is ready. That makes me think we should do something similar for this case where the window doesn't exist yet. But you can't add a listener to a window that doesn't exist, so I don't know what that would look like.

On the other hand maybe the fix in Kai's patch is fine and the calendar code will just retry later? I tried to see if that was the case.

But I haven't been able to reproduce the bug. I set a primary password and subscribed to four google calendars using caldav, and I only get a single prompt for the primary password at startup, and nothing about self.mWindow being null in the terminal. After the app started up I got more than one prompt to log in to my google account (via oauth windows etc.). I should have only gotten one prompt for that, but that would seem to be a separate issue.

(Also, where/how exactly the asyncPromptAuth in calAuthUtils is getting called is a bit obscure to me. It seems to be used by calendar providers that use nsIAuthPrompt2. It must be through the Prompt object set in calProviderUtils.jsm: this.calAuthPrompt = new cal.auth.Prompt();)

So at this point I don't know whether or not Kai's patch would cause other regressions.

Comment 7

[Kai Engert (:KaiE:)]

(In reply to Paul Morris [:pmorris] from comment #6)

But I haven't been able to reproduce the bug. I set a primary password and subscribed to four google calendars using caldav, and I only get a single prompt for the primary password at startup, and nothing about self.mWindow being null in the terminal.

Did you try using comm-esr78 branch?

You need to wait around 10 seconds, before the additional prompt(s) and the error appears).

Comment 8

[Kai Engert (:KaiE:)]

Also, which platform did you try? I saw it on Linux.

I tried adding a caldav calendar, that was insufficient.

To reproduce this bug, I had to install the Google Calendar Provider Add-on, and I subscribed to several calendar. (I don't know if one calendar might have been sufficient.)

Comment 9

[Kai Engert (:KaiE:)]

I traced the events in more detail. Paul was right. We don't automatically attempt to authenticate again.

Paul gave good inspiration, that we should try again when ready. We can set a timeout, and try again as long as the window is null.

I have a better patch, which also suppresses the additional prompt, and resumes calendar auth after the master password window has been completed.

Comment 10

[Kai Engert (:KaiE:)]

Paul, a new patch is up for review in phab

Comment 11

[Paul Morris [:pmorris]]

(In reply to Kai Engert (:KaiE:) from comment #7)

Did you try using comm-esr78 branch?

Aha, no, I only tried on trunk.

You need to wait around 10 seconds, before the additional prompt(s) and the error appears).

Okay, good to know. I'm pretty sure I had the app open that long.

(In reply to Kai Engert (:KaiE:) from comment #8)

Also, which platform did you try? I saw it on Linux.

On Linux.

I tried adding a caldav calendar, that was insufficient.
To reproduce this bug, I had to install the Google Calendar Provider Add-on, and I subscribed to several calendar.

I did not try with that add-on. It makes sense that it would be needed, since it uses the changes in Philipp's patch for registering calendar providers.

Thanks for the revised patch. I think that will do the job. Good idea to use a timeout to wait for the window.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/81ce3546ba1f
Avoid exception "self.mWindow is null" in calAuthUtils. r=pmorris

Comment 13

[James Rome]

This is in 81.0b3 on MacOC Mojave. I get 3 login prompts now

Comment 14

[Kai Engert (:KaiE:)]

Comment on attachment 9174743 [details]
Bug 1664016 - Avoid exception "self.mWindow is null" in calAuthUtils. r=pmorris

Let's test the regression fix in Beta

Comment 15

[Wayne Mery (:wsmwk)]

Comment on attachment 9174743 [details]
Bug 1664016 - Avoid exception "self.mWindow is null" in calAuthUtils. r=pmorris

[Triage Comment]
Approved for beta

Comment 16

[Rob Lemley [:rjl]]

Thunderbird 81.0b4:
https://hg.mozilla.org/releases/comm-beta/rev/76af380c5551

Comment 17

[Wayne Mery (:wsmwk)]

(In reply to James Rome from comment #13)

This is in 81.0b3 on MacOC Mojave. I get 3 login prompts now

you should test with b4

Comment 18

[James Rome]

broken in b4 too

Comment 19

[Kai Engert (:KaiE:)]

James, it's expected that macOS doesn't work as well as other platforms.

We had implemented a workaround for Linux and Windows, only, because it didn't work on macOS (bug 1610390).

Comment 20

[Kai Engert (:KaiE:)]

Comment on attachment 9174743 [details]
Bug 1664016 - Avoid exception "self.mWindow is null" in calAuthUtils. r=pmorris

[Approval Request Comment]
Regression caused by (bug #): 1652885
User impact if declined: several additional master password prompts
Testing completed (on c-c, etc.): yes
Risk to taking this patch (and alternatives if risky): low, it simply retries until ready

Comment 21

[Wayne Mery (:wsmwk)]

Comment on attachment 9174743 [details]
Bug 1664016 - Avoid exception "self.mWindow is null" in calAuthUtils. r=pmorris

[Triage Comment]
Approved for esr78

Let's be sure this is tested during snoketest

Comment 22

[Rob Lemley [:rjl]]

Thunderbird 78.3.0:
https://hg.mozilla.org/releases/comm-esr78/rev/e9ae68991558

Comment 23

[WaltS48 [:walts48]]

Testing 78.3.0 rc on Windows10.

Created a primary password.
Created a POP account, and my Gmail IMAP account.
Closed and reopened Thunderbird and only had one PP prompt.
Added the Provider for Google Calendar extension and added some calendars.
Closed and reopened Thunderbird and only had on PP prompt.
Added an IRC account that requires a password, joined a couple channels and left sign-on at startup enabled.
Closed and reopened Thunderbird and only had on PP prompt.

Comment 24

[Eckard Berberich]

Testing 78.3.0-candidates/build1/ on a Mac running macOS 10.14.6.
The double master password prompt is still present.

Comment 25

[fridok]

Testing 78.3.0-final on a Mac running macOS 10.15.7
The double master password prompt is still present.
Calendar is in use.

This is what happens:

Start Thunderbird.

Before any window appears the password dialog is shown. You wait or you start typing. It doesn't matter. Your input does not enter this password dialog.

Now, the email inbox list is shown in the background.

A second password window appears, and a second password dialog is shown. After a while it gets an input prompt. Now you see two password dialog windows on the screen.

You type your password into the second window. Type OK. The window appears again. You type your pasword into the window. Type OK. It disappears.

The first password window is still lingering around. You click Cancel and it disappears.

Now you can start working.

Comment 26

[Wayne Mery (:wsmwk)]

Do we need to reopen this?

Comment 27

[fridok]

My experience with 78.6.0 on a Mac (MacOS 10.15.7, Calendar is in use) is this:

The double master password prompt is still somewhat present but not as annoying as it was.

This is what happens:

Start Thunderbird. The email inbox list is shown in the background.

The password dialog is drawn two to three times at roughly the same position on screen. When the drawing stops, the last one drawn gets the focus and an input prompt.

You type your password into this window. Type OK. All password dialogs disappear. You are logged in.

I can live with that.

On Windows it works fine. The password prompt appears before you see the inbox/user interface.

Comment 28

[James Rome]

It is still bad on Mac 85.0b3

Comment 29

[Eckard Berberich]

macOS 10.14.6, tested without creating a calendar
No double master password prompt neither in TB 78.6.0 nor in TB 85.0b3.

Comment 30

[James Rome]

I get a double password prompt on Mojave TB 85.0b3.

Comment 31

[Antony]

Tested on 10.15.7 EN macOS with TB 85.0b3 NO PROBLEM

Added the pw closed TB, reopend, prompted ONCE, closed again and reopened, still ONE prompt

Comment 32

[Wayne Mery (:wsmwk)]

(In reply to James Rome from comment #30)

I get a double password prompt on Mojave TB 85.0b3.

If you get a double prompt, please file a new bug report and post the bug# here. Also, it would be helpful to know if it happens in a new profile.

Comment 33

[Eckard Berberich]

(In reply to Eckard Berberich from comment #29)

macOS 10.14.6, tested without creating a calendar
No double master password prompt neither in TB 78.6.0 nor in TB 85.0b3.

I forget to mention that I tested both versions in new profiles.

Comment 34

[James Rome]

Attachment: No profile manager

Comment 35

[James Rome]

I would try a new profile, but there seems to be no profile manager! See attachment

Comment 36

[James Rome]

Filed a new bug in bug 1685877