588874 - Replace Minefield with Firefox in UA string

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[[e-mail]]

A subset of bug 572665. Allowing sniffers to distinguish between Firefox, Fennec, and Seamonkey etc is perfectly reasonable. Which means that Minefield will break those sniffers. And if sites do sniff for it, they're not getting accurate information from the 'Minefield' string alone: it could be an old build representing a now-stable version.

We should make Minefield appear as close to Firefox as possible; for those that really care, they have the major version to play with. Comments for/against?

Comment 1

[Mike Shaver (:shaver emeritus)]

I'm in favour; it will reduce the amount of "false positive" reports we get which represent problems with sites, but which won't be problems for users of our released software.  That seems to me to be clearly a good economic trade, and wiser use of our resources.

I would be interested to know what our hit rate has been in getting people whose sites break with "Minefield" to switch to Gecko-neutral sniffing, versus those who a) don't bother, or b) just add Minefield to the Firefox path!

Comment 2

[Dave Garrett]

Note that without any indicator in the UA of being a nightly we run the risk of making support and bug reporting harder than necessary by not being able to easily tell what the user is running. I think replacing "Minefield" with "Firefox" is a great idea so long as other suggestions in other bugs to drop the build date, "pre", and alpha/beta indicators are not all done as well.

Just to be clear, this bug is also suggesting that any codename given to the future Firefox 4.0 branch not be in the UA string and instead simply list "Firefox" as well?

Comment 3

[[e-mail]]

I think we should keep alpha/beta/pre indicators. We'll probably drop build date. For support, I think the proposal (if we're not doing it already) is to have users copy/paste the text from Help/About, which can include whatever we want it to. (Most usefully, the hg rev it was built from.)

This would apply for future Firefox codenames.

Comment 4

[Mike Shaver (:shaver emeritus)]

This bug is about the User-Agent header only.  I believe that the other bugs to which you refer are better places to discuss surfacing that data (such as in about:support).

If we resolve this bug as I expect and hope, the UA string will contain "Firefox" for --enable-application=browser, for all trunk and branch builds descendent from the point at which we make the change.  Using per-version codenames in the UA string is a great way to borrow more trouble from spaces, etc.  Let's not.

Comment 5

[[e-mail]]

(Bug 572661 is about dropping build date, btw; if you want to discuss that, let's do it over there.)

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #3)
> I think we should keep alpha/beta/pre indicators. We'll probably drop build

There's another bug on removing those indicators - bug 582672 - arguing that presently the "pre" indicator ends up flagging false positive for Palm Pre optimized sites. Just FYI.

Comment 7

[Matthias Versen [:Matti]]

see bug 334967 for sites that break without Firefox in the UA

Comment 8

[[e-mail]]

This should be done ASAP, since all the other changes we're making to UA need to be well tested in our development builds, and having them not be 'Firefox' could jeopardize the quality of data.

Comment 9

[[e-mail]]

Attachment: patch

As advertised.

Comment 10

[[e-mail]]

Decided that we're doing this.

Comment 11

[Daniel Veditz [:dveditz]]

(In reply to comment #6)
> (In reply to comment #3)
> > I think we should keep alpha/beta/pre indicators.
> 
> There's another bug on removing those indicators - bug 582672

That's just about the string "pre", I haven't seen anyone argue that alphas and betas should be unidentified.

Comment 12

[Dave Garrett]

(In reply to comment #11)
> I haven't seen anyone argue that alphas and betas should be unidentified.

That was proposed as part of bug 572659, now WONTFIXed.

Comment 13

[Daniel C]

For reference, bug 334756 was the bug that initially changed the UA string of trunk builds to say Minefield.

Comment 14

[[e-mail]]

Attachment: patch v2

Changes 'MozillaDeveloperPreview' as well.

Comment 15

[[e-mail]]

Attachment: patch for 1.9.2

Same problem with Namoroka (bug 589608).

Comment 16

[Daniel C]

(In reply to comment #14)
> Created attachment 468329 [details] [diff] [review]
> patch v2
> 
> Changes 'MozillaDeveloperPreview' as well.

Unbranded builds will now advertise themselves as Firefox in the UA string. Can
someone clarify that this is OK from a trademark perspective?

http://www.mozilla.org/foundation/trademarks/policy.html says:

"Again, any modification to the Mozilla product, including adding to, modifying
in any way, or deleting content from the files included with an installer, file
location changes, added code, modification of any source files including
additions and deletions, etc., will require our permission if you want to use
the Mozilla Marks."

Comment 17

[Mike Shaver (:shaver emeritus)]

Yes, it's fine to use Firefox in a UA string, just like it's fine for all the browsers in the world to use Mozilla in a UA string.  It's not a representation to the user, it's a protocol element.

Comment 18

[[e-mail]]

(In reply to comment #16)
> Unbranded builds will now advertise themselves as Firefox in the UA string. Can
> someone clarify that this is OK from a trademark perspective?

Note that we did use that unbranded string for the 4.0 alphas, so we need to do something here for the 5.0 alphas. Whether that's updating that string when we branch, or just using official branding like we do in betas, I don't especially care.

But shaver's fast, he already spoke!

Comment 19

[[e-mail]]

Attachment: patch v3

Bug 581008 addresses the MOZ_UANAME bits, so the patch here just needs to set it appropriately.

Branch patch is unaffected, since we're not doing the pref juggling over there.

Comment 20

[Dão Gottwald [:dao]]

Comment on attachment 468467 [details] [diff] [review]
patch v3

>diff --git a/browser/branding/nightly/configure.sh b/browser/branding/nightly/configure.sh
>--- a/browser/branding/nightly/configure.sh
>+++ b/browser/branding/nightly/configure.sh
>@@ -1,2 +1,2 @@
> MOZ_APP_DISPLAYNAME=Minefield
>-MOZ_APP_UA_NAME=Minefield
>+MOZ_APP_UA_NAME=Firefox

Note that the app name in the Firefox UA string should automatically default to "Firefox" if you don't set MOZ_APP_UA_NAME at all.

Comment 21

[[e-mail]]

(In reply to comment #20)
> Note that the app name in the Firefox UA string should automatically default to
> "Firefox" if you don't set MOZ_APP_UA_NAME at all.

Howcome? I figured nsIXULAppInfo.name would be Minefield.

Comment 22

[Dão Gottwald [:dao]]

(In reply to comment #21)
> (In reply to comment #20)
> > Note that the app name in the Firefox UA string should automatically default to
> > "Firefox" if you don't set MOZ_APP_UA_NAME at all.
> 
> Howcome? I figured nsIXULAppInfo.name would be Minefield.

Nope, it's Firefox.

Comment 23

[[e-mail]]

Attachment: patch v4

Indeed it is. This patch just keeps getting smaller.

Comment 24

[[e-mail]]

Comment on attachment 468331 [details] [diff] [review]
patch for 1.9.2

Requesting approval. This is low-risk (only affects nightlies) and brings us in line with release builds and mozilla-central.

Comment 25

[[e-mail]]

http://hg.mozilla.org/mozilla-central/rev/2914a4cdd7e2

Comment 26

[Robert Kaiser]

As this bug killed the "use Minefield/ in the UA to discover bad sniffing for 'Firefox'" intention that we had before (there was an actual bug for this that we *solved* by not having "Firefox" in the UA, I must assume Tech Evang for that kind of bad sniffing is dead and it's becoming ludicrous for any Gecko-based browser to not set compatMode.firefox - and actually, SeaMonkey is probably already pretty laughable for not having "Firefox" in its UA right now, if not for existing in the first place. That's how life goes, apparently.

In any case, I start to wonder why we don't just hardwire a |if (appname != Firefox) append "Firefox/x.y" to UA| thing and forget about .compatMode completely, as we obviously have given in to "Firefox" sniffers and acknowledge that "Gecko" should equal "Firefox" on the web.

Shouldn't we just go the full way there? Even having .compatMode.firefox available as a pref is probably pref/code bloat when good parts of the web actually basically require anyone using Gecko and not being Firefox (or a nightly/alpha/beta of it) to set that pref. Better to just hardwire setting that Token in the UA for everyone and forget about the pref, IMHO.

Comment 27

[u88484]

Verified fixed

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b5pre) Gecko/20100827 Firefox/4.0b5pre

Comment 28

[LegNeato]

For the branches, is it supposed to say Minefield here? https://bugzilla.mozilla.org/attachment.cgi?id=468331&action=diff#a/browser/branding/nightly/configure.sh_sec1

I'm mainly worried about branch nightlies getting called Minefield after this change, which would cause all sorts of issues. If that can't happen I'll likely approve for the next branch release.

Comment 29

[[e-mail]]

I'm not sure, but the Namoroka part (i.e. unofficial branding) is apparently what we're using -- so the Minefield bit is irrelevant. (We're not changing that behavior here, just forcing the UA string to change.)

Comment 30

[Daniel Veditz [:dveditz]]

Comment on attachment 468331 [details] [diff] [review]
patch for 1.9.2

Approved for 1.9.2.11, a=dveditz for release-drivers

Comment 31

[[e-mail]]

Christian, you OK with me landing this for .11? (Sounded like you wanted to wait based on comment 28, but if an approval's an approval I'll take it!)

Comment 32

[LegNeato]

Yep, the flag and comment 30 should be all you need, thanks!

Comment 33

[[e-mail]]

Done.

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/bf0d8cb60959

Comment 34

[Mark Banner (:standard8)]

I think this caused test failures on 1.9.2 when it landed (shown below for reference).

When I looked at the debug it was obvious that the test was touching the network, so I ported the make-it-not-touch-the-network change from bug 534889 comment 1.

My current theory is that the change in the UA string cause a change in something at the other (google?) end which caused the test to fail.


http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.6/1285663645.1285664407.19761.gz

TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/search/test/browser_426329.js | Return key loaded results in current tab - Got [object XPCNativeWrapper [object HTMLDocument]], expected [object XPCNativeWrapper [object HTMLDocument]]
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/search/test/browser_426329.js | Alt+Return key loaded results in new tab - Didn't expect [object XPCNativeWrapper [object HTMLDocument]], but got it
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/search/test/browser_426329.js | Alt+Return key loaded results in foreground tab - Got [object XPCNativeWrapper [object HTMLDocument]], expected [object XPCNativeWrapper [object HTMLDocument]]
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/search/test/browser_426329.js | LeftClick loaded results in current tab - Got [object XPCNativeWrapper [object HTMLDocument]], expected [object XPCNativeWrapper [object HTMLDocument]]
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/search/test/browser_426329.js | MiddleClick loaded results in new tab - Didn't expect [object XPCNativeWrapper [object HTMLDocument]], but got it
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/search/test/browser_426329.js | MiddleClick loaded results in foreground tab - Got [object XPCNativeWrapper [object HTMLDocument]], expected [object XPCNativeWrapper [object HTMLDocument]]
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/search/test/browser_426329.js | Shift+MiddleClick loaded results in new tab - Didn't expect [object XPCNativeWrapper [object HTMLDocument]], but got it
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/search/test/browser_426329.js | Shift+MiddleClick loaded results in background tab - Didn't expect [object XPCNativeWrapper [object HTMLDocument]], but got it

Comment 35

[[e-mail]]

Awesome, thanks!

Comment 36

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #28)
> For the branches, is it supposed to say Minefield here?
> https://bugzilla.mozilla.org/attachment.cgi?id=468331&action=diff#a/browser/branding/nightly/configure.sh_sec1

For future reference: we manually change the default branding on branches from "nightly" to "unofficial" after the branch point, so changes to "nightly" branding don't affect branch builds by default:

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/04eb06993202
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/41615c2bda16
https://bugzilla.mozilla.org/show_bug.cgi?id=455395

Comment 37

[:Gavin Sharp [email: [e-mail]]]

This was backed out on 1.9.2 (bug 600984), and I don't think anyone plans on attempting to re-land, so status1.9.2->wontfix, I guess.

Comment 38

[[e-mail]]

I was planning on fixing it up and relanding, but I've got better things to do, and it's not blocking by any means -- so I'm fine with just leaving it as is.

Comment 39

[Daniel Veditz [:dveditz]]

Comment on attachment 468331 [details] [diff] [review]
patch for 1.9.2

In any case this isn't the patch to use and 1.9.2.11 builds are done: removing approval.