1701562 - Devtools Style editor - Style sheet could not be loaded [ x ] close button has wrong color with dark theme

Status: VERIFIED FIXED

(DevTools :: Style Editor, defect, P3)

Description

[Cristian Fogel [:cfogel]]

Attachment: xcss.png

Affected versions

• 88.0a1(2021-03-25), 87.0b2

Affected platforms

• Windows 10;

Steps to reproduce

• not sure how to trigger it consistently, happened slow-loading internet spikes;

1. Launch Firefox, enable DevTools set it to dark theme;
2. Open the Stylesheet tab;
3. Load any webpage;

Expected result

• Stylesheet error - [ x ] close button is visible;

Actual result

• with the current color scheme the [ x ] close button is not visible on Dark Theme (dark grey background + black icon);

Regression range

• cannot get a proper regression range until the message can be consistently triggered;
• Last good: 2019-09-27 build that I managed to confirm;

Additional notes

• attached "good version" screenshot;
• attached recording to best illustrate the issue on the next comment.

Comment 1

[Cristian Fogel [:cfogel]]

Attachment: xstylle.gif

Comment 2

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Thank you Cristian for the report, I can reproduce it on my machine Win10 + Fx Nightly

Edit: the description has changed, not clear how to reproduce now.

Honza

Comment 3

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Cristian, the description is talking about "the eye icon appears pixelated or deformed at times while loading" also in bug 1701555.
Cloned description?

Comment 4

[Cristian Fogel [:cfogel]]

Indeed, cloned and jumped over the section when updating data for this bug.
Thanks Honza for pointing it out, have updated the initial comment with correct description.

Comment 5

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Is there any page I could load to see the "Stylesheet could not be loaded" error?

Comment 6

[Cristian Fogel [:cfogel]]

With the initial report it was noticed it on https://www.stirileprotv.ro, https://www.bbc.com/and some links from the Pocket-newtab.
Nothing consistent and reproducible 100% of the time though.

Comment 7

[Alexandru Trif, Desktop QA [:atrif]]

Attachment: testcase.html

Attaching a test case found here as well as regression range made on Windows 10x64. It seems that the test case must be saved locally in order to reproduce the issue.

Last good revision: 5e43a93a9dc5eac61977cd87cf45d8d27565dcf5
First bad revision: c9a470a98696c8524f73d97d0255abc2a52ffa62
Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=5e43a93a9dc5eac61977cd87cf45d8d27565dcf5&tochange=c9a470a98696c8524f73d97d0255abc2a52ffa62

Comment 8

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Thank you Alexandru!

Note that the test case in comment #7 needs to be loaded through the file: protocol

This change might caused the problem. We should look at what CSS was broken by that.
https://hg.mozilla.org/integration/autoland/rev/8dae0f4af8f85654c68b853133d658aacdb9ea20

Honza

Comment 9

[Gagah Pangeran Rosfatiputra [:GPR]]

Hello, I would like to work on this bug.

Comment 10

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Thank you for the help Gagah.
Please don't forget to look at the comment #8 and see why this bug happened in the first place.

Comment 11

[Gagah Pangeran Rosfatiputra [:GPR]]

Attachment: close button hover

So the regression happens because messageCloseButton is changed from div to button. It makes currentColor in messageCloseButton css using inherit color from default firefox button style.

The fix is pretty simple, just add color: inherit or color: currentColor for messageCloseButton css to explicitly inherit or use color from its parent.

The next problem is background-color for .messageCloseButton:hover is hardcoded to gray. What color/variable should I use to change this background-color?

Comment 12

[Gagah Pangeran Rosfatiputra [:GPR]]

Hello, I still need information about background-color for .messageCloseButton:hover because currently it's hardcorded to gray and the background color is blending with the cross color.

Comment 13

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Julian, can you please help here, thank you!

Comment 14

[Julian Descottes [:jdescottes]]

Reusing the variables we use on the main toolbar should be fine here.

For fill (https://searchfox.org/mozilla-central/rev/a8773ba2a8f015e0525f219a7e55087b04d30258/devtools/client/shared/components/NotificationBox.css#98) you can use var(--theme-icon-color).

For background-color (https://searchfox.org/mozilla-central/rev/a8773ba2a8f015e0525f219a7e55087b04d30258/devtools/client/shared/components/NotificationBox.css#102), you can use var(--theme-button-active-background).

We also need to remove filter: invert(1) at https://searchfox.org/mozilla-central/rev/a8773ba2a8f015e0525f219a7e55087b04d30258/devtools/client/shared/components/NotificationBox.css#103

Those variables automatically change depending on the theme.

Comment 15

[Gagah Pangeran Rosfatiputra [:GPR]]

Thanks for the information! Now I can continue to work on the fix.

Comment 16

[Gagah Pangeran Rosfatiputra [:GPR]]

Attachment: Bug 1701562 - Change message close button fill and hover background color. r=nchevobbe,jdescottes

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c95906cdbbea
Change message close button fill and hover background color. r=jdescottes

Comment 18

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/c95906cdbbea

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Should we nominate this for Beta uplift?

Comment 20

[Julian Descottes [:jdescottes]]

We can let it ride the trains, it's a minor visual nit and it regressed in 85.

Comment 21

[Ciprian Georgiu, Desktop QA]

I have reproduced this bug using the test case from comment 7, on an affected Nightly build 2021-03-28.

The issue is verified as fixed on Beta 90.0b4 under Win 10 x64.