Closed Bug 1378834 Opened 7 years ago Closed 6 years ago

Change brandFullName to 'Firefox Nightly' in nightly builds

Categories

(Firefox :: General, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 59
Tracking Status
firefox57 --- wontfix
firefox58 --- wontfix
firefox59 --- fixed

People

(Reporter: pascalc, Assigned: Sylvestre)

References

(Depends on 3 open bugs)

Details

Attachments

(2 files)

We want people to use Nightly and to associate it with Firefox, currenlty our branding strings (brandShortName and brandFullName) only call it Nightly.

Since we want more people to use Nightly, updating these strings for Firefox Nightly makes sense.
Blocks: 1351268
I guess this is a pretty nice good-first-bug, can you point out the location of those strings to be changed so that I can work on this, also, do you know anyone who would be okay with mentoring this and reviewing the patch ?
Thanks
Flags: needinfo?(pascalc)
I think this needs to be evaluated by official team.
Product: Firefox → Release Engineering
Version: Trunk → unspecified
No longer blocks: 1351268
Keywords: uiwanted
Product: Release Engineering → Firefox
Summary: Nightly should be built as 'Firefox Nightly' → Change brandFullName to 'Firefox Nightly' in nightly builds
Flags: needinfo?(pascalc)
Jeff, can you make a call here?
Flags: needinfo?(jgriffiths)
Seems reasonable but you need to message this across teams, particularly mozilla.org ( David Tenser ) and SUMO to get websites and whatever else in sync if they're using "Nightly" without the Firefox.

+1
Flags: needinfo?(jgriffiths)
I will try to do a patch (I did it for Fennec)
Assignee: nobody → sledru
Attachment #8906292 - Flags: review?(dtownsend)
Comment on attachment 8906292 [details]
Bug 1378834 - Rename Nightly to Firefox Nightly

https://reviewboard.mozilla.org/r/178018/#review183432

r+ for most of this but please have mhowell review the nsi change, he was sat next to me and questioning why you weren't just changing BrandFullNameInternal.
Attachment #8906292 - Flags: review?(dtownsend) → review+
Attachment #8906292 - Flags: review?(mhowell)
(In reply to Dave Townsend [:mossop] from comment #7)
> Comment on attachment 8906292 [details]
> Bug 1378834 - Rename Nightly to Firefox Nightly
> 
> https://reviewboard.mozilla.org/r/178018/#review183432
> 
> r+ for most of this but please have mhowell review the nsi change
Done, thanks

>  he was sat next to me and questioning why you weren't just changing
> BrandFullNameInternal.

Because the comment above is pretty scary:
# BrandFullNameInternal is used for some registry and file system values
# instead of BrandFullName and typically should not be modified.
(In reply to Sylvestre Ledru [:sylvestre] from comment #8)
> Because the comment above is pretty scary:
> # BrandFullNameInternal is used for some registry and file system values
> # instead of BrandFullName and typically should not be modified.

Yeah, I had forgotten about that. The problem is that over in defines.nsi.in we define BrandFullName as BrandFullNameInternal, so a) that needs to be changed as well, and b) there might be negative implications of those things no longer being the same.

Let me look into it for a bit.
(In reply to Matt Howell [:mhowell] from comment #9)
> Let me look into it for a bit.

Okay, we're good; BrandShortName is only used for shortcuts, which we support changing the name of, and for logs. I was worried about registry entries, but those do use BrandFullNameInternal.

But we do need to remove
http://searchfox.org/mozilla-central/source/browser/installer/windows/nsis/defines.nsi.in#39
as part of this, and move that define into the other branding files.
Comment on attachment 8906292 [details]
Bug 1378834 - Rename Nightly to Firefox Nightly

https://reviewboard.mozilla.org/r/178018/#review183436

::: browser/branding/nightly/branding.nsi:12
(Diff revision 1)
>  # The unofficial build branding.nsi is located in browser/branding/unofficial/
>  
>  # BrandFullNameInternal is used for some registry and file system values
>  # instead of BrandFullName and typically should not be modified.
>  !define BrandFullNameInternal "Nightly"
> +!define BrandFullName         "Firefox Nightly"

BrandFullName is currently defined in defines.nsi.in, which might supercede this, so that needs to be removed over there. That means the other branding.nsi files need to get BrandFullName defined in them as well (just defining it to ${BrandFullNameInternal} as it currently is in defines.nsi.in will work fine for those).
(In reply to Jeff Griffiths (:canuckistani) (:⚡︎) from comment #4)
> Seems reasonable but you need to message this across teams, particularly
> mozilla.org ( David Tenser ) and SUMO to get websites and whatever else in
> sync if they're using "Nightly" without the Firefox.
> 
> +1

SUMO only has one page dedicated to Nightly https://support.mozilla.org/en-US/kb/using-dedicated-profile-firefox-nightly and it is already using Firefox Nightly as product name.

On mozilla.org, we already say Firefox Nightly on our download pages:
https://www.mozilla.org/en-US/firefox/channel/desktop/#nightly
https://www.mozilla.org/en-US/firefox/nightly/all/

Our styleguide was already using Firefox Nightly as wordmark and in its wording:
https://www.mozilla.org/en-US/styleguide/identity/firefox/wordmarks/#firefox-nightly

MDN has one page about Firefox Nightly features and also calls it Firefox Nightly
https://developer.mozilla.org/en-US/Firefox/Experimental_features

I'll send the message to David and other web content owners about the product name change but fortunately it seems that all of Mozilla already talks about Nightly as Firefox Nightly :)
Should the unofficial branding (which I assume is for local builds, not builds we propose for download) be also updated?
http://searchfox.org/mozilla-central/source/browser/branding/unofficial/locales/en-US/brand.dtd
Comment on attachment 8906927 [details]
bug 1378834 - Update of the installers to reflect the nightly name change

https://reviewboard.mozilla.org/r/178660/#review183846

Thanks!
Attachment #8906927 - Flags: review?(mhowell) → review+
Comment on attachment 8906292 [details]
Bug 1378834 - Rename Nightly to Firefox Nightly

https://reviewboard.mozilla.org/r/178018/#review183890
Attachment #8906292 - Flags: review?(pascalc) → review+
Pushed by sledru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d534d9e94ffb
Rename Nightly to Firefox Nightly r=mossop,pascalc
https://hg.mozilla.org/integration/autoland/rev/352457b9e339
Update of the installers to reflect the nightly name change r=mhowell
Backed out for Windows installer bustage: "BrandFullName" not defined:

https://hg.mozilla.org/integration/autoland/rev/5951cc2057a14fb414194127feeb3d82afda9205
https://hg.mozilla.org/integration/autoland/rev/634905bd50ef0080ac933d8e4613c5577cb52b7e

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=352457b9e339c04a8bb77f9355bc92485ec73544&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=130407391&repo=autoland

18:20:50     INFO -  mozmake.EXE[6]: Entering directory 'z:/build/build/src/obj-firefox/browser/installer/windows'
18:20:50     INFO -  cd instgen && c:/mozilla-build/nsis-3.01/makensis-3.01.exe  maintenanceservice_installer.nsi
18:20:50     INFO -  Processing config: c:\mozilla-build\nsis-3.01\nsisconf.nsh
18:20:50     INFO -  Processing script file: "maintenanceservice_installer.nsi" (ACP)
18:20:50     INFO -  !undef: "BrandFullName" not defined!
18:20:50     INFO -  Error in script "maintenanceservice_installer.nsi" on line 54 -- aborting creation process
18:20:50     INFO -  z:/build/build/src/toolkit/mozapps/installer/windows/nsis/makensis.mk:98: recipe for target 'maintenanceservice_installer' failed
18:20:50     INFO -  mozmake.EXE[6]: *** [maintenanceservice_installer] Error 1
18:20:50     INFO -  mozmake.EXE[6]: Leaving directory 'z:/build/build/src/obj-firefox/browser/installer/windows'
18:20:50     INFO -  Makefile:20: recipe for target 'libs' failed
18:20:50     INFO -  mozmake.EXE[5]: *** [libs] Error 2
Flags: needinfo?(sledru)
Matt told me on irc:
<mhowell> so that !undef line needs to get wrapped in an !ifdef BrandFullName / !endif
I will fix that probably tomorrow
Flags: needinfo?(sledru)
See Also: → 1367912
Pushed by sledru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3133f6a664c7
Rename Nightly to Firefox Nightly r=mossop,pascalc
https://hg.mozilla.org/integration/autoland/rev/3ad6c35d7220
Update of the installers to reflect the nightly name change r=mhowell
Has somebody reviewed where we use brandShortName from the dtd and properties files in-product? The whole point of having brandShortName vs. brandFullName is that there are contexts where the full string is too long, looks or feels cumbersome etc., and making Nightly behave differently from release in this regard seems undesirable.
Flags: needinfo?(sledru)
Flags: needinfo?(pascalc)
Flags: needinfo?(dtownsend)
This isn't respected, for example:
<!ENTITY  brandShortName        "Firefox Developer Edition">
Flags: needinfo?(sledru)
Flags: needinfo?(pascalc)
Flags: needinfo?(dtownsend)
however, happy to change my patch if you feel strongly about it
(In reply to Sylvestre Ledru [:sylvestre] from comment #24)
> This isn't respected, for example:
> <!ENTITY  brandShortName        "Firefox Developer Edition">

Correct, we already broke our own pattern there, but that doesn't seem like a good enough reason to repeat that here. Thankfully that code is dead now anyway, afaik.

DevEdition was also so radically different that it wasn't a very useful testbed for the UI anyway. I care more about Nightly being consistent with Beta and Release.
> Thankfully that code is dead now anyway, afaik.
it is not dead, we are still shipping dev edition.

> I care more about Nightly being consistent with Beta and Release.
OK, so, you want me to fix it, i will push a follow up patch
Backed out for busted installers, at least on OS X:

https://hg.mozilla.org/integration/autoland/rev/a7367247baa45a50f0082682f087c71b2d2e771f
https://hg.mozilla.org/integration/autoland/rev/4d573e9b4b0912b5673e514244e4ec4d18fe37df

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=3ad6c35d7220c4becb6b0dec19d0fba5ddb67ce0&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=130638750&repo=autoland

05:34:46     INFO - Getting output from command: ['/Users/cltbld/tasks/task_1505306020/build/venv/bin/mozinstall', '/Users/cltbld/tasks/task_1505306020/installer.dmg', '--destination', '/Users/cltbld/tasks/task_1505306020/build/application']
05:34:46     INFO - Copy/paste: /Users/cltbld/tasks/task_1505306020/build/venv/bin/mozinstall /Users/cltbld/tasks/task_1505306020/installer.dmg --destination /Users/cltbld/tasks/task_1505306020/build/application
05:34:57    ERROR - Errors received:
05:34:57     INFO - Reading from file tmpfile_stderr
05:34:57    ERROR -  Traceback (most recent call last):
05:34:57    ERROR -    File "/Users/cltbld/tasks/task_1505306020/build/venv/bin/mozinstall", line 9, in <module>
05:34:57    ERROR -      load_entry_point('mozInstall==1.13', 'console_scripts', 'mozinstall')()
05:34:57    ERROR -    File "/Users/cltbld/tasks/task_1505306020/build/venv/lib/python2.7/site-packages/mozinstall/mozinstall.py", line 360, in install_cli
05:34:57    ERROR -      install_path = install(src, options.dest)
05:34:57    ERROR -    File "/Users/cltbld/tasks/task_1505306020/build/venv/lib/python2.7/site-packages/mozinstall/mozinstall.py", line 126, in install
05:34:57    ERROR -      install_dir = _install_dmg(src, dest)
05:34:57    ERROR -    File "/Users/cltbld/tasks/task_1505306020/build/venv/lib/python2.7/site-packages/mozinstall/mozinstall.py", line 290, in _install_dmg
05:34:57    ERROR -      for appFile in os.listdir(appDir):
05:34:57    ERROR -  mozinstall.mozinstall.InstallError: Failed to install "/Users/cltbld/tasks/task_1505306020/installer.dmg ([Errno 2] No such file or directory: '/Volumes/Firefox')"
05:34:57    ERROR - Return code: 1
05:34:57  WARNING - setting return code to 3
Flags: needinfo?(sledru)
Blocks: 1399457
Jeff, is it too late to change this for 57, with comment 28 in particular?  If so, please mark 57wontfix.

Note: if we're ever going to change the name of the main Firefox binary, having this bug nailed is a pre-requisite.
Flags: needinfo?(jgriffiths)
This is a nightly only change, so, it won't impact 57 beta but yeah, let's wait until m-c == 58
Flags: needinfo?(sledru)
Flags: needinfo?(jgriffiths)
Depends on: 1404480
Keywords: uiwanted
Pushed by sledru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c508081bb938
Rename Nightly to Firefox Nightly r=mossop,pascalc
https://hg.mozilla.org/integration/autoland/rev/d60d9897fd39
Update of the installers to reflect the nightly name change r=mhowell
(In reply to Sylvestre Ledru [:sylvestre] from comment #27)
> > I care more about Nightly being consistent with Beta and Release.
> OK, so, you want me to fix it, i will push a follow up patch

So why did this land again with the change to brandShortName?
Flags: needinfo?(sledru)
Is there a reason why it's "FirefoxNightly" and not "Firefox Nightly" in the menu bar of macOS? I don't think that whitespaces are a problem, because other applications also have whitespaces, for example "Photoshop CC", so there seems to be another reason. In the dock it's "Firefox Nightly" with whitespace.
(In reply to Dão Gottwald [::dao] from comment #35)
> So why did this land again with the change to brandShortName?
I did in the other bug.

> Is there a reason why it's "FirefoxNightly" and not "Firefox Nightly" in the menu bar of macOS? 
For now, spaces in the name make the build fails on Mac. See comment #28.
This is on side, I will work on that later.
Flags: needinfo?(sledru)
Depends on: 1404717
I'm surprised that this landed without testing updates on a branch such as Oak first. This causes issues such as bug 1404717, i.e. we prompt all users again if they had "Nightly" set as default browser. Further, I'm concerned about bug 1404717 comment 2. macOS does a lot of name/signature matching to whitelist our browser. This could break things in a lot of unforeseen ways. We should have given QE a chance to find bugs and bang these issues out before landing on m-c.

(In reply to Sylvestre Ledru [:sylvestre] from comment #37)
> > Is there a reason why it's "FirefoxNightly" and not "Firefox Nightly" in the menu bar of macOS? 
> For now, spaces in the name make the build fails on Mac. See comment #28.
> This is on side, I will work on that later.

This seems like a simple matter of escaping a space and should probably have been fixed before landing. Displaying "FirefoxNightly" in the menu bar really doesn't look great.
Depends on: 1404796
(In reply to Stephen A Pohl [:spohl] from comment #38)
> I'm surprised that this landed without testing updates on a branch such as
> Oak first. This causes issues such as bug 1404717, i.e. we prompt all users
> again if they had "Nightly" set as default browser. Further, I'm concerned
> about bug 1404717 comment 2. macOS does a lot of name/signature matching to
> whitelist our browser. This could break things in a lot of unforeseen ways.
> We should have given QE a chance to find bugs and bang these issues out
> before landing on m-c.
Sorry about that, I didn't similar changes with Fennec, my tests with Linux didn't show any regression and the CI didn't show any symptoms. I didn't know that Mac was relying on package names :/ Sorry again.
How can I mitigate the impact of that?

> (In reply to Sylvestre Ledru [:sylvestre] from comment #37)
> This seems like a simple matter of escaping a space and should probably have
> been fixed before landing. Displaying "FirefoxNightly" in the menu bar
> really doesn't look great.
Indeed, I reported bug 1404796 about that.
Currently the Nightly app bundle is called >FirefoxFirefoxNightly.app<,
i'm guessing because of >--with-macbundlename-prefix=Firefox< and this change.
Matthias, Could you please report a bug for that? Thanks
Ryan, would you mind doing a backout of that patch?
Dolske is concerned about potential regressions and we don't have the bandwidth to manage potential unexpected regressions right now.

I will land that after 57 is released.
Flags: needinfo?(ryanvm)
Backed out. The next OSX nightlies will include the backout.

https://hg.mozilla.org/mozilla-central/rev/8f66e0613bd3b1fabb003f970bfe54fd0804e4ad
Status: RESOLVED → REOPENED
Flags: needinfo?(ryanvm)
Resolution: FIXED → ---
Target Milestone: Firefox 58 → ---
I've set up Balrog so mac nightly updates will be offered as they complete (aka removed the Darwin_x86_64-gcc3-u-i386-x86_64 section from Firefox-mozilla-central-nightly-latest, removed rule #654 which was pointing mac at Firefox-mozilla-central-nightly-20170929100122).
https://hg.mozilla.org/projects/oak/rev/20ff761d93b94103ecb285b60f6b86f95afb77e9
Bug 1378834 - Rename Nightly to Firefox Nightly r?mossup,pascalc

https://hg.mozilla.org/projects/oak/rev/84f309f63e57a975b55954a4bb5d60d6700372e1
bug 1378834 - Update of the installers to reflect the nightly name change r?mhowell
Depends on: 1406662
No longer blocks: 1399457
Depends on: 1399457
Comment on attachment 8906292 [details]
Bug 1378834 - Rename Nightly to Firefox Nightly

https://reviewboard.mozilla.org/r/178018/#review211780

::: browser/branding/nightly/branding.nsi:12
(Diff revision 4)
>  # The unofficial build branding.nsi is located in browser/branding/unofficial/
>  
>  # BrandFullNameInternal is used for some registry and file system values
>  # instead of BrandFullName and typically should not be modified.
>  !define BrandFullNameInternal "Nightly"
> +!define BrandFullName         "Firefox Nightly"

Does this have an effect, given https://searchfox.org/mozilla-central/rev/f5f1c3f294f89cfd242c3af9eb2c40d19d5e04e7/browser/installer/windows/nsis/defines.nsi.in#39 ?
Comment on attachment 8906292 [details]
Bug 1378834 - Rename Nightly to Firefox Nightly

https://reviewboard.mozilla.org/r/178018/#review211832
Attachment #8906292 - Flags: review?(dao+bmo)
See Also: → 1423934
Depends on: 1423856
Depends on: 1424294
Comment on attachment 8906292 [details]
Bug 1378834 - Rename Nightly to Firefox Nightly

https://reviewboard.mozilla.org/r/178018/#review214104

::: browser/branding/nightly/branding.nsi:12
(Diff revision 5)
>  # The unofficial build branding.nsi is located in browser/branding/unofficial/
>  
>  # BrandFullNameInternal is used for some registry and file system values
>  # instead of BrandFullName and typically should not be modified.
>  !define BrandFullNameInternal "Nightly"
> +!define BrandFullName         "Firefox Nightly"

See comment 48; I still have the same question.
Attachment #8906292 - Flags: review?(dao+bmo)
(In reply to Dão Gottwald [::dao] from comment #52)
> Comment on attachment 8906292 [details]
> Bug 1378834 - Rename Nightly to Firefox Nightly
> 
> https://reviewboard.mozilla.org/r/178018/#review214104
> 
> ::: browser/branding/nightly/branding.nsi:12
> (Diff revision 5)
> >  # The unofficial build branding.nsi is located in browser/branding/unofficial/
> >  
> >  # BrandFullNameInternal is used for some registry and file system values
> >  # instead of BrandFullName and typically should not be modified.
> >  !define BrandFullNameInternal "Nightly"
> > +!define BrandFullName         "Firefox Nightly"
> 
> See comment 48; I still have the same question.

Oh, I guess the other patch is supposed to deal with this. I hadn't looked at that. The order of these patches is a bit confusing.
Comment on attachment 8906292 [details]
Bug 1378834 - Rename Nightly to Firefox Nightly

https://reviewboard.mozilla.org/r/178018/#review214106
Attachment #8906292 - Flags: review+
Depends on: 1426083
Backout by dluca@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d381defe5fc2
Backed out changeset ffc76e7fc38b for failing Browser-chrome on toolkit/xre/test/browser_checkdllblockliststate.js on a CLOSED TREE
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f79bfb26b27f
Rename Nightly to Firefox Nightly r=mossop,pascalc,dao
https://hg.mozilla.org/integration/mozilla-inbound/rev/de73aaed82f3
Update of the installers to reflect the nightly name change r=mhowell,dao
https://hg.mozilla.org/mozilla-central/rev/f79bfb26b27f
https://hg.mozilla.org/mozilla-central/rev/de73aaed82f3
Status: REOPENED → RESOLVED
Closed: 7 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Flags: needinfo?(sledru)
Depends on: 1427000
Depends on: 1431477
You need to log in before you can comment on or make changes to this bug.