Change Firefox about dialogue visual on ESR

RESOLVED FIXED in Firefox 60

Status

()

P1
normal
RESOLVED FIXED
11 months ago
8 months ago

People

(Reporter: RT, Assigned: amin)

Tracking

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

Firefox Tracking Flags

(firefox60blocking fixed, firefox61 fixed)

Details

Attachments

(4 attachments)

(Reporter)

Description

11 months ago
Created attachment 8956810 [details]
Firefox Quantum on ESR about.png

The current ESR about dialogue visual (https://dxr.mozilla.org/comm-esr52/source/mozilla/browser/branding/official/content/about-wordmark.png) does not align with current release Firefox Quantum branding and after discussions with product and content marketing the recommendation is to update the visual as follows:
- 'Firefox Quantum', with subcopy of "Extended Support Release" underneath (see attached)

Notes:
- This has to be done on the ESR tree, not central
- The visual will be provided on this bug
- This does not require string changes
(Reporter)

Comment 1

11 months ago
Amin, can you please help us with producing the visual?
Flags: needinfo?(aalhazwani)

Comment 2

11 months ago
Tracked as blocking for 60/ESR60 to ensure this gets landed in a timely manner and is closely monitored by relman team.
status-firefox60: --- → affected
tracking-firefox60: --- → blocking
(Assignee)

Comment 3

11 months ago
(In reply to Romain Testard [:RT] from comment #1)
> Amin, can you please help us with producing the visual?

Here you can find the SVG version of the logo https://design.firefox.com/photon/visuals/product-identity-assets.html#firefox. For size and placement please refer to the Firefox Release about dialog.

Do we have any update on the name? Once we have a decision there I can also prepare the SVG for that as well!
Flags: needinfo?(aalhazwani)
Amin, I am assigning this bug to you, feel free to reassign to whoever is supposed to land this change.
Assignee: nobody → aalhazwani
Status: NEW → ASSIGNED
Priority: -- → P1
(Assignee)

Comment 5

11 months ago
Created attachment 8957499 [details]
firefox-enterprise-about-dialog.jpg

About dialog for Firefox Enterprise, Romain could you assign this bug to who is going to implement it? Thanks!

The assets are currently hosted here: https://dxr.mozilla.org/mozilla-central/source/browser/branding/official/content

- about-logo.png
- about-logo@2x.png
- about-woodmark.svg

Would be possible to also have the about-logo as an SVG? You can find the SVG file here https://design.firefox.com/photon/visuals/product-identity-assets.html#firefox
Flags: needinfo?(rtestard)
(Reporter)

Comment 6

11 months ago
Thanks Amin
Panos, can you please help assign this?
Also NI Felipe in case he's able to look into it.
Flags: needinfo?(rtestard)
Flags: needinfo?(past)
Flags: needinfo?(felipc)
This is Justin's turf.
Flags: needinfo?(past) → needinfo?(dolske)
(Reporter)

Updated

10 months ago
Blocks: 1433173
I can't commit to this right now as my main focus is getting the policies implemented. However, the biggest chunk of work here will be from UX to produce the new image; once that's done, replacing the old one can probably be done very quickly by any front-end engineer.
Flags: needinfo?(felipc)

Comment 9

10 months ago
Note that the main holdup here is the art and the art would have to be redone anyway (because it uses the old Firefox logo).

Who would be on the hook to redo the art work? I think Stephen Horlander did the Quantum work.
Flags: needinfo?(shorlander)
Hi there, as I explained in comment#5 all visual assets are already available on m-c.
Flags: needinfo?(shorlander)
so then are we going to add the Extended Support Release part as text?
If possible, yes.
Created attachment 8959015 [details]
With my code change

Here's what it looks like with the patch I'm about to attach.

I went with 125% over existing font.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Updated patch adds a margin on the right so it looks correct in RTL languages.

And before folks ask, I checked all the major languages of https://www.mozilla.org/en-US/firefox/organizations/all/.

Extended Support Release is NOT translated, so this is not a string for translation.

We'll also need to remove the hack from the ESR branch when we do this.

Updated

10 months ago
Flags: needinfo?(dolske)

Comment 17

10 months ago
mozreview-review
Comment on attachment 8959016 [details]
Bug 1443771 - Add ESR name to the about dialog. It is not translated.

https://reviewboard.mozilla.org/r/227880/#review234050

The code looks to me, but a couple of things before going ahead:

- You should check with Flod about having this string as an untranslatable string, and the desire to uplift this to beta

- I had heard that there was some sort of hack during the build process of ESR that replaced the (now) "Firefox Quantum" image with the older "Firefox ESR" image. We should make sure that won't be a problem, and check if there are any images to be removed from the tree and specially from being shipped but unused.

::: browser/base/content/aboutDialog.css:34
(Diff revision 2)
> +#esr {
> +  font-weight: bold;
> +  font-size: 125%;
> +  margin-top: 10px;
> +  margin-left: 0;
> +  margin-right: 0;

if this `margin-right: 0` was done to support RTL languages, you should instead use `margin-inline-start: 0`, which applies as margin-left on LTR and margin-right on RTL.
Attachment #8959016 - Flags: review?(felipc) → review+
flod said:

"the string should be hard coded in the XUL/HTML template, or JS code. If you expose it, it will be translated for sure"

As far as the ESR hack, I plan to open a bug against release engineering. There was some hack that was done in 38 that has been carried forward.

> if this `margin-right: 0` was done to support RTL languages, you should instead use `margin-inline-start: 0`, which applies as margin-left on LTR and margin-right on RTL.

Good point. I was copying the other code.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
I've changed my tack on this a little. I made release generic in the XUL and set the title via JS. That way this is more general than just the ESR case.
Comment on attachment 8959016 [details]
Bug 1443771 - Add ESR name to the about dialog. It is not translated.

Requesting Jared's feedback on the new patch since he seems to be the goto person for about changes.
Attachment #8959016 - Flags: feedback?
Mike: the feedback flag here is blank...
Flags: needinfo?(mozilla)
Comment on attachment 8959016 [details]
Bug 1443771 - Add ESR name to the about dialog. It is not translated.

doh! Also doing a needinfo.
Flags: needinfo?(mozilla) → needinfo?(jaws)
Attachment #8959016 - Flags: feedback? → feedback?(jaws)
Comment on attachment 8959016 [details]
Bug 1443771 - Add ESR name to the about dialog. It is not translated.

https://reviewboard.mozilla.org/r/227880/#review234842

::: browser/base/content/aboutDialog.js:83
(Diff revision 4)
>          currentChannelText.hidden = true;
>    }
>  
> +  if (AppConstants.MOZ_UPDATE_CHANNEL == "esr") {
> +    let release = document.getElementById("release");
> +    release.textContent = "Extended Support Release";

Can you add a comment here to say that this is explicitly not localizable?

> I've changed my tack on this a little. I made release generic in the XUL and set the title via JS. That way this is more general than just the ESR case.

Since this isn't used outside of ESR I'm not sure what we're gaining by setting the string via JS. I'd prefer to keep the string in the markup, and then here we would just make it visible.
Attachment #8959016 - Flags: review+
> Since this isn't used outside of ESR I'm not sure what we're gaining by setting the string via JS. I'd prefer to keep the string in the markup, and then here we would just make it visible.

I'm picturing a world where we might have other custom releases and use this string. But I can do it thr other way if you think that's better.
Comment hidden (mozreview-request)

Updated

10 months ago
Blocks: 1447399

Comment 28

10 months ago
mozreview-review-reply
Comment on attachment 8959016 [details]
Bug 1443771 - Add ESR name to the about dialog. It is not translated.

https://reviewboard.mozilla.org/r/227880/#review234842

> Can you add a comment here to say that this is explicitly not localizable?
> 
> > I've changed my tack on this a little. I made release generic in the XUL and set the title via JS. That way this is more general than just the ESR case.
> 
> Since this isn't used outside of ESR I'm not sure what we're gaining by setting the string via JS. I'd prefer to keep the string in the markup, and then here we would just make it visible.

Comment added and code moved.

Comment 29

10 months ago
Pushed by mozilla@kaply.com:
https://hg.mozilla.org/integration/autoland/rev/0fd93dddaf2c
Add ESR name to the about dialog. It is not translated. r=Felipe,jaws
Comment on attachment 8959016 [details]
Bug 1443771 - Add ESR name to the about dialog. It is not translated.

Approval Request Comment
[Feature/Bug causing the regression]: ESR identifier
[User impact if declined]: Unable to identify ESR
[Is this code covered by automated tests?]: N/A
[Has the fix been verified in Nightly?]: Not yet
[Needs manual test from QE? If yes, steps to reproduce]: No. Can't until we have an ESR beta
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: no
[Why is the change risky/not risky?]: Only affects ESR
[String changes made/needed]: None
Attachment #8959016 - Flags: approval-mozilla-beta?
https://hg.mozilla.org/mozilla-central/rev/0fd93dddaf2c
Status: ASSIGNED → RESOLVED
Last Resolved: 10 months ago
status-firefox61: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Comment on attachment 8959016 [details]
Bug 1443771 - Add ESR name to the about dialog. It is not translated.

about dialog update for esr60, beta60+
Attachment #8959016 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 33

10 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/9ba5a964b20d
status-firefox60: affected → fixed

Comment 34

8 months ago
mozreview-review
Comment on attachment 8959016 [details]
Bug 1443771 - Add ESR name to the about dialog. It is not translated.

https://reviewboard.mozilla.org/r/227880/#review248812

::: browser/base/content/aboutDialog.js:81
(Diff revision 5)
>      channelLabel.value = UpdateUtils.UpdateChannel;
>      if (/^release($|\-)/.test(channelLabel.value))
>          currentChannelText.hidden = true;
>    }
>  
> +  if (AppConstants.MOZ_UPDATE_CHANNEL == "esr") {

This is an abuse of the MOZ_UPDATE_CHANNEL variable. This makes downstream not show the string because their update channel is "default".

Comment 35

8 months ago
mozreview-review-reply
Comment on attachment 8959016 [details]
Bug 1443771 - Add ESR name to the about dialog. It is not translated.

https://reviewboard.mozilla.org/r/227880/#review248812

> This is an abuse of the MOZ_UPDATE_CHANNEL variable. This makes downstream not show the string because their update channel is "default".

I'm not sure what you mean by downstream? I thought the default update channel was only used for private builds, never in release.
downstream meaning Linux distros.
(In reply to Julien Cristau [:jcristau] from comment #36)
> downstream meaning Linux distros.

Unfortunately there is no other way to identify the ESR except for the update channel.

Can you think of something we missed?
Adding needinfo for this question. How do you suggest we identify the ESR?

Do we know how the distros are setting the correct build flags for building ESR?
Flags: needinfo?(mh+mozilla)
There was bug 1432737, but it was backed out :(
ESR used to have a different branding content, did that go away?
Flags: needinfo?(mh+mozilla)
> ESR used to have a different branding content, did that go away?

Yes. We were going to have to redo the logo anyway (for Quantum) and we thought just adding text would be easier.

I didn't realize that the reason they went with an image is so it could be easily checked into the branch and downstream folks wouldn't have to worry about it.

This is going to come up with other things too (policy and autoconfig) so we're probably going to need an easy way to identify ESR anyway.

I wonder if bug 1432737 will make it in eventually?
You need to log in before you can comment on or make changes to this bug.