1377413 - Chrome UI broken in non-Photon mode.

Status: VERIFIED FIXED

(Firefox :: General, defect, P1)

Description

[Johann Hofmann [:johannh] - On leave until October, slow/no response on BMO, DM me if urgent]

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 <[e-mail]>
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.

Comment 1

[Johann Hofmann [:johannh] - On leave until October, slow/no response on BMO, DM me if urgent]

Attachment: Screenshot

Comment 2

[:Gijs (he/him)]

(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" ?

Comment 3

[:Gijs (he/him)]

 
+%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.

Comment 4

[:Gijs (he/him)]

Attachment: Bug 1377413 - unbreak non-photon by adding a missing %endif,

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

Comment 5

[Dão Gottwald [::dao]]

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

https://reviewboard.mozilla.org/r/153654/#review158742

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f987b4c70006
unbreak non-photon by adding a missing %endif, r=dao

Comment 7

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/f987b4c70006

Comment 8

[u554753]

Hi Gijs,

Any suggested STR to verify this bug? Just flip the pref off?

Comment 9

[:Gijs (he/him)]

(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.

Comment 10

[:Gijs (he/him)]

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)

Comment 11

[u554753]

Verified on Windows, Mac, and Linux.