rebranding should affect the name of the executable

REOPENED

Status

()

Firefox
Build Config
--
enhancement
REOPENED
11 years ago
11 years ago

People

(Reporter: Mook, Assigned: O. Atsushi (Torisugari))

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [linux+windows fixed], URL)

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

11 years ago
When we rebrand firefox (via --with-branding) there should also be some way to change the name of the executable.  It's kind of strange starting IceWeasel or FirePants or whatever by launching "firefox.exe"...

This sounds like a job for $(MOZ_APP_NAME) except that's not easily settable either.  Suiterunner appears to use that already.
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/configure.in&rev=1.1732&mark=4274#4270
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/suite/app/Makefile.in&rev=1.11&mark=55,57#50

Comment 1

11 years ago
How would a generic application written to open a link in Firefox find the alternative browser, esp. on Linux?
(Reporter)

Comment 2

11 years ago
Why would a generic application specifically code for Firefox as opposed to things like Opera, Konq, etc?  And why would somebody who wants to rebrand (i.e. in the position of Netscape or Flock) care? :)

Best guess is something like ShellExecute (Win32), Launch Services (Mac), /etc/alternatives (Linux).

Comment 3

11 years ago
> Why would

Not why would, they do. Given that there's no standard, most Linux applications hardcode a limited number of browsers. I've seen that for many if not most chat clients, IDEs, mailers.

> And why would somebody who wants to rebrand care?

Because it's a huge problem for the user and the distributor, if a click on a link in chat or email opens the also-installed Firefox instead of IceWeasel, or simply doesn't work at all. Esp. if the only reason is branding, i.e. this bug.
(Assignee)

Comment 4

11 years ago
(In reply to comment #3)
> Because it's a huge problem for the user and the distributor, if a click on a
> link in chat or email opens the also-installed Firefox instead of IceWeasel, or
> simply doesn't work at all. Esp. if the only reason is branding, i.e. this bug.

According to the Debian iceweasel diff against Fx 2.0.0.1

http://ftp.debian.org/debian/pool/main/i/iceweasel/iceweasel_2.0.0.1+dfsg-1.diff.gz

, Iceweasel's MOZ_APP_NAME is still "firefox", while MOZ_APP_DISPLAY_NAME
is "Iceweasel". So nothing changes after Mook's suggestion applied, as long as
they keep MOZ_APP_NAME as it is. This hack just give the chance to change the executable name via editing %DIR from -with-branding flag%/configure.sh

Removing brand hard-coding from mozilla/browser/app/Makefile.in should be
a good thing, anyway.
OS: Windows XP → All
Hardware: PC → All
(Assignee)

Comment 5

11 years ago
Created attachment 250578 [details] [diff] [review]
patch (Windows/Linux)

Note:
>-	firefox-config \
>...
>+	$(MOZ_APP_NAME)-config \

"firefox-config" is already a non-hardcoded file name.

http://lxr.mozilla.org/mozilla/search?string=%24%28MOZ_APP_NAME%29-config

Comment 6

11 years ago
I wrote:
> How would a generic application written to open a link in Firefox find the
> alternative browser, esp. on Linux?

Via mozilla-xremote-client, I guess
(Assignee)

Comment 7

11 years ago
(In reply to comment #6)
> 
> Via mozilla-xremote-client, I guess
> 

$> %SOMEWHERE%/mozilla-xremote-client -a firefox %SOME_COMMAND%

should work anyway, becauase that application name depends on the
appData (of lower cases version) defined in 

http://lxr.mozilla.org/mozilla/source/browser/app/nsBrowserApp.cpp#50

> 46 static const nsXREAppData kAppData = {
> 47   offsetof(nsXREAppData, xreDirectory),
> 48   nsnull,
> 49   "Mozilla",
> 50   "Firefox",
> 51   NS_STRINGIFY(APP_VERSION),
> 52   NS_STRINGIFY(BUILD_ID),
> 53   "{ec8030f7-c20a-464f-9b0e-13a3a9e97384}",
> 54   "Copyright (c) 1998 - 2007 mozilla.org",
> 55   NS_XRE_ENABLE_PROFILE_MIGRATOR |
> 56   NS_XRE_ENABLE_EXTENSION_MANAGER
> 57 }; 

and it has nothing to do with the executable name, if I understand
things correctly...?
(Assignee)

Updated

11 years ago
Attachment #250578 - Flags: review?(benjamin)
Attachment #250578 - Flags: review?(benjamin) → review+
Whiteboard: [checkin needed]
(Assignee)

Comment 8

11 years ago
Created attachment 251370 [details] [diff] [review]
Followup #1 (feeds)

Another hard-coding of executable name.
(Assignee)

Updated

11 years ago
Attachment #251370 - Attachment is obsolete: true
(Assignee)

Comment 9

11 years ago
Created attachment 251371 [details] [diff] [review]
Followup #1 (feeds)
(Assignee)

Updated

11 years ago
Attachment #251371 - Flags: review?(mano)
Comment on attachment 251371 [details] [diff] [review]
Followup #1 (feeds)

OK... there's similar code  in the feeds pref pane IIRC.
Attachment #251371 - Flags: review?(mano) → review+
(Assignee)

Comment 11

11 years ago
Created attachment 251393 [details] [diff] [review]
Followup #1 (feeds/preferences)

(In reply to comment #10)
> (From update of attachment 251371 [details] [diff] [review])
> OK... there's similar code  in the feeds pref pane IIRC.
> 

Thanks the review. Yes, I overlooked it.
Comments addressed.
Attachment #251371 - Attachment is obsolete: true
(Assignee)

Updated

11 years ago
Attachment #251393 - Flags: review?(mano)
Comment on attachment 251393 [details] [diff] [review]
Followup #1 (feeds/preferences)

r=mano
Attachment #251393 - Flags: review?(mano) → review+

Comment 13

11 years ago
Checked in both patches:
browser/app/Makefile.in                     1.118
browser/components/feeds/src/FeedWriter.js  1.35
browser/components/feeds/src/Makefile.in    1.11
browser/components/preferences/Makefile.in  1.5
browser/components/preferences/feeds.js     1.10
browser/installer/Makefile.in               1.26
browser/installer/unix/Makefile.in          1.12
browser/installer/unix/installer.cfg        1.9
browser/installer/unix/packages-static      1.89
browser/installer/windows/Makefile.in       1.41
browser/installer/windows/installer.cfg     1.23
browser/installer/windows/packages-static   1.99
browser/installer/windows/nsis/defines.nsi.in 1.5
Assignee: nobody → torisugari
Whiteboard: [checkin needed]

Updated

11 years ago
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Assignee)

Comment 14

11 years ago
(In reply to comment #13)
> Checked in both patches:
Thank you, asqueella.

This bug is not fixed on Mac. -> REOPEN
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 15

11 years ago
Created attachment 253082 [details] [diff] [review]
untested patch (Mac)

Minimum patch.


BTW, the problem is, there are a lot of variable names around, such as
MOZ_APP_NAME, MOZ_UI_APP_NAME, and just APP_NAME etc. and it's hard to
tell the differcne at a glance. So I think these things should be
cleanuped in the future, but probably in another bug.

That file in suiterunner looks neat.
http://lxr.mozilla.org/mozilla/source/suite/app/macbuild/Contents/Info.plist.in
(Assignee)

Updated

11 years ago
Whiteboard: [linux+windows fixed]
(Assignee)

Updated

11 years ago
Attachment #253082 - Flags: review?(benjamin)
If the app name is going to change then maybe using MOZ_APP_DISPLAYNAME would be a better option, since that would also fix bug 273977 and bug 275706. That change also appears to require a special case value of PROGRAM for the mac, using MOZ_APP_DISPLAYNAME again, see the patches in bug 273977. That's for thunderbird but the same should apply to the Firefox bug too. There are some consequences for the tinderboxes noted on 273977.

(Assignee)

Comment 17

11 years ago
(In reply to comment #16)
> If the app name is going to change then ...

These patches affect nothing unless you edit MOZ_APP_NAME.
So the app name is not going to change.

In terms of bug 275706, IMHO, both repackaged name and executable
name should be based on MOZ_APP_NAME, not on MOZ_APP_DISPLAYNAME.
I'm not too sure, but I guess Build Config gurus would know how to
make MOZ_APP_NAME title-cases ("firefox" -> "Firefox"), if you like
"Firefox.app" rather than "firefox.app".
Attachment #253082 - Flags: review?(benjamin) → review+
You need to log in before you can comment on or make changes to this bug.