621762 - Window title bar flickers between unified and non-unified toolbar

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[[e-mail]]

User-Agent:       Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b9pre) Gecko/20101228 Firefox/4.0b9pre
Build Identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b9pre) Gecko/20101228 Firefox/4.0b9pre

When displaying some pages, the window title/toolbar area flickers between unified and non-unified toolbar.

Reproducible: Sometimes

Steps to Reproduce:
1. Load the Facebook page above.
2. Observe the window title/toolbar when the page is loading.
Actual Results:  
The title/toolbar flickers. It switches between unified and non-unified  OS X toolbar.

Expected Results:  
The title/toolbar remains in unified toolbar appearance.

This happens with both tabs-on-top on and off.

Comment 1

[Stefan [:stefanh]]

I can confirm this with the 20101227 nightly thunderbird build (10.6.5). Upon launch, there is a border between the titlebar and the toolbar and the whole titlebar flickers. If I click outside the window the title bar becomes unified and the flicker disappears. Moving focus back to the window seems to keep the unified look for a while, but the issue reappears after I've switched between a couple of tabs or when the password prompt have appeared.

Thinking of it, I have seen this in a seamonkey nightly for a while ago, but I couldn't reproduce it in the next nightly so I thought it was a one-time issue. I have never seen this in my own seamonkey debug builds, though (64-bit only).

Comment 3

[Markus Stange [:mstange]]

My guess is that this was caused by bug 615870, which added empty layer transactions for which no display lists are constructed. Our unified toolbar resetting code (last changed in bug 579250) always expects a display list construction during -[ChildView drawRect:], so if that doesn't happen the unified toolbar will be reset.
We probably need to move the begin/endMaybeResetUnifiedToolbar calls out of drawRect and to the place where we know that display list construction is going to happen.

Comment 4

[Marek Stępień [:marcoos, inactive]]

FWIW, it seems that a temporary workaround for this bug is to install a Persona, e.g. from getpersonas.com.

Comment 5

[Stefan [:stefanh]]

(In reply to comment #1)
> I have never seen this in my own seamonkey debug builds, though (64-bit only).

Forget that - I now see it.

Comment 6

[jfftck]

I have the same issue but also affects the tabs. If I have many tabs open it causes them to disappear when scrolling for up to 1 second if I have hardware acceleration on or up to 4 seconds with it off. This issue is not just the titlebar

Comment 7

[Markus Stange [:mstange]]

Attachment: v1

Instead of notifying the theme one-by-one for every widget and having nsChildView expect the notifications during painting, let nsDisplayListBuilder::LeavePresShell notify the widget with an array of all themed widgets at the same time.

I'm not sure if LeavePresShell is a good place for this.

Comment 8

[jfftck]

Attachment: Screenshot of broken tabs

This shows the tabs disappearing after being scrolled.

Comment 9

[Markus Stange [:mstange]]

jfftck, what you're seeing is bug 623852, not this one.

Comment 10

[Markus Stange [:mstange]]

Attachment: v2

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 502366 [details] [diff] [review]
v2

This is really great. A few nits:

+   * Unused. This is only here because the interface is frozen for 2.0.

Add an XXX here

+    nsIFrame* displayRoot = nsLayoutUtils::GetDisplayRootFrame(aReferenceFrame);

You should be able to just use mReferenceFrame.

+    nsAutoTArray<ThemeGeometry,8> mThemeGeometries;

We normally wouldn't have 8 toolbars in the Firefox window so wouldn't 2 or 3 be a better value? Doesn't matter much.

+    virtual void UpdateThemeGeometries(nsTArray<ThemeGeometry>& aThemeGeometries) = 0;

const nsTArray&

r+ with that

Comment 12

[Timothy Nikkel (:tnikkel)]

Would it make more sense to put the LeavePresShell bit in nsLayoutUtils::PaintFrame? That's where we do the transparency stuff for aero.

Comment 13

[jfftck]

I am not having problems with the web sites flickering, just the titlebar. So, that bug is not the same issue and I am not seeing the same issues that are reported in that bug.

Comment 14

[Markus Stange [:mstange]]

Attachment: for checkin

Comment 15

[Markus Stange [:mstange]]

Attachment: for checkin

One "const" was missing in the last patch so it didn't compile. This one does.

Comment 16

[Markus Stange [:mstange]]

http://hg.mozilla.org/mozilla-central/rev/6cfffe34531c

Comment 17

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

(In reply to comment #11)
> Comment on attachment 502366 [details] [diff] [review]
> v2
> 
> This is really great. A few nits:
> 
> +   * Unused. This is only here because the interface is frozen for 2.0.
> 
> Add an XXX here

And file a bug, blocked on bug 610267, pretty please?

Comment 18

[Markus Stange [:mstange]]

Done.

Comment 19

[Cpuroast]

Kinda sucks, that's it not going to be included in Beta9, it makes Firefox unusable on OSX. It will cause me to stick with Beta8 until Beta10 is released and skip Beta 9 completely.

This will be a big turn off, for the people on OSX testing the beta milestones.
They will either stick with Beta8 until Beta10 is released or move to a recent nightly containing the fix.

Comment 22

[Timothy Nikkel (:tnikkel)]

Comment on attachment 502633 [details] [diff] [review]
for checkin

+  // If we're finished building display list items for painting of the outermost
+  // pres shell, notify the widget about any toolbars we've encountered.
+  if (mIsPaintingToWindow && mPresShellStates.Length() == 1) {
+    nsIWidget* widget = aReferenceFrame->GetNearestWidget();
+    if (widget) {
+      nsIWidget_MOZILLA_2_0_BRANCH* widget2 =
+        static_cast<nsIWidget_MOZILLA_2_0_BRANCH*>(widget);
+      widget2->UpdateThemeGeometries(CurrentPresShellState()->mThemeGeometries);
+    }
+  }
+
   // Unmark and pop off the frames marked for display in this pres shell.
   PRUint32 firstFrameForShell = CurrentPresShellState()->mFirstFrameMarkedForDisplay;
   for (PRUint32 i = firstFrameForShell;
        i < mFramesMarkedForDisplay.Length(); ++i) {
     UnmarkFrameForDisplay(mFramesMarkedForDisplay[i]);
   }
   mFramesMarkedForDisplay.SetLength(firstFrameForShell);
   mPresShellStates.SetLength(mPresShellStates.Length() - 1);
@@ -710,59 +721,48 @@ PRBool nsDisplayItem::RecomputeVisibilit
 
 void nsDisplaySolidColor::Paint(nsDisplayListBuilder* aBuilder,
                                 nsIRenderingContext* aCtx) {
   aCtx->SetColor(mColor);
   aCtx->FillRect(mVisibleRect);
 }
 
 static void
-RegisterThemeWidgetGeometry(nsIFrame* aFrame)
+RegisterThemeGeometry(nsDisplayListBuilder* aBuilder, nsIFrame* aFrame)
 {
-  nsPresContext* presContext = aFrame->PresContext();
-  nsITheme* theme = presContext->GetTheme();
-  if (!theme)
-    return;
-
   nsIFrame* displayRoot = nsLayoutUtils::GetDisplayRootFrame(aFrame);
-  nsIWidget* widget = displayRoot->GetNearestWidget();
-  // If the display root doesn't have a widget, just bail. Something
-  // weird is going on, maybe we're printing?
-  if (!widget)
-    return;
 
   for (nsIFrame* f = aFrame; f; f = f->GetParent()) {
     // Bail out if we're in a transformed subtree
     if (f->IsTransformed())
       return;
     // Bail out if we're not in the displayRoot's document
     if (!f->GetParent() && f != displayRoot)
       return;
   }
 
   nsRect borderBox(aFrame->GetOffsetTo(displayRoot), aFrame->GetSize());
-  theme->RegisterWidgetGeometry(widget,
-      aFrame->GetStyleDisplay()->mAppearance,
-      borderBox.ToNearestPixels(presContext->AppUnitsPerDevPixel()));
+  aBuilder->RegisterThemeGeometry(aFrame->GetStyleDisplay()->mAppearance,
+      borderBox.ToNearestPixels(aFrame->PresContext()->AppUnitsPerDevPixel()));
 }

Isn't aReferenceFrame in LeavePresShell is going to be different from displayRoot in RegisterThemeGeometry if aFrame is in a popup? Does that matter for the situations this is used?

Comment 24

[Anurag Phadke[:[e-mail]]]

I downloaded the latest MineField release (1/15) on Mac OS X, the title bar seems to be hosed. I can type an address but it doesn't seem to show up in the title-bar, tried restarting, force quitting etc, same issue.

Comment 25

[Wesley W. Garland]

This is affecting me on ff4b9, ff4b8 was okay on the same platform.

Possibly related to my missing UI elements, bug 626090?

If anybody wants to see this bug in action, here's a short video: http://www.youtube.com/watch?v=MARwpBmW6dk

I'm on Leopard, x86, nothing fancy

Comment 26

[Timothy Nikkel (:tnikkel)]

This bug didn't get fixed in time for beta 9. If you want to test you can either wait for the next beta or download a nightly build.

Comment 27

[LegNeato]

(In reply to comment #25)
> This is affecting me on ff4b9, ff4b8 was okay on the same platform.
> 
> Possibly related to my missing UI elements, bug 626090?
> 
> If anybody wants to see this bug in action, here's a short video:
> http://www.youtube.com/watch?v=MARwpBmW6dk
> 
> I'm on Leopard, x86, nothing fancy

FWIW this bug is in the release notes as a known issue for beta9...

Comment 28

[Graham Anderson]

I'm seeing this is in Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b9) Gecko/20100101 Firefox/4.0b9 on 

Model Name:	Mac Pro
  Model Identifier:	MacPro3,1
  Processor Name:	Quad-Core Intel Xeon
  Processor Speed:	3 GHz
  Number Of Processors:	2
  Total Number Of Cores:	8
  L2 Cache (per processor):	12 MB
  Memory:	4 GB
  Bus Speed:	1.6 GHz
  Boot ROM Version:	MP31.006C.B05
  SMC Version (system):	1.25f4