Chrome UI broken in non-Photon mode.

VERIFIED FIXED in Firefox 56

Status

()

defect
P1
normal
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: johannh, Assigned: Gijs)

Tracking

(Blocks 1 bug, {regression})

unspecified
Firefox 56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

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

Details

(Whiteboard: [photon-structure])

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
I just tried to actually run non-Photon Firefox and was unpleasantly surprised. The UI is totally screwed. See screenshot. Bisecting gave me this result:

$ hg bisect -b
The first bad revision is:
changeset:   366590:518a396f9e78
user:        Gijs Kruitbosch <gijskruitbosch@gmail.com>
date:        Wed Jun 28 22:03:04 2017 +0100
summary:     Bug 1354145 - fix background colour across customize mode as well as integration with the footer, r=daleharvey

I only tried it on Windows but it might affect other platforms.
(Reporter)

Comment 1

2 years ago
Posted image Screenshot
(Assignee)

Comment 2

2 years ago
(In reply to Johann Hofmann [:johannh] from comment #0)
> I just tried to actually run non-Photon Firefox and was unpleasantly
> surprised.

Define "non-Photon Firefox" ?
Flags: needinfo?(jhofmann)
(Assignee)

Comment 3

2 years ago
 
+%ifdef MOZ_PHOTON_THEME
+:root {
+  --toolbar-background-color: -moz-dialog;
+  --toolbar-foreground-color: -moz-dialogtext;
+}
+/* Colour to use for toolbars and customize mode */
+%if defined(XP_WIN) || defined(XP_MACOSX)
+%ifdef XP_WIN
+@media (-moz-windows-default-theme) {
+%endif
+:root {
+  --toolbar-background-color: #f9f9fa;
+  --toolbar-foreground-color: #0c0c0d;
+}
+%ifdef XP_WIN
+}
+%endif
+%endif
+
+

There are 4 %if*s here, and 3 %endifs. I don't understand why the preprocessor doesn't complain. Anyway, in non-photon all of browser.css is gone.
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Flags: needinfo?(jhofmann)
Whiteboard: [photon-structure]
(Assignee)

Updated

2 years ago
Iteration: --- → 56.2 - Jul 10
Has Regression Range: --- → yes
Flags: qe-verify+
QA Contact: gwimberly
Comment hidden (mozreview-request)

Comment 5

2 years ago
mozreview-review
Comment on attachment 8882527 [details]
Bug 1377413 - unbreak non-photon by adding a missing %endif,

https://reviewboard.mozilla.org/r/153654/#review158742
Attachment #8882527 - Flags: review?(dao+bmo) → review+

Comment 6

2 years ago
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f987b4c70006
unbreak non-photon by adding a missing %endif, r=dao
Priority: -- → P1

Comment 7

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f987b4c70006
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56

Comment 8

2 years ago
Hi Gijs,

Any suggested STR to verify this bug? Just flip the pref off?
Flags: needinfo?(gijskruitbosch+bugs)
(Assignee)

Comment 9

2 years ago
(In reply to Grover Wimberly IV [:Grover-QA] from comment #8)
> Hi Gijs,
> 
> Any suggested STR to verify this bug? Just flip the pref off?

No, you'd have to rebuild with a different toolkit/moz.configure file. Generally speaking, we do this on cedar, but it seems that hasn't seen a merge for a while and the automated tests there are failing. I'll try to see what can be done about that today, leaving ni for that.
(Assignee)

Comment 10

2 years ago
Grover, you should be able to try with a build off https://treeherder.mozilla.org/#/jobs?repo=cedar&revision=458bf72f6fa7a90e2f58d7778140c7a43f727e47 once they finish (looks like OS X is available now, Windows/Linux should be around in an hour or two)
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(gwimberly)

Comment 11

2 years ago
Verified on Windows, Mac, and Linux.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Flags: needinfo?(gwimberly)

Updated

2 years ago
Blocks: 1387512
You need to log in before you can comment on or make changes to this bug.