Closed Bug 1377789 Opened 5 years ago Closed 5 years ago

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

Categories

(Firefox :: Theme, defect, P1)

56 Branch
x86_64
Windows 7
defect

Tracking

()

VERIFIED FIXED
Firefox 56
Iteration:
56.3 - Jul 24
Tracking Status
firefox-esr52 --- unaffected
firefox54 --- unaffected
firefox55 --- unaffected
firefox56 --- verified

People

(Reporter: Virtual, Assigned: dao)

References

Details

(Keywords: nightly-community, regression, Whiteboard: [photon-visual][p1])

Attachments

(5 files)

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)
Flags: needinfo?(dale)
Whiteboard: [photon] [triage] → [photon-visual] [triage]
Has Regression Range: --- → yes
Has STR: --- → yes
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)
Flags: needinfo?(dale)
Whiteboard: [photon-visual] [triage] → [photon-visual][p3]
Flags: qe-verify?
Priority: -- → P2
Duplicate of this bug: 1378843
See Also: → 1379230
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.
No longer depends on: 1349555
Whiteboard: [photon-visual][p3] → [photon-visual][p1]
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Priority: P2 → P1
Iteration: --- → 56.3 - Jul 24
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?
(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.
> 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 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
Attachment #8885269 - Flags: review?(dharvey) → review+
(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.
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3b419fb92d30
Use toolbarHighlight and -moz-dialog instead of --toolbar-background-color on Windows. r=daleharvey
https://hg.mozilla.org/mozilla-central/rev/3b419fb92d30
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Summary: Transition between tab bar and address bar is broken with Light Theme/Appearance/Persona → Transition between tab bar and address bar is broken with Light Theme/Appearance/Persona after landing patch from bug #1367439
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.
Status: RESOLVED → VERIFIED
Version: Trunk → 56 Branch
Flags: qe-verify?
You need to log in before you can comment on or make changes to this bug.