Open Bug 1737489 Opened 3 years ago Updated 2 years ago

Nested macOS sheets (modal dialogs) cause app lock-ups

Categories

(Core :: Widget: Cocoa, defect, P3)

All
macOS
defect

Tracking

()

REOPENED
Tracking Status
firefox-esr78 --- unaffected
firefox-esr91 --- affected
firefox93 --- wontfix
firefox94 --- wontfix
firefox95 --- wontfix

People

(Reporter: KaiE, Unassigned)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression, Whiteboard: [mac:stability])

Attachments

(1 file)

Bug 1691171 caused a major regression for Thunderbird in several scenarios with nested modal dialogs, and it took us a while to identify the cause.

I have suggested that we back out this change for Thunderbird 91, because we have an imminent need to upgrade the userbase of TB 78 to TB 91, and a lot of functionality is affected by this regression.

For comm-central and future versions, can we try to modify the change from bug 1691171 to be compatible with Thunderbird?

I wonder why we couldn't reproduce the issue with nested modal dialogs in Firefox. For example, when using the certificate manager, it opens a content area dialog, and when opening an additional modal dialog, it's shown as another dialog on top, and isn't causing any problems.

In Thunderbird's scenario however, the first dialog gets hidden, and at the time the topmost dialog closes, the earlier dialog is restored incorrectly (as described in bug 1715740 etc.) and with window lockup issues.

Do you understand why nested modal dialogs in Thunderbird behave differently?

Do you have ideas how we could change the code to avoid those problems?

I'll point out the worst case situation, which is the user can't open Thunderbird and therefore Thunderbird is unusable.

When a user has a primary password, and also has at least one account where the account password is not saved, then both dialogs will pop up. I myself have experienced this. This also affects openpgp users (at least one has been reported). The (non-obvious) workaround is bug 1712980 comment 2.

The other scenarios are less severe only in that they occur while user is actually using Thunderbird. But they still are effectively hangs.

Severity: -- → S1
Priority: -- → P1

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

Bug 1691171 caused a major regression for Thunderbird in several scenarios with nested modal dialogs, and it took us a while to identify the cause.

What can be done to make these kinds of bugs easier to identify in the future? Does mozregression --app thunderbird not work for this use case?

I have suggested that we back out this change for Thunderbird 91, because we have an imminent need to upgrade the userbase of TB 78 to TB 91, and a lot of functionality is affected by this regression.

See bug 1691171 comment 9 - it's a little late for that.

For comm-central and future versions, can we try to modify the change from bug 1691171 to be compatible with Thunderbird?

We can certainly try, yes! Bug 1691171 was not intended to affect observable behavior.

I wonder why we couldn't reproduce the issue with nested modal dialogs in Firefox. For example, when using the certificate manager, it opens a content area dialog, and when opening an additional modal dialog, it's shown as another dialog on top, and isn't causing any problems.

Firefox has switched to non-native dialogs in many places where it used native macOS sheets in the past, so it's possible that there isn't actually a way anymore to encounter nested sheets in Firefox.

Not a Firefox issue. Happy to consider a low-risk uplift if/when a patch is ready, but this isn't something we're going to actively monitor either.

Blocks: 1715740
Severity: S1 → --
No longer depends on: 1691171
Keywords: regression
OS: Unspecified → macOS
Priority: P1 → --
Regressed by: 1691171
Hardware: Unspecified → All
See Also: 1715740
Has Regression Range: --- → yes
Summary: Need follow-up to macOS Cocoa modal dialog handling that is compatible with Thunderbird → Nested macOS sheets (modal dialogs) cause app lock-ups, affects Thunderbird
Assignee: nobody → kaie
Status: NEW → ASSIGNED

Comment on attachment 9247742 [details]
Bug 1737489 - Application specific backout of bug 1691171, only for MOZ_THUNDERBIRD. r=mstange

Beta/Release Uplift Approval Request

  • User impact if declined: Thunderbird is very broken on macOS
  • Is this code covered by automated tests?: Unknown
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Does not affect Firefox at all, because of #ifdef MOZ_THUNDERBIRD
  • String changes made/needed: none
Attachment #9247742 - Flags: approval-mozilla-beta?

The attached fix is only a temporary measure. We'd want to continue working on a better fix, so setting leave-open keyword.

Keywords: leave-open

Set release status flags based on info from the regressing bug 1691171

Comment on attachment 9247742 [details]
Bug 1737489 - Application specific backout of bug 1691171, only for MOZ_THUNDERBIRD. r=mstange

94 is on release now.

Attachment #9247742 - Flags: approval-mozilla-beta? → approval-mozilla-release?

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

The attached fix is only a temporary measure. We'd want to continue working on a better fix

Let's do that in a new bug though, so that the tracking flags on this bug make sense.

Keywords: leave-open

Did you want to nominate this for ESR91 approval also?

Flags: needinfo?(kaie)

(In reply to Ryan VanderMeulen [:RyanVM] from comment #10)

Did you want to nominate this for ESR91 approval also?

I think Magnus is worried that in theory the backout could result in a behavior change, and I think he wants to wait a couple of days for testing with TB nightly and TB beta, before uplifting to 91.

Although I personally see very little risk here. Given this patch only changes macOS, and given TB on macOS is unusable in many ways, I'd consider any potential backout effects as less problematic and something that could be followed up separately.

So, yes most likely we want to request uplift to mozilla-esr91 very soon, but I'd want to wait for Magnus' decision how soon we'll request it.

Flags: needinfo?(kaie)

Note that we're in RC week, so I'm going to have to build the 91.3esr RC Really Soon Now. Not sure if that's a big deal for TB or not (or if y'all will just take a later commit if this lands after that).

Pushed by kaie@kuix.de:
https://hg.mozilla.org/integration/autoland/rev/d72f32d61af4
Application specific backout of bug 1691171, only for MOZ_THUNDERBIRD. r=mstange

(In reply to Ryan VanderMeulen [:RyanVM] from comment #8)

94 is on release now.

Why this is an argument for not landing into mozilla-beta. Are you unable to make any changes to mozilla-beta this week? Thunderbird is often slightly behind, so if we have to wait until after the next merge, it will mean a delay for being able to roll out this fix for testing with Thunderbird beta users.

Yes, beta is frozen until 95 merges to it next week. This has been standard practice for as long as I can remember.

thanks for the explanation!

(Tangential comment, in reply to part of comment 11)

TB on macOS is unusable in many ways

Sorry to butt in like this. And feel free to take your time answering my question. But I use TB all the time on macOS, and am very happy with it. So I'd like to know more about the issues that make some people consider it unusable. (Besides those related to this bug, of course.)

(In reply to Steven Michaud [:smichaud] (Retired) from comment #17)

(Tangential comment, in reply to part of comment 11)

TB on macOS is unusable in many ways

Sorry to butt in like this. And feel free to take your time answering my question. But I use TB all the time on macOS, and am very happy with it. So I'd like to know more about the issues that make some people consider it unusable. (Besides those related to this bug, of course.)

Hi, sorry if my comment sounded too strongly!
I should have explained my thought in a better way.

Because of this lockup bug, you could very easily run in one of the many scenarios that caused Thunderbird to completely lock up. You could then lose emails you're actively working on and that haven't been saved as drafts. And Wayne found a way that for some users, based on their configuration, you wouldn't even be able to start up at all, because you'd get a lockup immediately on startup.

In order to use Thunderbird 91.x+ on macOS with this bug present, you'd have to be very careful and knowledgeable about all the scenarios that trigger the lockup, and would have to avoid them.

I was trying to emphasize the severity of this bug, which is making Thunderbird on macOS very difficult to use, and some users might get the impression that it is completely unusable. That is why this bug also has gotten a very high priority and we worked on it with urgency. I hope that we'll soon be able to release this bugfix.

Severity: -- → S1
Priority: -- → P1

With this bug fixed, I consider Thunderbird very usable.

Thanks for the reply :-)

Comment on attachment 9247742 [details]
Bug 1737489 - Application specific backout of bug 1691171, only for MOZ_THUNDERBIRD. r=mstange

Per discussion with Kai on Element, uplifting this to mozilla-release doesn't help the TB team because they don't build any releases from there. Given that, I'm OK with landing this on Beta since the change is scoped to only Thunderbird and doesn't need to ride to release anyway. This will at least enable them to spin a new TB beta with this change included for faster verification.

Attachment #9247742 - Flags: approval-mozilla-release?
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 95 Branch

In bug 1699514 comment 27 we have someone reporting a way to reproduce the problem in Firefox. (Bug 1712980.)

Let's reopen this then to track landing the more general fix.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: 95 Branch → ---
Status: REOPENED → NEW

We haven't seen any problems on beta, so please land on esr91.

Flags: needinfo?(ryanvm)

I think I'm not the ideal assignee for this.
Could we get help?

Assignee: kaie → nobody

Adjusting the priority and severity since a temporary fix has landed for Thunderbird. However, we'd want to get this investigated and fixed since it also affects Firefox.

From bug 1699514 comment 27:

[...] If multiple Master Password dialog boxes are triggered then one of them will become attached/stacked behind the parent window and not be able to be interacted with. Firefox must be force-quit, and the the user must aggressively cancel the Master Password boxes as they open and before they can stack. This behavior can be triggered by having multiple tabs/browser windows open that will need access to the password store, and then restart Firefox. As it reloads the prior state the different tabs/windows will trigger their own Master Password prompt. The video in 1712980 (a Thunderbird bug) shows the behavior.

Severity: S1 → S2
Priority: P2 → --
Summary: Nested macOS sheets (modal dialogs) cause app lock-ups, affects Thunderbird → Nested macOS sheets (modal dialogs) cause app lock-ups
Whiteboard: [mac:stability]
Priority: -- → P2
See Also: → 435448, 476541
See Also: → 225174
Blocks: 1737608
See Also: → 1755330

(In reply to Stephen A Pohl [:spohl] from comment #29)

Adjusting the priority and severity since a temporary fix has landed for Thunderbird. However, we'd want to get this investigated and fixed since it also affects Firefox.

From bug 1699514 comment 27:

[...] If multiple Master Password dialog boxes are triggered then one of them will become attached/stacked behind the parent window and not be able to be interacted with. Firefox must be force-quit, and the the user must aggressively cancel the Master Password boxes as they open and before they can stack. This behavior can be triggered by having multiple tabs/browser windows open that will need access to the password store, and then restart Firefox. As it reloads the prior state the different tabs/windows will trigger their own Master Password prompt. The video in 1712980 (a Thunderbird bug) shows the behavior.

Stephen, this doesn't seem a super common use case, maybe we can reassess the severity of this bug?

Flags: needinfo?(spohl.mozilla.bugs)
Flags: needinfo?(spohl.mozilla.bugs)

Per bug 1712980 and bug 1699514, this appears to be fixed (or at least no longer reproducible).

Status: NEW → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → DUPLICATE

Stephen, are you sure you didn't misunderstand this bug?
The issue can no longer be reproduced, because we removed the code that triggers it.

But we're still requiring a fix, that can allow us to undo the workaround - because the current code requires a deprecated API that might go away soon.

See also comment 25, where Ryan suggested to reopen this bug, to track solving the general issue (as far as I can tell, this hasn't happened yet, but I'm happy to be corrected).

Status: RESOLVED → REOPENED
Flags: needinfo?(spohl.mozilla.bugs)
Resolution: DUPLICATE → ---

Please read bug 1755330 for the more detailed explanation.

See Also: → 1792424
Severity: S2 → S3
Flags: needinfo?(spohl.mozilla.bugs)
Priority: P2 → P3

The severity field for this bug is relatively low, S3. However, the bug has 6 See Also bugs.
:spohl, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Flags: needinfo?(spohl.mozilla.bugs)
Flags: needinfo?(spohl.mozilla.bugs)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: