Closed Bug 591573 Opened 14 years ago Closed 14 years ago

Kill general.useragent.vendor & vendorSub / remove distro identification from UA string and add it to about:support

Categories

(Core :: Networking: HTTP, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla2.0b7

People

(Reporter: dao, Assigned: dao)

References

(Blocks 2 open bugs)

Details

(Keywords: dev-doc-complete)

Attachments

(2 files, 3 obsolete files)

general.useragent.vendor is the only remaining way to easily append something to the UA string. We should do something about this before add-ons discover it.

Reading it only on Linux would be an easy stop-gap solution. Ultimately though, I don't think distributions need to announce themselves in the UA string at all.
Blocks: 586165
The distribution information in the UA string makes it a bit easier in support cases as it contains the exact package version number. (Obviously also available in the distributions package manager). The support argument was valid in the other bug so why it isn't here?
In short:
So how long should it take for Mozilla support folks to find out who was the vendor of the build?
That's what about:support is for, not the UA string.
And what exactly tells you something about it there?
Attached patch kill it (obsolete) — Splinter Review
Attached patch read it only on linux (obsolete) — Splinter Review
(In reply to comment #4)
> And what exactly tells you something about it there?

Nothing -- yet. Want to file a bug on getting that in there? There's bug 589444 for the build ID.

Picking up from the discussion in bug 581008, if we want to kill the pref off entirely, we could do two things here:
1) Remove the pref and have a build-time VENDOR_UANAME etc.
2) Have neither.

Arguments against having appName set in stone were made in that bug, but I don't think they apply so much to the vendor part. If people make XULRunner apps, they can identify themselves suitably using the appName bits. So I think the typical distro usage here would be to have the distro name in the vendor. If they really want runtime configurability, then we just add a 'vendor' property to nsIXULAppInfo, so they can stash it in application.ini. Separate bug.

For just build-time configurability, distros can always just carry a local patch to add whatever they want.

What do distro folks think?
(Uh, XULAppInfo already has 'vendor', so if it means the right thing then we're set.)
(In reply to comment #8)
> (Uh, XULAppInfo already has 'vendor', so if it means the right thing then we're
> set.)

It's "Mozilla" in a stock Firefox on Ubuntu.
Trivial patch, just adds support for a app.support.vendor pref, no new API or build time magic.
the right patch
Attachment #470167 - Attachment is obsolete: true
(In reply to comment #9)
> (In reply to comment #8)
> > (Uh, XULAppInfo already has 'vendor', so if it means the right thing then we're
> > set.)
> 
> It's "Mozilla" in a stock Firefox on Ubuntu.

More importantly, it decides the directory under which the user profiles will go. The default is ${HOME}/.${vendor}/${name}, where vendor and name are taken from application.ini/XULAppInfo. Note there is a way to override this with Profile in application.ini, and a proposal to have it added to application.ini as part of bug 525882.

I have always been reluctant to change the vendor because I have never actually searched what else would break if vendor != Mozilla.
OK, so, let's say I want to build an alternative browser based on Mozilla. Let's call it "Beonex EasySurfer". Due to compatibility and some sites looking for "Firefox", I have to add that. Where would I add the true app name, "BeonexEasySurfer/1.0"? In vendor, right?
No, you just set the appName to BeonexEasySurfer, the appVersion to 1.0, and toggle general.useragent.compatMode.firefox.
)In reply to comment #7)
> So I think
> the typical distro usage here would be to have the distro name in the vendor.
> If they really want runtime configurability, then we just add a 'vendor'
> property to nsIXULAppInfo, so they can stash it in application.ini. Separate
> bug.

There is a vendor in application.ini as others pointed out and for reasons Mike pointed out that is no option.

> For just build-time configurability, distros can always just carry a local
> patch to add whatever they want.
> 
> What do distro folks think?

For the current vendor and vendor-version thing I don't need a pref. I can do it at compile time.
If it appears in the UA is a decoupled question and I could certainly live without. But then it should be prominent enough in about:support. Also compiled in instead of a pref is preferred.
People doing statistics based on UA are screwed though but I guess that's the wanted outcome anyway.
(In reply to comment #15)
> People doing statistics based on UA are screwed though but I guess that's the
> wanted outcome anyway.

Not directly wanted but accepted, I think.

The statistics are probably significantly skewed anyway since browsers don't have the distro in the UA string across the board.
Comment on attachment 470163 [details] [diff] [review]
kill it

It sounds like Wolfgang and Mike are happy enough with killing it and having something in about:support. Is this accurate?

>diff -r 9b2e618462a2 dom/base/nsGlobalWindow.cpp

> nsNavigator::GetVendor(nsAString& aVendor)
> {

>+  return NS_OK;

'aVendor.Truncate()' first, and put a one-liner here referencing this bug and saying why it was removed?

Having it return blank for Linux distros should serve as deprecation warning for people gathering stats, so we can remove it in Firefox 5.

> nsNavigator::GetVendorSub(nsAString& aVendorSub)

Same here.

r=dwitte, needs sr. ;)
Attachment #470163 - Flags: review+
(In reply to comment #17)
> > nsNavigator::GetVendor(nsAString& aVendor)
> > {
> 
> >+  return NS_OK;
> 
> 'aVendor.Truncate()' first,

Why? FWIW, I just followed GetSecurityPolicy.

> and put a one-liner here referencing this bug and
> saying why it was removed?
> 
> Having it return blank for Linux distros should serve as deprecation warning
> for people gathering stats, so we can remove it in Firefox 5.

I don't think we can remove it, as it would break scripts trying to read it (other browser still support it). I don't think we need to warn either, as it wasn't actually intended to expose the Linux distro in the first place. This happened kind of accidentally with bug 274928.
(In reply to comment #18)
> Why? FWIW, I just followed GetSecurityPolicy.

If a getter succeeds, it has to set the outparam. Someone could pass in a nonzero string. (In this case, that's unlikely, since the callers will be JS. But still, any code that doesn't truncate is wrong.)

> I don't think we can remove it, as it would break scripts trying to read it
> (other browser still support it).

Well, it'd just return 'undefined' if we removed it, which probably won't break anything other than people trying to collect metrics. But that's a wild guess.

> I don't think we need to warn either

Sure -- I just meant that returning an empty string will serve as a warning to people who use it for stats right now. They'll notice it, for sure.
(In reply to comment #19)
> Well, it'd just return 'undefined' if we removed it, which probably won't break
> anything other than people trying to collect metrics. But that's a wild guess.

No, people detect browsers this way (e.g. navigator.vendor.indexOf("Apple")) in order to cater to different capabilities. That's of course the wrong way to do it, but they do it anyway.
Sure, but it's the empty string in Firefox. Are you suggesting they detect Gecko that way? I'd believe it, I just wouldn't imagine it's common. But like I said, wild guess.
Wordpress' statpress and Wikipedia are using the UA string for Linux distribution statistics.
(In reply to comment #21)
> Sure, but it's the empty string in Firefox. Are you suggesting they detect
> Gecko that way? I'd believe it, I just wouldn't imagine it's common. But like I
> said, wild guess.

No, they're detecting other browsers this way, and their test would fail non-gracefully with Gecko if it didn't have the vendor property at all.
(In reply to comment #22)
> Wordpress' statpress and Wikipedia are using the UA string for Linux
> distribution statistics.

They must have seen a drop in Debian usage, then, since I removed it 6 months ago.

(In reply to comment #17)
> It sounds like Wolfgang and Mike are happy enough with killing it and having
> something in about:support. Is this accurate?

For me, very much, yes.
> > it would break scripts trying to read it

> it'd just return 'undefined' if we removed it, which probably won't break
> anything other than

FYI, navigator.vendor.indexOf("Ubuntu") != 0 would throw (or whatever) with "'indexOf()' is not a function of 'undefined'", so it could well break the whole script and site, if it's negligent. So, better to return the empty string.
Alternatively, you could return "obsolete/2.0" as warning/error ;-)
Attachment #470163 - Flags: superreview?(dveditz)
(In reply to comment #24)
> (In reply to comment #17)
> > It sounds like Wolfgang and Mike are happy enough with killing it and having
> > something in about:support. Is this accurate?
> 
> For me, very much, yes.

Also for me but can we please make sure that we get an easily accessible vendor information (compiled in) somewhere _before_ this one is removed?
(In reply to comment #27)
> Also for me but can we please make sure that we get an easily accessible vendor
> information (compiled in) somewhere _before_ this one is removed?

Agreed, but I'd really like this to be per-app.  If a 3rd party builds against the libxul we ship, I'd rather not claim support on the 3rd party app.
Attachment #470168 - Flags: review?(enndeakin)
Comment on attachment 470168 [details] [diff] [review]
allow displaying a vendor in about:support

So it sounds like, for about:support info, the distro people would be happier with Vendor/Profile settings in application.ini, rather than a pref. (bug 525882)

Up to you guys, what would you like to do? Sounds like there's risk in changing Vendor (comment 12), so if you guys would prefer that, you should test it out and see if it'll do what you want. Otherwise we can just do the pref. (I don't think we'd want both!)

If this part ends up depending on bug 525882, we should probably split it out into another bug so this one doesn't get bogged down.
I must have missed something, what's the downside of a pref?
(In reply to comment #29)
> Comment on attachment 470168 [details] [diff] [review]
> allow displaying a vendor in about:support
> 
> So it sounds like, for about:support info, the distro people would be happier
> with Vendor/Profile settings in application.ini, rather than a pref. (bug
> 525882)

I'm not convinced about that. I (as openSUSE) don't want to claim full vendorship on Firefox.
The purpose is to make it easy to recognize who created the build so everyone involved (mozilla.org, distributors) know how to handle stuff.
There are different possibilities doing that.
For Firefox we already have distribution.id preference from distribution.js which is user facing in the about box.
I think what I prefer is a fixed ID in my builds. There is already a
MOZ_DISTRIBUTION_ID but it looks unsafe to reuse that as it's used in Google landing pages.

(In reply to comment #28)
> Agreed, but I'd really like this to be per-app.  If a 3rd party builds against
> the libxul we ship, I'd rather not claim support on the 3rd party app.

That's true indeed. I'm not sure where and how to display the information.
(In reply to comment #30)
> I must have missed something, what's the downside of a pref?

That vendors probably don't want users fiddling with that information?

I don't really have anything more to add here; I'm just trying to clarify what the distros would prefer. As long as that's made clear, the system will work. ;)
(In reply to comment #32)
> (In reply to comment #30)
> > I must have missed something, what's the downside of a pref?
> 
> That vendors probably don't want users fiddling with that information?

There's no good reason for people to fiddle with the pref and then ask for support. So I don't think I care...
> For Firefox we already have distribution.id preference from distribution.js
> which is user facing in the about box.

Am I interpreting it correctly that you already have what you need? That the vendor from UA string can be removed without replacement?

That would assume that anybody may create a distribution.ini/js without negotiation with Mozilla Corporation.
(at least for the purpose of setting distribution ID)
Can you summarize what this bug actually changes?

Also, does this change the user agent sent to a web site?
(In reply to comment #36)
> Can you summarize what this bug actually changes?
> 
> Also, does this change the user agent sent to a web site?

It does, it kills the SUSE/x.x, Fedora/x.x, Ubuntu/x.x & Co. part of the UA. Distros currently set the general.useragent.vendor and general.useragent.vendorSub prefs for this. What I'd like you to review is the part that allows distros to add this info to about:support instead. We could reuse general.useragent.vendor and general.useragent.vendorSub for this, but that seemed slightly weird to me, since general.useragent.* is really supposed to be about the UA string.
(In reply to comment #37)
> > Also, does this change the user agent sent to a web site?
> 
> It does, it kills the SUSE/x.x, Fedora/x.x, Ubuntu/x.x & Co. part of the UA.

So a distributor would no longer have a means from their web site to tell if a user was using their distribution or not? That would have annoyed them long ago, but maybe this isn't as important nowadays?
Neil, we had people from 3 distros here commenting, and none of them seems to have an issue with this change, as it ended up after the discussion.
(In reply to comment #37)
> since general.useragent.* is really supposed to be about the UA string.

Note this is not entirely true, since you act on the UI locale with general.useragent.locale.
(In reply to comment #38)
(In reply to comment #39)
Yep, the relevant people are on the CC list. Also, from what I've seen Linux browsers don't send this token consistently and Debian dropped it of its own accord from Firefox. So it doesn't seem to be important at this point, at least not important enough to send it with each HTTP request to any site.

(In reply to comment #40)
> > since general.useragent.* is really supposed to be about the UA string.
> 
> Note this is not entirely true, since you act on the UI locale with
> general.useragent.locale.

Well, nevertheless I would assume that general.useragent.locale was added for the UA string originally. We've stopped using it for that just recently.
Changing bug title to match the apparent consensus on the path forward here.
Summary: Make general.useragent.vendor Linux-only or kill it altogether → Kill general.useragent.vendor & vendorSub / remove distro identification from UA string and add it to about:support
(In reply to comment #41)
> (In reply to comment #38)
> (In reply to comment #39)
> Yep, the relevant people are on the CC list. Also, from what I've seen Linux
> browsers don't send this token consistently and Debian dropped it of its own
> accord from Firefox.

I wasn't actually referring to Linux. I was referring to the days when PC manufacturers and ISPs would distribute browsers with an extra token to identify such browsers. I was asking whether that is still common to do that or whether that wasn't important anymore.
AFAIK it's neither common for ISPs to add this to the UA string nor supported by Mozilla.
Attachment #470164 - Attachment is obsolete: true
And historically they couldn't have used the vendor and vendorSub prefs for that, as they were used for the application name.
OK.

Also I see on https://developer.mozilla.org/en/User_Agent_Strings_Reference that general.useragent.extra.* is supposed to be used for that purpose anyway, but maybe that isn't supported either?
(In reply to comment #46)
> that general.useragent.extra.* is supposed to be used for that purpose anyway,
> but maybe that isn't supported either?

Support for general.useragent.extra.* was removed in bug 581008.
Some other quick footnotes from the other related bugs:
* general.useragent.vendorComment was already removed in bug 581008
* This bug should also give Linux users a fingerprintability win (bug 572650)
  for both distro and mozilla.org builds as they'd now share the same UA.
Attachment #470168 - Flags: review?(enndeakin) → review+
Comment on attachment 470163 [details] [diff] [review]
kill it

(In reply to comment #18)
> (In reply to comment #17)
> > > nsNavigator::GetVendor(nsAString& aVendor)
> > > {
> > 
> > >+  return NS_OK;
> > 
> > 'aVendor.Truncate()' first,
> 
> Why? FWIW, I just followed GetSecurityPolicy.

I'm with dwitte, please Truncate(). We'll be in a more sane state should someone start calling this from C++ or develops a new language binding.

The GetSecurityPolicy implementation is 10-year-old code, not a reliable model. (IE, Safari and Chrome return 'undefined' for navigator.securityPolicy: http://code.google.com/p/doctype/wiki/NavigatorSecurityPolicyProperty   We have returned a useless empty string for a decade--can we kill it now?)

sr-, but sr+ if you truncate in GetVendor() and GetVendorSub()
Attachment #470163 - Flags: superreview?(dveditz) → superreview-
Attached patch patch v2Splinter Review
Truncate() added
Assignee: nobody → dao
Attachment #470163 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #475073 - Flags: superreview?(dveditz)
Comment on attachment 475073 [details] [diff] [review]
patch v2

Marking sr=dveditz based on comment 50. :)
Attachment #475073 - Flags: superreview?(dveditz) → superreview+
Attachment #475073 - Flags: approval2.0?
Attachment #470168 - Flags: approval2.0?
Attachment #470168 - Flags: approval2.0? → approval2.0+
Attachment #475073 - Flags: approval2.0? → approval2.0+
http://hg.mozilla.org/mozilla-central/rev/11ab351f5bdc
http://hg.mozilla.org/mozilla-central/rev/dd6a7bbba7b8
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
Adding dev-doc-needed for https://developer.mozilla.org/en/User_Agent_Strings_Reference (removal of VendorProductToken and VendorComment from bug 581008). That document also needs a prominent link to the new 
https://developer.mozilla.org/en/Gecko_user_agent_string_reference, which is already up-to-date AFAICT.
Keywords: dev-doc-needed
I updated the "User Agent Strings Reference" to say that it's obsolete and to refer to the newer article. If someone thinks the older article actually should be updated, please do so.
Target Milestone: mozilla2.0b8 → mozilla2.0b7
Depends on: 647090
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: