1824053 - When opening a new Firefox window, window surface may initially appear white (causing a flash in dark mode)

Status: VERIFIED FIXED

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

Description

[Dan]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:109.0) Gecko/20100101 Firefox/111.0

Steps to reproduce:

1. Open Firefox
2. In Firefox settings, switch the "Website appearance" option from "Automatic" to "Dark".
3. Open 2 tabs with both having dark pages loaded (for example YouTube.com . It should appear dark automatically due to the Firefox website appearance setting, otherwise use "appearance" setting in YouTube set to "dark".)
4. now hold left mousebutton on one tab down and drag the tab out of the tab-bar (to split the Firefox window into two separate windows, in each being one tab).
5. release mousebutton

Actual results:

the entire selected window shortly brightly flashes in white (possibly due to some kind of redraw-function?)

Expected results:

The window should not flash in a very bright color if the Firefox "Website appearance" option is set to "Dark", but rather in an eye-friendly black color or very dark color, to not cause a massive short contrast moment for users who are interacting with mostly dark-design websites. (This issue getting fixed also might be a big welcome to all seizure-sensitive people.)

Comment 1

[Dan]

clarification for "actual results:" the window shortly brightly flashes in white, before the actual website successfully loads and appears. (it loads in a splitsecond however, due to caching, so loading-time of the actual website itself can be ignored as far as I know)

Comment 2

[Dan]

Tested again, happens in about 7/10 cases, so not always.

Comment 3

[Ray Kraesig [:rkraesig]]

Much to my surprise, this appears to be a (mis)feature of DWM itself! This StackOverflow link has some detailed third-party investigation, as well as several known workarounds of various degrees of effectiveness and intrusiveness.

We should be able to implement one of them without much difficulty — most likely some variant of "cloak before showing".

Comment 5

[Ray Kraesig [:rkraesig]]

Attachment: Bug 1824053 - [1/4] Remove Win32 implementation of nsIWidget::SetBackgroundColor() r?#win-reviewers!

This never worked according to its description: it merely set the
window-class background-color. We neither have nor need any mechanism
to give individual windows their own per-widget background color under
Win32.

Instead, we just set the background color to be dark grey, which is
probably good enough for the few purposes for which the background brush
is actually used. (Alas, this does not quite include the initial white
flash.)

The comment in this bug about this being important for Windows dates
from bug 574638, and is no longer relevant -- at least, not in the
particular way it used to be.

Comment 6

[Ray Kraesig [:rkraesig]]

Attachment: Bug 1824053 - [2/4] Cloak and clear new windows on creation r?#win-reviewers!

DWM will draw our window's surface before we get a chance to clear it:
specifically it will draw the window immediately upon ::ShowWindow(),
and we cannot draw to the window surface before that.

We can, however, cloak the window before showing it; this causes DWM
not to composite it until it's uncloaked. A cloaked window still has a
drawable surface, though, so we just manually clear the window with an
appropriate color before its uncloaking.

Comment 7

[Ray Kraesig [:rkraesig]]

Attachment: Bug 1824053 - [3/4] DRY up skeleton-ui dynamic-resolution code r?#win-reviewers!,glandium!

Cleanup prior to the addition of new code. No functional changes.

Comment 8

[Ray Kraesig [:rkraesig]]

Attachment: Bug 1824053 - [4/4] cloak and clear new window before showing it r?#win-reviewers!,glandium!

When creating the skeleton UI window, cloak and clear it to a theme-
appropriate color before first showing it, just as is done in nsWindow.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/49613184d359
[1/4] Remove Win32 implementation of nsIWidget::SetBackgroundColor()  r=win-reviewers,handyman
https://hg.mozilla.org/integration/autoland/rev/48521d359050
[2/4] Cloak and clear new windows on creation  r=win-reviewers,handyman
https://hg.mozilla.org/integration/autoland/rev/f5f062211a87
[3/4] DRY up skeleton-ui dynamic-resolution code  r=glandium,win-reviewers,handyman
https://hg.mozilla.org/integration/autoland/rev/25851866b7b7
[4/4] cloak and clear new window before showing it  r=win-reviewers,handyman

Comment 11

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/49613184d359
https://hg.mozilla.org/mozilla-central/rev/48521d359050
https://hg.mozilla.org/mozilla-central/rev/f5f062211a87
https://hg.mozilla.org/mozilla-central/rev/25851866b7b7

Comment 12

[Ina Popescu, Desktop QA]

I was unable to reproduce the issue as described in Comment 1 while using Nightly 129.0a1 on Windows10 x64. @Dan, could you please confirm if the issue persists in the latest Firefox 129.0b7? Thank you.

Comment 13

[Dan]

Fixed in latest Beta "129.0" for me too, yes, thanks!!

Comment 14

[Ina Popescu, Desktop QA]

Thank you for confirming. Updating the remaining flags as well.

Comment 15

[Ray Kraesig [:rkraesig]]

Attachment: Bug 1824053 - [5/4] Correctly set flag after initial ::Show() r?emilio,handyman

The mHasBeenShown flag was not being set after the window had indeed
been shown. Correct this.

Comment 16

[Phabricator Automation]

Comment on attachment 9435586 [details]
Bug 1824053 - [5/4] Correctly set flag after initial ::Show() r?emilio,handyman

Revision D228108 was moved to bug 1929413. Setting attachment 9435586 [details] to obsolete.

Comment 17

[Ray Kraesig [:rkraesig]]

(Sorry about that, all! Nothing to see here.)