Closed Bug 588874 Opened 14 years ago Closed 14 years ago

Replace Minefield with Firefox in UA string

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 4.0b5
Tracking Status
status1.9.2 --- wontfix

People

(Reporter: dwitte, Assigned: dwitte)

References

Details

Attachments

(2 files, 3 obsolete files)

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?
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!
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?
OS: Linux → All
Hardware: x86_64 → All
Version: unspecified → Trunk
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.
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.
(Bug 572661 is about dropping build date, btw; if you want to discuss that, let's do it over there.)
(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.
see bug 334967 for sites that break without Firefox in the UA
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.
OS: All → Linux
Hardware: All → x86_64
Version: Trunk → unspecified
Assignee: nobody → dwitte
Status: NEW → ASSIGNED
Attached patch patch (obsolete) (deleted) — Splinter Review
As advertised.
Attachment #467603 - Flags: superreview?(jst)
Attachment #467603 - Flags: review?(ted.mielczarek)
Decided that we're doing this.
Attachment #467603 - Flags: superreview?(jst) → superreview+
(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.
(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.
For reference, bug 334756 was the bug that initially changed the UA string of trunk builds to say Minefield.
Blocks: 589608
Attached patch patch v2 (obsolete) (deleted) — Splinter Review
Changes 'MozillaDeveloperPreview' as well.
Attachment #467603 - Attachment is obsolete: true
Attachment #468329 - Flags: superreview+
Attachment #468329 - Flags: review?(ted.mielczarek)
Attachment #467603 - Flags: review?(ted.mielczarek)
Attached patch patch for 1.9.2 (deleted) — Splinter Review
Same problem with Namoroka (bug 589608).
Attachment #468331 - Flags: superreview?(jst)
Attachment #468331 - Flags: review?(ted.mielczarek)
(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."
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.
(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!
Attached patch patch v3 (obsolete) (deleted) — Splinter Review
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.
Attachment #468329 - Attachment is obsolete: true
Attachment #468467 - Flags: superreview+
Attachment #468467 - Flags: review?(ted.mielczarek)
Attachment #468329 - Flags: review?(ted.mielczarek)
Depends on: 581008
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.
(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.
(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.
Attached patch patch v4 (deleted) — Splinter Review
Indeed it is. This patch just keeps getting smaller.
Attachment #468467 - Attachment is obsolete: true
Attachment #468492 - Flags: superreview+
Attachment #468492 - Flags: review?(ted.mielczarek)
Attachment #468467 - Flags: review?(ted.mielczarek)
Attachment #468492 - Flags: review?(ted.mielczarek) → review+
Attachment #468331 - Flags: review?(ted.mielczarek) → review+
Attachment #468331 - Flags: superreview?(jst) → superreview+
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.
Attachment #468331 - Flags: approval1.9.2.10?
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Blocks: 591536
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.
Blocks: 591617
Verified fixed Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b5pre) Gecko/20100827 Firefox/4.0b5pre
OS: Linux → All
Hardware: x86_64 → All
Target Milestone: --- → Firefox 4.0b5
Version: unspecified → Trunk
Status: RESOLVED → VERIFIED
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.
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 on attachment 468331 [details] [diff] [review] patch for 1.9.2 Approved for 1.9.2.11, a=dveditz for release-drivers
Attachment #468331 - Flags: approval1.9.2.11? → approval1.9.2.11+
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!)
Yep, the flag and comment 30 should be all you need, thanks!
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
Awesome, thanks!
Depends on: 600992
Depends on: 600984
No longer depends on: 600992
(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
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.
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 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.
Attachment #468331 - Flags: approval1.9.2.11+ → approval1.9.2.11-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: