1366363 - Rename Compact Dark and Compact Light themes to just "Dark" and "Light"

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, enhancement, P1)

Description

[Dão Gottwald [:dao]]

Firefox 57 will have compact and touch modes independently from current compact themes, but we want to keep these themes as dark/light alternatives to the default. We need to rename them accordingly.

Comment 1

[Michael Kohler [:mkohler]]

What should this be renamed to? I'm happy to do this if you tell me the new names. :)

Comment 2

[Dão Gottwald [:dao]]

This will have to wait till Nightly 57.

The new names should perhaps just be "Light" and "Dark". I don't have a better idea. Will need to check with Shorlander.

Comment 5

[Michael Kohler [:mkohler]]

Attachment: Bug 1366363 - Rename Compact Dark and Compact Light themes to just 'Dark' and 'Light'

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

Comment 6

[Michael Kohler [:mkohler]]

How should we handle "compact" references and IDs?

Examples:
https://dxr.mozilla.org/mozilla-central/source/browser/app/profile/firefox.js#1241 (do we need a migration?)
https://dxr.mozilla.org/mozilla-central/source/browser/base/content/browser-compacttheme.js#10

Comment 7

[Dão Gottwald [:dao]]

I don't think we need to worry about internal IDs for now, they're not visible to users in any way.

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 8893520 [details]
Bug 1366363 - Rename Compact Dark and Compact Light themes to just 'Dark' and 'Light'

https://reviewboard.mozilla.org/r/164608/#review169934

::: browser/components/nsBrowserGlue.js:630
(Diff revision 1)
>        LightweightThemeManager.addBuiltInTheme({
>          id: "[e-mail]",
> -        name: gBrowserBundle.GetStringFromName("compactLightTheme.name"),
> -        description: gBrowserBundle.GetStringFromName("compactLightTheme.description"),
> +        name: gBrowserBundle.GetStringFromName("lightTheme.name"),
> +        description: gBrowserBundle.GetStringFromName("lightTheme.description"),
>          headerURL: "resource:///chrome/browser/content/browser/defaultthemes/compact.header.png",
> -        iconURL: "resource:///chrome/browser/content/browser/defaultthemes/compactlight.icon.svg",
> +        iconURL: "resource:///chrome/browser/content/browser/defaultthemes/light.icon.svg",

The iconURL change is broken because you didn't rename the SVG file itself...

You could either revert this change or rename the SVG file.

Comment 9

[Michael Kohler [:mkohler]]

Comment on attachment 8893520 [details]
Bug 1366363 - Rename Compact Dark and Compact Light themes to just 'Dark' and 'Light'

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/164608/diff/1-2/

Comment 10

[Dão Gottwald [:dao]]

Comment on attachment 8893520 [details]
Bug 1366363 - Rename Compact Dark and Compact Light themes to just 'Dark' and 'Light'

https://reviewboard.mozilla.org/r/164608/#review169946

Thanks!

Comment 11

[Michael Kohler [:mkohler]]

Comment on attachment 8893520 [details]
Bug 1366363 - Rename Compact Dark and Compact Light themes to just 'Dark' and 'Light'

https://reviewboard.mozilla.org/r/164608/#review169950

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3f96fc2c078c
Rename Compact Dark and Compact Light themes to just 'Dark' and 'Light' r=dao

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

This seems to have caused a spike in failures like https://treeherder.mozilla.org/logviewer.html#?job_id=120808860&repo=autoland

This was a previously known intermittent but starting on your push, it started failing multiple times per push.

Backed out in https://hg.mozilla.org/integration/autoland/rev/a4e153426d4c5b2b862446c1e975530e9054dc72

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Hrm, those failures are still frequently happening on the backout... I guess this can reland as-is.

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

You're going to need to reopen the mozreview request if you want someone to land it for you.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/64526a900fd5
Rename Compact Dark and Compact Light themes to just 'Dark' and 'Light' r=dao

Comment 18

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/64526a900fd5

Comment 19

[Aki Sasaki (not active)]

https://hg.mozilla.org/projects/date/rev/3f96fc2c078cc6b6b150bbc191dbbaf2b9f39eea
Bug 1366363 - Rename Compact Dark and Compact Light themes to just 'Dark' and 'Light' r=dao

https://hg.mozilla.org/projects/date/rev/64526a900fd51d1fb784fca7eac2ae9b0bae30d1
Bug 1366363 - Rename Compact Dark and Compact Light themes to just 'Dark' and 'Light' r=dao

Comment 20

[Fahima Zulfath]

Managed to reproduce the issue on an affected build. Bug 1366363 is fixed & verified.
Version : Firefox Nightly 57.0a1
build ID: 20170809100326
OS      :Windows 10 X 64 bit. 

[bugday-20170809]

Comment 21

[Deac Alin]

I tested this issue on Mac Os X 10.12, Windows 10 x64, Ubuntu 16.04 with FF Nightly 57.0a1(2017-08-11) Compact Dark and Compact Light themes are now renamed to "Dark" and "Light". I will mark this as verified fixed.