Closed Bug 1377413 Opened 3 years ago Closed 3 years ago

Chrome UI broken in non-Photon mode.

Categories

(Firefox :: General, defect, P1)

defect

Tracking

()

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

People

(Reporter: johannh, Assigned: Gijs)

References

Details

(Keywords: regression, Whiteboard: [photon-structure])

Attachments

(2 files)

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.
(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)
 
+%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]
Iteration: --- → 56.2 - Jul 10
Has Regression Range: --- → yes
Flags: qe-verify+
QA Contact: gwimberly
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+
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
https://hg.mozilla.org/mozilla-central/rev/f987b4c70006
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Hi Gijs,

Any suggested STR to verify this bug? Just flip the pref off?
Flags: needinfo?(gijskruitbosch+bugs)
(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.
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)
Verified on Windows, Mac, and Linux.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Flags: needinfo?(gwimberly)
Blocks: 1387512
You need to log in before you can comment on or make changes to this bug.