941559 - On OSX inactive windows are only partially repainted when :-moz-window-active changes, affecting Australis private inactive windows

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Nils Maier [:nmaier]]

Attachment: inactive-private.png

Comment 1

[Nils Maier [:nmaier]]

Attachment: inactive-private-fullscreen.png

Sorry, hit ENTER too soon without giving a text...

STR:
- Open a new private window
- Switch to another window (e.g. open About Nightly), while the private window is still visible, but inactive in the background.
--> Colors aren't correct as the screenshots depict, neither in normal nor in native full-screen mode.

Comment 2

[Mike de Boer [:mikedeboer]]

Nils, I can't reproduce it :( Which version of OSX are you using?

Comment 3

[Mike de Boer [:mikedeboer]]

Hmm, I can see it flashing, but it looks OK after a repaint.

Comment 4

[Nils Maier [:nmaier]]

OSX Lion.
Latest nightly the update service spit at me:
Built from http://hg.mozilla.org/mozilla-central/rev/597287004ff5

Comment 5

[Nils Maier [:nmaier]]

(In reply to Mike de Boer [:mikedeboer] from comment #3)
> Hmm, I can see it flashing, but it looks OK after a repaint.

It doesn't just flash for me. It stays that way till the window gets focus again.

Comment 6

[Nils Maier [:nmaier]]

Turns out this seems to be a layout bug that is very similar to bug 833033. I think stuff regressed in the meantime.

Just scheduling a paint of the frame is not enough when the window active state changes to inactive. Unless something else triggers invalidation, the repaint will only redraw parts of the window where there is affected "content", but not the actual window background.

One way to deal with this is to just invalidate the whole frame if the window state changes to inactive.

Comment 7

[Nils Maier [:nmaier]]

Attachment: Invalidate the whole root frame when a window becomes inactive.

Invalidating the rect works for me. However, I'm a total layout n00b and have no clue what I'm actually doing here ;)

Comment 8

[Boris Zbarsky [:bzbarsky]]

Why is the window background change not being picked up?  Do we need to restyle, not just schedule a paint?

Comment 9

[Markus Stange [:mstange]]

That patch basically reverts bug 532828, which should not be the goal here :)

I see the same problem when I set style="background-color: red" on the private browsing window using the DOM Inspector, with no focus change involved. Seems more like a DLBI problem to me.

Comment 10

[Nils Maier [:nmaier]]

I got curious, and found a way to reproduce the partial paint without relying on focus changes:

If you do the following:
- Open a new regular browser window
- DOM Inspector it
- Set #main-window background-color to red (to make it more obvious)
- Set #titlebar -moz-appearance to none

Then the whole area gets redrawn as expect.


But if you switch the order:
- Open a new regular browser window
- DOM Inspector it
- Set #titlebar -moz-appearance to none
- Set #main-window background-color to red (to make it more obvious)

Then the area only gets partially redrawn where there are other frames, same way as in my screenshots.
Making the window active or inactive actually does nothing.


To figure out if the defining thing is #titlebar -moz-appearance, or just a non-transparent #titlebar background in general I furthermore tried the following:
- Open a new regular browser window
- DOM Inspector it
- Set #titlebar -moz-appearance to none
- Set #titlebar background-color green
- Set #main-window background-color to red (to make it more obvious)
- Remove #titlebar background-color again.

Turns out this makes it display correctly as well.

So the problem here seems to be that a window will not invalidate the areas not covered by something else  when the background-color of the window (root frame/layer/display item/whatever?) itself is changed.

Comment 11

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

This is odd, there's nothing particularly special about the root background as far as DLBI is concerned, as far as I know.

Comment 12

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

And this bug doesn't seem to affect Linux.

Comment 13

[Matt Woodrow (:mattwoodrow)]

This is because the display item that is changing colour isn't a nsDisplayThemedBackground, it's an nsDisplaySolidColor.

The frame that we use to compute this background color (in PresShell::UpdateCanvasBackground) isn't the same as the frame we use for the nsDisplaySolidColor (in PreShell::AddCanvasBackgroundColorItem). DLBI can't know that the former being marked as invalid should invalidate the pixels for the latter.

I'm not sure how (or if we even want) to fix this, so I'll just make nsDisplaySolidColor compare the colour that it's going to draw, and invalidate if it changes.

Sound good?

Comment 14

[Matt Woodrow (:mattwoodrow)]

The frame we're using for the style is nsDocElementBoxFrame*, and the nsDisplaySolidColor is created for the ViewportFrame*

Comment 15

[Matt Woodrow (:mattwoodrow)]

Attachment: Invalidate for color changes with nsDisplaySolidColor

Comment 16

[Nils Maier [:nmaier]]

Comment on attachment 8336576 [details] [diff] [review]
Invalidate for color changes with nsDisplaySolidColor

Fixes the drawing for me.

Comment 17

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3a9f42854971

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3a9f42854971

Comment 22

[Markus Stange [:mstange]]

Comment on attachment 8336576 [details] [diff] [review]
Invalidate for color changes with nsDisplaySolidColor

Bug 950224 alerted me to the fact that this bug is still present on the 27 branch, we should try to get the fix uplifted.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 532828
User impact if declined: strange titlebar appearance in private browsing windows when changing window focus
Testing completed (on m-c, etc.): three weeks on mozilla-central
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/5a01890560d7

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Add missing nsColor.h #include to nsDisplayListInvalidation.h to fix bustage.
https://hg.mozilla.org/releases/mozilla-beta/rev/7736d6971bc3