Modal dialog breaks invalidation of tab strip changing from background to foreground

NEW
Unassigned

Status

()

Core
Widget: Cocoa
P3
normal
4 years ago
2 years ago

People

(Reporter: johnath, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: tpi:+)

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
Created attachment 8460524 [details]
Screen Shot 2014-07-22 at 1.33.35 PM.png

[There is a real risk this is not a widget bug - apologies if mis-filed]

STR in brief is to launch firefox in the background with a modal window, then dismiss the modal window and see broken invalidation.

Specific STR:
1) New profile, set firefox to restore tabs on restart
2) Open two tabs, one of which has HTTP auth (e.g. https://intranet.mozilla.org) and one normal (e.g. http://bing.com)
3) Close Firefox, relaunch it, and then switch focus off of Firefox before it starts up
4) When Firefox starts and restores the page, it will prompt for auth. Dismiss the auth prompt (thus foregrounding Firefox again), and note that the window is still painted as a background window.
5) Swap to the other tab for added sadness (see attachment)

Comment 1

4 years ago
Markus, can you have a look? I wonder if this is related to our other tabstrip invalidation issues, but this time with good STR...
Flags: needinfo?(mstange)
I can reproduce (in FF 30), and I suspect this *is* a widget bug.

Without some serious debugging, though, I won't know what we can do about it.  Any ideas, Markus?

> 1) New profile, set firefox to restore tabs on restart

I don't know what you mean here, but I get the same result by setting browser.showQuitWarning in about:config, then choosing "Save and Quit" in the resulting dialog.

> 5) Swap to the other tab for added sadness (see attachment)

I get the same result just mousing over other tabs in the tab bar.
(In reply to Steven Michaud from comment #2)
> Without some serious debugging, though, I won't know what we can do about
> it.  Any ideas, Markus?

Some, but I'll also need to do some debugging to know exactly what's happening.

Whether the titlebar is drawn light gray or dark gray is determined using [window isMainWindow] here:
http://hg.mozilla.org/mozilla-central/annotate/a91ec42d6a9c/widget/cocoa/nsNativeThemeCocoa.mm#l2049

When this value changes, we need to invalidate.

Titlebar invalidations happen from here:
http://hg.mozilla.org/mozilla-central/annotate/a91ec42d6a9c/layout/base/nsDisplayList.cpp#l2498

... which is conditional on the document having changed NS_DOCUMENT_STATE_WINDOW_INACTIVE state:
http://hg.mozilla.org/mozilla-central/annotate/a91ec42d6a9c/layout/base/nsDisplayList.cpp#l2473

... and this state is set whenever nsPIDOMWindow::IsActive() for the root DOM window returns true:
http://hg.mozilla.org/mozilla-central/annotate/a91ec42d6a9c/content/base/src/nsDocument.cpp#l9320
http://hg.mozilla.org/mozilla-central/annotate/a91ec42d6a9c/layout/base/nsPresContext.cpp#l1672

... and nsPIDOMWindow::IsActive() changes when nsPIDOMWindow::SetActive() is called, and that happens here:
http://hg.mozilla.org/mozilla-central/annotate/a91ec42d6a9c/dom/base/nsGlobalWindow.cpp#l9693
plus a few lines further down when sheets are involved.

nsGlobalWindow::ActivateOrDeactivate is called from nsFocusManager::WindowRaised
http://dxr.mozilla.org/mozilla-central/source/dom/base/nsFocusManager.cpp#689

... which is usually called with this callstack:

nsFocusManager::WindowRaised(nsIDOMWindow*)
nsWebShellWindow::WindowActivated()
-[WindowDelegate sendToplevelActivateEvents]
+[TopLevelWindowData activateInWindow:]
__CFNOTIFICATIONCENTER_IS_CALLING_OUT_TO_AN_OBSERVER__
_CFXNotificationPost
-[NSNotificationCenter postNotificationName:object:userInfo:]
_NXShowKeyAndMain
-[NSWindow sendEvent:]
-[BaseWindow sendEvent:]
-[ToolbarWindow sendEvent:]
-[NSApplication sendEvent:]
-[GeckoNSApplication sendEvent:]
-[NSApplication run]


My guess is that when sheets are involved, the call to WindowRaised probably happens at a time before isMainWindow changes, and so when we paint after the invalidation, isMainWindow is still false and we paint a light gray titlebar, and then when isMainWindow actually changes we fail to invalidate because we think the window has already changed its activeness status.

My preferred way of fixing this would be to uncouple nsGlobalWindow::ActivateOrDeactivate from focus handling and add add events that are fired only in response to "isMainWindow" status changes. Then the special handling of sheets could be removed from nsGlobalWindow::ActivateOrDeactivate.
Flags: needinfo?(mstange)

Comment 4

2 years ago
Hi,

I notice the same problem on Linux & Windows.
I am the CookieKeeper add-on developer. For the preferences of my add-on I used to open a modal dialog.
After closing the dialog, the tabs are broken, like in the screenshot.
I don't know if it is exactly the same bug though, but for me, it is not an OSX specific bug.

Updated

2 years ago
Whiteboard: tpi:?

Updated

2 years ago
Priority: -- → P3
Whiteboard: tpi:? → tpi:+
You need to log in before you can comment on or make changes to this bug.