1694734 - [Win7/8.1] Minimize, Maximize and Close buttons are no longer visible if Firefox Alpenglow theme is used with Webrender

Status: RESOLVED FIXED

(Firefox :: Theme, defect, P1)

Description

[root_pfad]

+++ This bug was initially created as a clone of Bug #1677696 +++

Affected versions

• 88.0a1 (20210224100119)

Affected platforms

• Windows 8.1x64

Steps to reproduce

1. Open Firefox and go to Customize mode.
2. Set Alpenglow theme and observe the top buttons.

Expected result

• Buttons are displayed as expected.

Actual result

• The buttons are not displayed.

Notes
Tested with a new profile.

Comment 1

[Alexandru Trif, Desktop Test Engineering [:atrif]]

Hello! I can reproduce the above issue on Windows 7x64. This happenes with custom themes as well. Attaching the regression range:

1:53.95 INFO: Last good revision: b3fd546349160dfa293982cace9ba0f289d7788f
1:53.95 INFO: First bad revision: e5ee2e40acc0271d1e6032ab7e6058469cd7eef9
1:53.95 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=b3fd546349160dfa293982cace9ba0f289d7788f&tochange=e5ee2e40acc0271d1e6032ab7e6058469cd7eef9

Moving to regressor's component. Feel free to change it if needed.

Harry, it seems that the patch from bug 1594132 caused the regression, can you please have a look? Thank you!

Comment 2

[Harry Twyford]

I can't reproduce on Windows 10. Does this bug reproduce when WebRender is not enabled? Could you please also send a screenshot of the broken behaviour?

Comment 3

[Cletus Van Damme]

Attachment: nightlyalpenglow.png

Comment 4

[Cletus Van Damme]

Reproduced on Windows 8.1 with 88.0a1 per screenshot above. Can't reproduce with 86.0 and 87.0b2.

How to disable Webrender and retest? A web search tells me to toggle gfx.webrender.all and verify by checking the "compositing" field in about:support, but it was already set to "false" yet compositing status shows as Webrender, and toggling either way doesn't change it.

Comment 5

[root_pfad]

Attachment: Shows the missing min/max/close buttons with the solarize fox theme

You can see the missing buttons in the top right.
As with Cletus I can't seem to disable webrender anymore.

Comment 6

[Timothy Nikkel (:tnikkel)]

(In reply to Cletus Van Damme from comment #4)

How to disable Webrender and retest? A web search tells me to toggle gfx.webrender.all and verify by checking the "compositing" field in about:support, but it was already set to "false" yet compositing status shows as Webrender, and toggling either way doesn't change it.

gfx.webrender.all can enable webrender if it is not enabled on your hardware by default.
gfx.webrender.force-disabled can disable webrender.

Comment 7

[Cletus Van Damme]

Ok. Not reproducible with Webrender disabled.

Comment 8

[Alexandru Trif, Desktop Test Engineering [:atrif]]

I can confirm as well that this is not reproducible with WebRender disabled on Windows7x64. Also, Windows 10x64 is not affected.

Comment 10

[Harry Twyford]

Bug 1594132 mostly changed macOS CSS styles. The only cross platform changes were to move a few background properties from :root :-moz-lwtheme to #navigator-toolbox :-moz-lwtheme. That bug also moves some background-image properties from :root:-moz-lwtheme[lwtheme-image] to :root[lwtheme-image] #navigator-toolbox. Alpenglow is a [lwtheme-image] theme, but it doesn't look like the theme in comment 5 is one, so I'm assuming it was the background-color change that broke this.

I spoke with mstange about this bug, and he pointed out that on Windows 7/8, WebRender erases the window-management buttons from its drawing for them to be visible. It does that by pushing a ClearRect item. That happens after all the other drawing items are pushed, so my CSS change shouldn't have been able to erase the window-management buttons. That makes this a likely bug in WebRender.

I'm going to submit a Band-Aid fix that resets background-color on :root only on Windows 7/8 and file a follow-up for the WebRender team.

Comment 11

[Harry Twyford]

I wrote a patch for this, but I don't have a Windows 7 or 8 machine to test it. Could someone please install one of these builds to see if the issue is fixed?

64-bit: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/YDZTEML9SRaylqGUZcTe-g/runs/0/artifacts/public/build/install/sea/target.installer.exe

32-bit: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/WUYkLfN7R62YAG4rBEUw3Q/runs/0/artifacts/public/build/install/sea/target.installer.exe

Comment 12

[Harry Twyford]

Ah, hold on, I think that build will break some different styling. I'll post a new build shortly. Sorry for the spam!

Comment 13

[Harry Twyford]

New builds:

64-bit: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/GjFQUQhfRmKBQ2zhKUTi0Q/runs/0/artifacts/public/build/install/sea/target.installer.exe

32-bit: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/XhdAikseS_6YYrmnOCSpvA/runs/0/artifacts/public/build/install/sea/target.installer.exe

Treeherder: https://treeherder.mozilla.org/jobs?repo=try&revision=986ee4f0a848ff3737b0d5b601abada897fdb619

Comment 14

[Harry Twyford]

Attachment: Bug 1694734 - Reapply background-color rule to :root on Windows 7/8.

While using -moz-os-version selectors in a shared CSS file isn't ideal, I think it's the best approach here. These selectors will hopefully be temporary, and will be removed when bug 1695280 is fixed. I considered a creating a ruleset like

@media (-moz-os-version: windows-win7),
       (-moz-os-version: windows-win8) {
  #navigator-toolbox:-moz-lwtheme {
    background-color: unset;
  }
  :root:-moz-lwtheme {
    background-color: var(--lwt-accent-color);
  }
}

in browser/themes/windows/browser.css, but I think unsetting the background-color could become a headache if we need to make any other changes to the #navigator-toolbox background. We could also move these background rules to platform-specific stylesheets, but that way they're defined much later in the CSS despite being fairly foundational rules. It would also create more code to remove in bug 1695280.

Comment 15

[root_pfad]

Attachment: Patched version shows the buttons on win 8.1

Looks good! The buttons now appear correctly on Win 8.1 with WebRender enabled.

Comment 16

[Alexandru Trif, Desktop Test Engineering [:atrif]]

I can confirm as well that the buttons are displayed as expected with the provided build from comment 13 on Windows7x64.

Comment 18

[Harry Twyford]

Could someone also please confirm if the alternate approach used in these builds fixes the issue as well?

64-bit: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/BQg7KS63S2GLoe4COeDuVw/runs/0/artifacts/public/build/install/sea/target.installer.exe

32-bit: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/bBl88SQsTCiIc2Ak1J-GwQ/runs/0/artifacts/public/build/install/sea/target.installer.exe

Treeherder: https://treeherder.mozilla.org/jobs?repo=try&revision=537307bb906ea562e52dd6a34a261da4474af800

Comment 19

[Thorin [:thorin]]

Attachment: Win7 64bit alt approach not working

Comment 20

[Harry Twyford]

Thanks, Simon.

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/66ff1258a8ac
Reapply background-color rule to :root on Windows 7/8. r=desktop-theme-reviewers,ntim,mixedpuppy

Comment 25

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/66ff1258a8ac

Comment 26

[root_pfad]

Attachment: solarizefox_win81_fixed.PNG

Confirmed fixed in Nightly build 20210304092248 on Win 8.1.

Comment 27

[Thorin [:thorin]]

Attachment: focus.png

The buttons still get hidden when the application or window loses focus, and is quite distracting

• ctrl-shift-del: dialog has focus
• ctrl-n: new window
• ctrl-shift-k/j: console (see attachment)
• just click off into windows explorer or the desktop

Comment 28

[Alexandru Trif, Desktop Test Engineering [:atrif]]

Attachment: button.gif

Thank you, Simon!
Confirming the above behavior with 88.0a1 (20210310215846) on Windows 7x64 when the firefox window is not in focus and Firefox dark or light theme is used, I don't see this with random custom themes. Harry, should we reopen this bug or open another one? Thank you!

Comment 29

[Alexandru Trif, Desktop Test Engineering [:atrif]]

I have opened bug 1700873 for this. Thank you!