Closed Bug 628949 Opened 12 years ago Closed 11 years ago

Tab bar turns black bar temporarily when maximizing window

Categories

(Core :: Widget, defect)

x86
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- -

People

(Reporter: luke.iliffe, Assigned: jimm)

References

Details

(Keywords: regression)

Attachments

(3 files, 1 obsolete file)

When maximizing minefield the empty part of the tab bar turns black during the transition to the maximized state then disappears. This occurs both with hardware acceleration turned on or off. Recent regression in the 2011-01-25 nightly confirmed by another Windows 7 user on mozillazine; http://forums.mozillazine.org/viewtopic.php?p=10351061#p10351061

Hourly regression range:
works	2011-01-24-1295915186-20110124162626-742bc2628690	http://hg.mozilla.org/mozilla-central/rev/742bc2628690
broken	2011-01-24-1295920388-20110124175308-9d9dbd3c1a6d	http://hg.mozilla.org/mozilla-central/rev/9d9dbd3c1a6d

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=742bc2628690&tochange=9d9dbd3c1a6d
I forgot to add the simple STR:

1. Open new profile, ensure the window has a normal windowstate.
2. Maximise window

Expected:
Empty tab bar doesn't flash solid black.

Actual:
Empty part of tab bar flashes black.

Note in a new profile this happens very quickly, it is easier to see in my normal profile for some reason. It is easier to see if you only have 1 or 2 tabs open rather than a full or overflowing tab bar. 

Not H/A related but I include my graphics information in case it is not universally reproducible.

Adapter Description: Mobile Intel(R) 4 Series Express Chipset Family
Vendor ID: 8086
Device ID: 2a42
Adapter RAM: Unknown
Adapter Drivers: igdumdx32 igd10umd32
Driver Version: 8.15.10.2226
Driver Date: 10-15-2010
Direct2D Enabled: true
DirectWrite Enabled: true (6.1.7600.20830)
WebGL Renderer: (WebGL unavailable)
GPU Accelerated Windows: 1:/1 Direct3D 10

Mozilla/5.0 (Windows NT 6.1; rv:2.0b11pre) Gecko/20110126 Firefox/4.0b11pre
This reminds me of an old bug that we seen before.
Attaching a brief video illustrating the problem. This was originally posted on mozillazine http://forums.mozillazine.org/viewtopic.php?p=10355021#p10355021
I believe what's happening here is the aero glass area is quickly transitioning from the restored window to the maximized window position but Firefox's XUL navigation toolbar is slower to make that transition. So, there's a short period of time where there is no Firefox chrome below the bottom edge of the aero glass drawn area. 

I wonder if we could extend the aero glass down further behind the navigation toolbar so that when we maximized, that flash was of glass rather than empty (black).  Alternatively, if we had some way to color it toolbar color rather than black, that would help the flashing as well.
This is a pretty ugly experience in a fairly common user interaction (maxmizing a window) so I'm nominating as a Firefox 4 blocker. I believe that bug 624197 is the same issue. (I can see it just hitting alt twice to open and close the menubar when in maximized state.)

Jim, do you have any ideas here? Would putting a tiny (imperceptible) border radius on the nav toolbar's top outside corners force glass behind the nav bar and fix this?  Are there downsides to having glass extend behind our Chrome that would make that a poor solution (hack) to kill the ugly black flashing?
blocking2.0: --- → ?
The glass margin looks right, but for some reason the content isn't repainting quickly enough to fill in the content area. I wonder if that margin is a carry over from normal mode or if it's been updated after the reflow but the paint hasn't happened yet.
(In reply to comment #5)
> Jim, do you have any ideas here? Would putting a tiny (imperceptible) border
> radius on the nav toolbar's top outside corners force glass behind the nav bar
> and fix this?  Are there downsides to having glass extend behind our Chrome
> that would make that a poor solution (hack) to kill the ugly black flashing?

I believe it already extends down to content. Changing this effects the position of the haze in the window border.
OK. I was just guessing because while I can repeat the black flash experience with alt key hit twice in maximized mode, I cannot in restored window mode.
(In reply to comment #7)
> (In reply to comment #5)
> > Jim, do you have any ideas here? Would putting a tiny (imperceptible) border
> > radius on the nav toolbar's top outside corners force glass behind the nav bar
> > and fix this?  Are there downsides to having glass extend behind our Chrome
> > that would make that a poor solution (hack) to kill the ugly black flashing?
> 
> I believe it already extends down to content. Changing this effects the
> position of the haze in the window border.

Actually, I should note this totally depends on how you have your chrome configured. On my system I have tabs on top, the nav bar and the favorites bar, and I get a glass margin of 112px.


(In reply to comment #8)
> OK. I was just guessing because while I can repeat the black flash experience
> with alt key hit twice in maximized mode, I cannot in restored window mode.

Yeah I can totally reproduce that even with gdi rendering.
Looks like we reflow, set the margin, then repaint. The delay between the margin set and the paint gives us the black area.
(In reply to comment #10)
> Looks like we reflow, set the margin, then repaint. The delay between the
> margin set and the paint gives us the black area.

..and it only happens in maximized mode. Interesting.
So I think your idea of rounding the nav bar edges or some other similar fix would address this. Whatever we can do to push the margin down so it's below the top of the nav bar after hitting alt.
Felipe and I talked about this in another bug. A radius that's small enough to not be visible would be ideal. I wonder if that's got any kind of perf cost that would make it an unsuitable bandaid?
Extending the glass area surely isn't free. Doesn't sound like the right tradeoff to me.
Dão, while I'll take your word that it's not free, it's what we already do for restored windows.
(In reply to comment #10)
> Looks like we reflow, set the margin, then repaint. The delay between the
> margin set and the paint gives us the black area.

When/where do we set the margins?
(In reply to comment #16)
> (In reply to comment #10)
> > Looks like we reflow, set the margin, then repaint. The delay between the
> > margin set and the paint gives us the black area.
> 
> When/where do we set the margins?

In widget's UpdateTransparentRegion.
Can we just make the UpdateTransparentRegion call in nsLayoutUtils::PaintFrame after the PaintRoot call there?
Attached patch layout utils patch (obsolete) — Splinter Review
This seemed to cut down on the number of occurrences, but it didn't fix this 100% of the time.
Comment on attachment 510253 [details] [diff] [review]
layout utils patch

You should guard the new UpdateTransparentRegion call with !willFlushRetainedLayers && widget && !(aFlags & PAINT_DOCUMENT_RELATIVE) like it was before it was moved. I don't think that will make the remaining problem go away though.
Component: Theme → Widget
Product: Firefox → Core
QA Contact: theme → general
Not a blocker - would take an extremely safe patch. Is this that?
blocking2.0: ? → -
(In reply to comment #20)
> Comment on attachment 510253 [details] [diff] [review]
> layout utils patch
> 
> You should guard the new UpdateTransparentRegion call with
> !willFlushRetainedLayers && widget && !(aFlags & PAINT_DOCUMENT_RELATIVE) like
> it was before it was moved. I don't think that will make the remaining problem
> go away though.

Let me clean this up, maybe we can still get this approved for 4.0.
This significantly improves the about:addons case.

STR -
1) enable fx button
2) open about:addons in a tab
3) hit the alt key repeatedly

Without this patch you always see a black band in glass when the menu bar is shown. With the patch this anomaly doesn't occur. The maximized case isn't addressed 100% of the time, but it improves.

I'll fire off a try build for people to test with.
Assignee: nobody → jmathies
Attachment #510253 - Attachment is obsolete: true
Blocks: 624197
No longer blocks: 624197
Duplicate of this bug: 624197
Comment on attachment 512806 [details] [diff] [review]
call UpdateTransparentRegion after PaintRoot v.1

Try server likes it.
Attachment #512806 - Flags: review?(tnikkel)
Comment on attachment 512806 [details] [diff] [review]
call UpdateTransparentRegion after PaintRoot v.1

This looks good to me, but I'd like roc to look at it to make sure setting the transparent region after painting is ok.
Attachment #512806 - Flags: review?(tnikkel)
Attachment #512806 - Flags: review?(roc)
Attachment #512806 - Flags: feedback+
Comment on attachment 512806 [details] [diff] [review]
call UpdateTransparentRegion after PaintRoot v.1

Great idea!
Attachment #512806 - Flags: review?(roc) → review+
Attachment #512806 - Flags: approval2.0?
Nasty bug, big patch - my vote is to approve it, but would like other drivers to weigh in.
Comment on attachment 512806 [details] [diff] [review]
call UpdateTransparentRegion after PaintRoot v.1

Not really a big patch IMHO. It's mostly just moving code around.
Attachment #512806 - Flags: approval2.0? → approval2.0+
http://hg.mozilla.org/mozilla-central/rev/792bb2cb1cb1
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Depends on: 647555
You need to log in before you can comment on or make changes to this bug.