Closed Bug 1664016 Opened 1 year ago Closed 1 year ago

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

Categories

(Calendar :: General, defect, P1)

Tracking

(thunderbird_esr78+ fixed, thunderbird81 fixed)

RESOLVED FIXED
82 Branch
Tracking Status
thunderbird_esr78 + fixed
thunderbird81 --- fixed

People

(Reporter: KaiE, Assigned: KaiE)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

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.

Assignee: nobody → kaie
Status: NEW → ASSIGNED

Bug 1663430 may be caused by this.

See Also: → 1663430

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

Depends on: 1652885
Keywords: regression

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?

Flags: needinfo?(philipp)
Flags: needinfo?(paul)
Severity: -- → S3
Priority: -- → P1
No longer depends on: 1652885
Regressed by: 1652885

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)

Severity: S3 → S1

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.

Flags: needinfo?(paul)

(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).

Flags: needinfo?(paul)

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.)

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.

Paul, a new patch is up for review in phab

Flags: needinfo?(philipp)
Flags: needinfo?(paul)
Attachment #9174743 - Attachment description: Bug 1664016 - Avoid exception "self.mWindow is null" in calAuthUtils. r=mkmelin → Bug 1664016 - Avoid exception "self.mWindow is null" in calAuthUtils. r=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.

Pushed by kaie@kuix.de:
https://hg.mozilla.org/comm-central/rev/81ce3546ba1f
Avoid exception "self.mWindow is null" in calAuthUtils. r=pmorris

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 82 Branch

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

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

Attachment #9174743 - Flags: approval-comm-beta?

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

[Triage Comment]
Approved for beta

Attachment #9174743 - Flags: approval-comm-beta? → approval-comm-beta+

(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

Flags: needinfo?(jamesrome)

broken in b4 too

Flags: needinfo?(jamesrome)

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 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

Attachment #9174743 - Flags: approval-comm-esr78?

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

Flags: needinfo?(wls220spring)
Flags: needinfo?(vseerror)
Flags: needinfo?(de.berberich)
Attachment #9174743 - Flags: approval-comm-esr78? → approval-comm-esr78+

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.

Flags: needinfo?(wls220spring)

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

Flags: needinfo?(de.berberich)

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.

Do we need to reopen this?

Flags: needinfo?(vseerror)
Flags: needinfo?(de.berberich)
Flags: needinfo?(acdp)

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.

It is still bad on Mac 85.0b3

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.

Flags: needinfo?(de.berberich)

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

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

Flags: needinfo?(acdp)

(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.

(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.

Attached image No profile manager

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

Filed a new bug in bug 1685877

You need to log in before you can comment on or make changes to this bug.