1377789 - Transition between tab bar and address bar is broken with Light Theme/Appearance/Persona after landing patch from bug #1367439

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

STR:
1. Use some Light Theme/Appearance/Persona
and see that transition between tab bar and address bar is broken



"Speedy" Regression window (mozilla-central)
Good:
https://ftp.mozilla.org/pub/firefox/nightly/2017/07/2017-07-01-03-02-03-mozilla-central/

Bad:
https://ftp.mozilla.org/pub/firefox/nightly/2017/07/2017-07-02-03-02-04-mozilla-central/

Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=587daa4bdc4b40b7053f4ca3b74723ca747f3b52&tochange=4d3de12dcdc539f14fcb06539da39fa7176c8955

Probably caused by:
bug #1367439 [Firefox:Theme]-Update toolbar background colors on OS X and Windows, update customize mode background to match
bug #1363502 [Firefox:Theme]-Implement new identity block appearance
bug #1350210 [Firefox:Toolbars and Customization]-Add setting for Compact and Touch theme modes (a.k.a. "Density" control)

Comment 1

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Attachment: bug.png

Comment 2

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Attachment: bug (500%).png

Comment 3

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Attachment: no bug.png

Comment 4

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Attachment: no bug (500%).png

Comment 5

[Dale Harvey (:daleharvey)]

Yes, this is visible without lwtheme as well (although less pronounced) and will happen until https://bugzilla.mozilla.org/show_bug.cgi?id=1349555 is landed (after the 57 merge)

Comment 9

[Dão Gottwald [:dao]]

Actually bug 1349555 won't solve this. According to the mockups, we need to keep using @toolbarHighlight@ or something similar when using a lightweight theme. When not using a lightweight theme, the color change from bug 1367439 is barely visible to the naked eye, so I suggest we revert these changes on Windows.

Comment 10

[Dão Gottwald [:dao]]

Attachment: Bug 1377789 - Use toolbarHighlight and -moz-dialog instead of --toolbar-background-color on Windows.

Review commit: https://reviewboard.mozilla.org/r/156136/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/156136/

Comment 11

[Dale Harvey (:daleharvey)]

We are using --toolbar-background-color in the customise panel but not the toolbar background which gives us an inconsistent colour and a misleading name for that variable. If we do this should we not remove --toolbar-background-color and also have it use -moz-dialog? if we do that I would like to see how we could loop this through the design process because it seems a little arbitrary which parts of the spec we are implementing and which parts we arent

Alternatively can we not just use toolbarhighlight in lightweighttheme mode and the spec colours when not in lwtheme?

Comment 12

[Dão Gottwald [:dao]]

(In reply to Dale Harvey (:daleharvey) from comment #11)
> We are using --toolbar-background-color in the customise panel but not the
> toolbar background which gives us an inconsistent colour

Like I said that's not visible to the naked eye on Windows.

> and a misleading
> name for that variable. If we do this should we not remove
> --toolbar-background-color

Probably, but we don't need to do that now. I'd wait till the dust has settled a bit.

> and also have it use -moz-dialog?

Note that -moz-dialog is darker than -moz-dialog + toolbarHighlight, so that would actually give us inconsistency.

Comment 13

[Dale Harvey (:daleharvey)]

> Like I said that's not visible to the naked eye on Windows.

> Note that -moz-dialog is darker than -moz-dialog + toolbarHighlight, so that would 
> actually give us inconsistency.

Having them set to the same value seems like the most obvious way to ensure that they dont become inconsistent even if they visually dont look that different now, but will defer to your experience here, if you could explain why not it would probably be helpful for me

Comment 14

[Dale Harvey (:daleharvey)]

Comment on attachment 8885269 [details]
Bug 1377789 - Use toolbarHighlight and -moz-dialog instead of --toolbar-background-color on Windows.

https://reviewboard.mozilla.org/r/156136/#review161252

Comment 15

[Dão Gottwald [:dao]]

(In reply to Dale Harvey (:daleharvey) from comment #13)
> > Like I said that's not visible to the naked eye on Windows.
> 
> > Note that -moz-dialog is darker than -moz-dialog + toolbarHighlight, so that would 
> > actually give us inconsistency.
> 
> Having them set to the same value seems like the most obvious way to ensure
> that they dont become inconsistent even if they visually dont look that
> different now, but will defer to your experience here, if you could explain
> why not it would probably be helpful for me

It's not possible to set --toolbar-background-color to -moz-dialog + toolbarHighlight. It could maybe work if this used the background shorthand rather than background-color.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3b419fb92d30
Use toolbarHighlight and -moz-dialog instead of --toolbar-background-color on Windows. r=daleharvey

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 18

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

I'm confirming that it's fixed, starting in Mozilla Firefox 56.0a1 (2017-07-13) (64-bit).
It's fixed partially, so leftover will be tracked in new bug #1380733.
Thanks. I'm marking this bug as VERIFIED.