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

VERIFIED FIXED in Firefox 56

Status

()

Firefox
Theme
P1
normal
VERIFIED FIXED
4 months ago
3 months ago

People

(Reporter: Virtual, Assigned: dao)

Tracking

({nightly-community, regression})

56 Branch
Firefox 56
x86_64
Windows 7
nightly-community, regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox54 unaffected, firefox55 unaffected, firefox56 verified)

Details

(Whiteboard: [photon-visual][p1])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(5 attachments)

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)
Created attachment 8882916 [details]
bug.png
Created attachment 8882917 [details]
bug (500%).png
Created attachment 8882918 [details]
no bug.png
Created attachment 8882919 [details]
no bug (500%).png

Updated

4 months ago
Whiteboard: [photon] [triage] → [photon-visual] [triage]
status-firefox54: --- → unaffected
status-firefox55: --- → unaffected
status-firefox56: --- → affected
status-firefox-esr52: --- → unaffected
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)
Depends on: 1349555
(Assignee)

Updated

3 months ago
Whiteboard: [photon-visual] [triage] → [photon-visual][p3]

Updated

3 months ago
Flags: qe-verify?
Priority: -- → P2
Duplicate of this bug: 1378843
Duplicate of this bug: 1377702
See Also: → bug 1379230
See Also: bug 1379230
Duplicate of this bug: 1379230
(Assignee)

Comment 9

3 months ago
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]
Comment hidden (mozreview-request)
(Assignee)

Updated

3 months ago
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Priority: P2 → P1

Updated

3 months ago
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?
(Assignee)

Comment 12

3 months ago
(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 14

3 months ago
mozreview-review
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+
(Assignee)

Comment 15

3 months ago
(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

3 months ago
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

Comment 17

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3b419fb92d30
Status: ASSIGNED → RESOLVED
Last Resolved: 3 months ago
status-firefox56: affected → fixed
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
Depends on: 1380733
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
status-firefox56: fixed → verified
Version: Trunk → 56 Branch

Updated

3 months ago
Flags: qe-verify?
QA Contact: Virtual
You need to log in before you can comment on or make changes to this bug.