Last Comment Bug 591573 - Kill general.useragent.vendor & vendorSub / remove distro identification from UA string and add it to about:support
: Kill general.useragent.vendor & vendorSub / remove distro identification from...
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: Networking: HTTP (show other bugs)
: Trunk
: All All
: -- normal with 1 vote (vote)
: mozilla2.0b7
Assigned To: Dão Gottwald [:dao]
:
Mentors:
Depends on: 647090
Blocks: http-fingerprint about:support++ 581008 586165
  Show dependency treegraph
 
Reported: 2010-08-28 00:24 PDT by Dão Gottwald [:dao]
Modified: 2011-04-01 09:20 PDT (History)
17 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
kill it (4.59 KB, patch)
2010-08-28 01:00 PDT, Dão Gottwald [:dao]
dwitte: review+
dveditz: superreview-
Details | Diff | Splinter Review
read it only on linux (1.24 KB, patch)
2010-08-28 01:00 PDT, Dão Gottwald [:dao]
no flags Details | Diff | Splinter Review
allow displaying a vendor in about:support (17.76 KB, patch)
2010-08-28 01:40 PDT, Dão Gottwald [:dao]
no flags Details | Diff | Splinter Review
allow displaying a vendor in about:support (752 bytes, patch)
2010-08-28 01:41 PDT, Dão Gottwald [:dao]
enndeakin: review+
jst: approval2.0+
Details | Diff | Splinter Review
patch v2 (7.39 KB, patch)
2010-09-14 07:00 PDT, Dão Gottwald [:dao]
dwitte: superreview+
jst: approval2.0+
Details | Diff | Splinter Review

Description Dão Gottwald [:dao] 2010-08-28 00:24:16 PDT
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.
Comment 1 Wolfgang Rosenauer [:wolfiR] 2010-08-28 00:52:18 PDT
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?
Comment 2 Wolfgang Rosenauer [:wolfiR] 2010-08-28 00:53:41 PDT
In short:
So how long should it take for Mozilla support folks to find out who was the vendor of the build?
Comment 3 dwitte@gmail.com 2010-08-28 00:55:11 PDT
That's what about:support is for, not the UA string.
Comment 4 Wolfgang Rosenauer [:wolfiR] 2010-08-28 00:59:46 PDT
And what exactly tells you something about it there?
Comment 5 Dão Gottwald [:dao] 2010-08-28 01:00:28 PDT
Created attachment 470163 [details] [diff] [review]
kill it
Comment 6 Dão Gottwald [:dao] 2010-08-28 01:00:49 PDT
Created attachment 470164 [details] [diff] [review]
read it only on linux
Comment 7 dwitte@gmail.com 2010-08-28 01:14:48 PDT
(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?
Comment 8 dwitte@gmail.com 2010-08-28 01:19:48 PDT
(Uh, XULAppInfo already has 'vendor', so if it means the right thing then we're set.)
Comment 9 Dão Gottwald [:dao] 2010-08-28 01:32:48 PDT
(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.
Comment 10 Dão Gottwald [:dao] 2010-08-28 01:40:33 PDT
Created attachment 470167 [details] [diff] [review]
allow displaying a vendor in about:support

Trivial patch, just adds support for a app.support.vendor pref, no new API or build time magic.
Comment 11 Dão Gottwald [:dao] 2010-08-28 01:41:14 PDT
Created attachment 470168 [details] [diff] [review]
allow displaying a vendor in about:support

the right patch
Comment 12 Mike Hommey [:glandium] 2010-08-28 01:51:36 PDT
(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.
Comment 13 Ben Bucksch (:BenB) 2010-08-28 02:19:21 PDT
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?
Comment 14 Mike Hommey [:glandium] 2010-08-28 02:26:50 PDT
No, you just set the appName to BeonexEasySurfer, the appVersion to 1.0, and toggle general.useragent.compatMode.firefox.
Comment 15 Wolfgang Rosenauer [:wolfiR] 2010-08-28 03:57:04 PDT
)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.
Comment 16 Dão Gottwald [:dao] 2010-08-28 04:09:43 PDT
(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 17 dwitte@gmail.com 2010-08-29 11:40:16 PDT
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. ;)
Comment 18 Dão Gottwald [:dao] 2010-08-29 14:06:01 PDT
(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.
Comment 19 dwitte@gmail.com 2010-08-29 22:12:25 PDT
(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.
Comment 20 Dão Gottwald [:dao] 2010-08-30 00:20:57 PDT
(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.
Comment 21 dwitte@gmail.com 2010-08-30 00:30:04 PDT
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.
Comment 22 Wolfgang Rosenauer [:wolfiR] 2010-08-30 00:37:13 PDT
Wordpress' statpress and Wikipedia are using the UA string for Linux distribution statistics.
Comment 23 Dão Gottwald [:dao] 2010-08-30 00:38:55 PDT
(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.
Comment 24 Mike Hommey [:glandium] 2010-08-30 00:51:24 PDT
(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.
Comment 25 Ben Bucksch (:BenB) 2010-08-30 03:33:56 PDT
> > 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.
Comment 26 Ben Bucksch (:BenB) 2010-08-30 03:35:02 PDT
Alternatively, you could return "obsolete/2.0" as warning/error ;-)
Comment 27 Wolfgang Rosenauer [:wolfiR] 2010-08-30 04:55:43 PDT
(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?
Comment 28 Christopher Aillon (sabbatical, not receiving bugmail) 2010-08-30 10:20:55 PDT
(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.
Comment 29 dwitte@gmail.com 2010-08-30 10:47:13 PDT
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.
Comment 30 Dão Gottwald [:dao] 2010-08-30 11:06:14 PDT
I must have missed something, what's the downside of a pref?
Comment 31 Wolfgang Rosenauer [:wolfiR] 2010-08-30 11:07:55 PDT
(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.
Comment 32 dwitte@gmail.com 2010-08-30 11:17:33 PDT
(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. ;)
Comment 33 Dão Gottwald [:dao] 2010-08-30 11:23:13 PDT
(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...
Comment 34 Ben Bucksch (:BenB) 2010-08-30 11:42:38 PDT
> 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.
Comment 35 Ben Bucksch (:BenB) 2010-08-30 11:43:11 PDT
(at least for the purpose of setting distribution ID)
Comment 36 Neil Deakin 2010-09-02 06:14:55 PDT
Can you summarize what this bug actually changes?

Also, does this change the user agent sent to a web site?
Comment 37 Dão Gottwald [:dao] 2010-09-02 06:23:40 PDT
(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.
Comment 38 Neil Deakin 2010-09-02 06:33:26 PDT
(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?
Comment 39 Ben Bucksch (:BenB) 2010-09-02 06:39:32 PDT
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.
Comment 40 Mike Hommey [:glandium] 2010-09-02 06:47:30 PDT
(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.
Comment 41 Dão Gottwald [:dao] 2010-09-02 06:51:36 PDT
(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.
Comment 42 Dave Garrett 2010-09-02 08:44:25 PDT
Changing bug title to match the apparent consensus on the path forward here.
Comment 43 Neil Deakin 2010-09-02 08:57:19 PDT
(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.
Comment 44 Dão Gottwald [:dao] 2010-09-02 08:59:47 PDT
AFAIK it's neither common for ISPs to add this to the UA string nor supported by Mozilla.
Comment 45 Dão Gottwald [:dao] 2010-09-02 09:03:23 PDT
And historically they couldn't have used the vendor and vendorSub prefs for that, as they were used for the application name.
Comment 46 Neil Deakin 2010-09-02 09:09:25 PDT
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?
Comment 47 Dave Garrett 2010-09-02 09:13:21 PDT
(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.
Comment 48 Dave Garrett 2010-09-02 09:14:37 PDT
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.
Comment 50 Daniel Veditz [:dveditz] 2010-09-13 16:59:11 PDT
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()
Comment 51 Dão Gottwald [:dao] 2010-09-14 07:00:44 PDT
Created attachment 475073 [details] [diff] [review]
patch v2

Truncate() added
Comment 52 dwitte@gmail.com 2010-09-16 10:25:19 PDT
Comment on attachment 475073 [details] [diff] [review]
patch v2

Marking sr=dveditz based on comment 50. :)
Comment 54 Steffen Wilberg 2010-10-15 03:20:40 PDT
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.
Comment 55 Eric Shepherd [:sheppy] 2010-10-22 12:02:23 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.