Closed
Bug 588874
Opened 14 years ago
Closed 14 years ago
Replace Minefield with Firefox in UA string
Categories
(Firefox :: General, defect)
Firefox
General
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)
(deleted),
patch
|
khuey
:
review+
jst
:
superreview+
dveditz
:
approval1.9.2.11-
|
Details | Diff | Splinter Review |
(deleted),
patch
|
khuey
:
review+
dwitte
:
superreview+
|
Details | Diff | Splinter Review |
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!
Comment 2•14 years ago
|
||
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
Assignee | ||
Comment 3•14 years ago
|
||
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.
Assignee | ||
Comment 5•14 years ago
|
||
(Bug 572661 is about dropping build date, btw; if you want to discuss that, let's do it over there.)
Comment 6•14 years ago
|
||
(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•14 years ago
|
||
see bug 334967 for sites that break without Firefox in the UA
Assignee | ||
Comment 8•14 years ago
|
||
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 | ||
Updated•14 years ago
|
Assignee: nobody → dwitte
Status: NEW → ASSIGNED
Assignee | ||
Comment 9•14 years ago
|
||
As advertised.
Attachment #467603 -
Flags: superreview?(jst)
Attachment #467603 -
Flags: review?(ted.mielczarek)
Assignee | ||
Comment 10•14 years ago
|
||
Decided that we're doing this.
Updated•14 years ago
|
Attachment #467603 -
Flags: superreview?(jst) → superreview+
Comment 11•14 years ago
|
||
(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•14 years ago
|
||
(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•14 years ago
|
||
For reference, bug 334756 was the bug that initially changed the UA string of trunk builds to say Minefield.
Assignee | ||
Comment 14•14 years ago
|
||
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)
Assignee | ||
Comment 15•14 years ago
|
||
Same problem with Namoroka (bug 589608).
Attachment #468331 -
Flags: superreview?(jst)
Attachment #468331 -
Flags: review?(ted.mielczarek)
Comment 16•14 years ago
|
||
(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.
Assignee | ||
Comment 18•14 years ago
|
||
(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!
Assignee | ||
Comment 19•14 years ago
|
||
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)
Comment 20•14 years ago
|
||
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.
Assignee | ||
Comment 21•14 years ago
|
||
(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•14 years ago
|
||
(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.
Assignee | ||
Comment 23•14 years ago
|
||
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+
Updated•14 years ago
|
Attachment #468331 -
Flags: superreview?(jst) → superreview+
Assignee | ||
Comment 24•14 years ago
|
||
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?
Assignee | ||
Comment 25•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment 26•14 years ago
|
||
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•14 years ago
|
||
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
Comment 28•14 years ago
|
||
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.
Assignee | ||
Comment 29•14 years ago
|
||
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•14 years ago
|
||
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+
Assignee | ||
Comment 31•14 years ago
|
||
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•14 years ago
|
||
Yep, the flag and comment 30 should be all you need, thanks!
Assignee | ||
Comment 33•14 years ago
|
||
Comment 34•14 years ago
|
||
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
Assignee | ||
Comment 35•14 years ago
|
||
Awesome, thanks!
status1.9.2:
--- → .12-fixed
Updated•14 years ago
|
Comment 36•14 years ago
|
||
(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•14 years ago
|
||
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.
Assignee | ||
Comment 38•14 years ago
|
||
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•14 years ago
|
||
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.
Description
•