Last Comment Bug 166395 - Need ability to override navigator.appName and navigator.appVersion from prefs.js
: Need ability to override navigator.appName and navigator.appVersion from pref...
Status: RESOLVED FIXED
parity-opera
: fixed-aviary1.0, fixed1.7.5, helpwanted
Product: Core
Classification: Components
Component: Networking: HTTP (show other bugs)
: Trunk
: All All
: -- enhancement with 19 votes (vote)
: Future
Assigned To: Darin Fisher
: Paul Wyskoczka
Mentors:
: 245789 (view as bug list)
Depends on: 115773
Blocks: 205631
  Show dependency treegraph
 
Reported: 2002-09-03 10:51 PDT by Ian Neal
Modified: 2006-03-12 17:01 PST (History)
17 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch to allow setting navigator.appName and appVersion via about:config (6.22 KB, patch)
2003-11-20 15:10 PST, Greg Singer
no flags Details | Diff | Splinter Review
Here's a hopefully less lousy version that seems to work without barfing on more files than it needs to. Patch with -p1. (3.07 KB, patch)
2003-11-21 19:25 PST, Greg Singer
no flags Details | Diff | Splinter Review
By popular demand, navigator.appname now set by general.appname.override (2.07 KB, patch)
2003-11-22 13:33 PST, Greg Singer
no flags Details | Diff | Splinter Review
It seems my persistence is rivalled by my incompetence. (1.37 KB, patch)
2003-11-22 16:16 PST, Greg Singer
caillon: review+
Details | Diff | Splinter Review
Updated patch (2.10 KB, patch)
2004-05-12 16:36 PDT, Greg Singer
no flags Details | Diff | Splinter Review
Updated updated patch (1.76 KB, patch)
2004-05-13 10:44 PDT, Greg Singer
no flags Details | Diff | Splinter Review
Misc. fixes; also covers platform and appCodeName (3.59 KB, patch)
2004-05-19 09:51 PDT, Greg Singer
no flags Details | Diff | Splinter Review
Superfluous appCodeName override pulled. (3.39 KB, patch)
2004-05-19 16:29 PDT, Greg Singer
no flags Details | Diff | Splinter Review
Update to apply cleanly against recent SeaMonkey CVS (3.38 KB, patch)
2004-07-14 10:19 PDT, Greg Singer
jst: review+
jst: superreview+
Details | Diff | Splinter Review
Aviary port (3.64 KB, patch)
2004-07-22 11:32 PDT, Greg Singer
jst: review+
jst: superreview+
asa: approval1.7.5+
Details | Diff | Splinter Review

Description Ian Neal 2002-09-03 10:51:43 PDT
Certain sites sniff for which browser is which by looking at their appName and
appVersion and deny access. For Evangelism purposes it would be useful to
overide these variables so that mozilla can get into these sites to test what
else works or doesn't work.

I have looked around on bugzilla and mozilla to see if these prefs already exist
but couldn't find anything. I've tried the obvious ones of
general.useragent.appName, general.useragent.appName.override, general.appName
and general.appName.override but none of them work.
Comment 1 Olav Vitters 2002-09-03 11:04:42 PDT
appVersion is bug 115773
Comment 2 Brian Nesse (gone) 2002-09-03 11:14:25 PDT
Not preferences... over to samir for further triage.
Comment 3 Ian Neal 2002-09-03 15:05:13 PDT
bug 115773 wants to have the appVersion change when the useragent string changes
which is one possible solution but the appName really needs to change too to be
effective. It would mean you'd only need the useragent.override preference as
long as their was a reasonably simple way of mapping the UA string to appName
and appVersion.
Comment 4 Henry James 2002-09-03 19:13:04 PDT
I think the MozDev project UABar just does this job. Also the MultiZilla addon
has a Quick Pref menu with a submenu "HTTP User Agent settings" to fake all
kinds of user agents.

http://uabar.mozdev.org/
http://multizilla.mozdev.org/
Comment 5 Ian Neal 2002-09-04 03:13:27 PDT
The whole point of this enhancement request is that changing the UA string,
which I frequently do for Evaneglism purposes, does not change navigator.appName
and navigator.appVersion.

I'm looking for either a pref that does change these two variables or for these
two variables to be changed when the UA string is changed.
Comment 6 HJ 2002-09-04 23:42:32 PDT
I really wonder how anybody can ask for a preference setting when the code isn't
even implemented. However, the HTTP UserAgent code should change this when the
HTTP UserAgent override is set. 

I don't think we need a new preference setting for this. The user should simply
get what he asked for when he set/changes the HTTP UserAgent. That is just
common sense to me.
Comment 7 Darin Fisher 2002-09-04 23:56:57 PDT
ian: isn't general.useragent.override sufficient?
Comment 8 Ian Neal 2002-09-05 05:48:56 PDT
I suppose I should have been more explict with my summary and said prefs and
backend code (probably why I logged it against prefs backend to start with
though I expect that was the wrong component). 
Tweeking summary, original summary was "Need prefs for overiding
navigator.appName and navigator.appVersion"

general.useragent.override would be fine as long as there was the code to change
what is reported by navigator.appName and navigator.appVersion to match what you
would expect from a browser reporting that UA string.
There would need to be someway of converting UA strings to appName and
appVersion, which I expect would have to be different for IE, Mozilla, Netscape
(various versions), Opera, etc.
Comment 9 Darin Fisher 2002-09-05 09:15:34 PDT
ian: ok, ic what your getting at.  currently, general.useragent.override only
changes what we send on network connections.  it does not effect what our DOM
reports.  perhaps it should.  cc'ing some folks for comment.
Comment 10 Boris Zbarsky [:bz] 2002-09-05 23:45:08 PDT
timeless, didn't you have some thoughts on this?
Comment 11 timeless 2002-09-06 05:22:57 PDT
yeah, my project to refactor useragent so that each consumer could have its own
forgery would fix this.
Comment 12 Darin Fisher 2002-09-06 09:54:44 PDT
timeless: then you might be interested to know that nsIHttpProtocolHandler is
probably going to be frozen come mozilla 1.2 (see bug 157133).
Comment 13 HARUNAGA Hirotoshi 2002-09-11 05:36:15 PDT
http://www.fujifilm.co.jp/efa/body.html uses scripts as follows.

if (navigator.appName.indexOf("Netscape") >= 0) {
	if (navigator.appVersion.charAt(0) >= 5) {
		var aver = parseFloat(navigator.vendorSub);
		if (navigator.vendorSub.length > 4) {
			aver += parseFloat(navigator.vendorSub.substring(4,5))/100;
		}
	} else {
		var ep = navigator.appVersion.indexOf(" ");
		var aver = parseFloat(navigator.appVersion.substring(0,ep));
	}
	return (nnv != 0 && nnv <= aver);
}

What about navigator.vendorSub?
Comment 14 John Vance 2002-10-12 00:21:12 PDT
Konqueror 3.0 already does this.  Spoofing a different browser changes the user
agent string as well as the navigator object.  It even allows you to spoof
different browsers on a site by site basis.  This would be really useful for
getting around bug 89853.  Capital One checks to see if navigator.appVersion
starts with 4, and if it doesn't you're denied access to online credit card
services.
Comment 15 Darin Fisher 2002-11-17 22:41:16 PST
-> future
Comment 16 Bill Sellers 2003-03-24 05:41:48 PST
Darin:

I am requesting that this bug get escalated.  The company that I work for has
implemented checks for appName and appVersion in cold fusion code.  They are
doing this to "enforce" that users use the corporate approved version of
Netscape or IE.  It probably isn't prudent to say what I really think of this
policy, so I won't. 
;)
Comment 17 noririty 2003-11-02 23:34:23 PST
"navigator.appName" seems defined at here.
http://lxr.mozilla.org/mozilla/source/dom/src/base/nsGlobalWindow.cpp#5881
Comment 18 Greg Singer 2003-11-20 15:10:31 PST
Created attachment 136016 [details] [diff] [review]
Patch to allow setting navigator.appName and appVersion via about:config

Should apply against latest CVS, NOT 0.7.
Comment 19 Christian :Biesinger (don't email me, ping me on IRC) 2003-11-20 15:15:21 PST
firstly, that patch lacks the changes to modules/libpref/src/init/all.js
secondly, why do you bother with putting this on nsHttpHandler? that seems
totally not worth the effort.
Comment 20 Greg Singer 2003-11-20 15:43:14 PST
Ick.  Bluff called.

This was mainly done by copying, pasting, and modifying the code for setting the
user agent, so that's why stuff got shoveled into nsHttpHandler.  I figured
someone must have done something right with that and it would be easier for me
to build off what they'd done rather than trying to figure out the correct way
to do this.  >_<  Since I also seem to be unable to use Bugzilla correctly, I
might as well add that navigator.appName() is set by general.appname, which is
defined in browser/app/profile/all.js (remember, I copied useragent stuff), and
navigator.appVersion() can be overridden by general.appversion.override.

I'm feeling very silly at this point.  Sorry if this is causing anyone any
trouble, since I seem to be spamming twenty or so people every time I post
something to this thread :/.
Comment 21 Christian :Biesinger (don't email me, ping me on IRC) 2003-11-20 16:02:12 PST
>This was mainly done by copying, pasting, and modifying the code for setting the
>user agent

yeah, the useragent is necko business really, and hence belongs in
nsHttpHandler. navigator.appName however is only of interest to the DOM.

(btw, I think you patch addresses only half of this bug - only appName not
appVersion)

>browser/app/profile/all.js

that's the firebird file...
Comment 22 Greg Singer 2003-11-21 19:25:17 PST
Created attachment 136080 [details] [diff] [review]
Here's a hopefully less lousy version that seems to work without barfing on more files than it needs to.  Patch with -p1.

Note:  both of these patches do cover navigator.appVersion.
Comment 23 Ian Neal 2003-11-22 09:07:43 PST
Couple of things:
1) Can you not do appname the same way as appversion then you would not need to
alter all.js at all and keeps the behaviour consistent with how setting UA
string works. 
2) Whitespace changes - as far as I can see they all break formatting rather
than fixing any.
Comment 24 Greg Singer 2003-11-22 13:33:04 PST
Created attachment 136130 [details] [diff] [review]
By popular demand, navigator.appname now set by general.appname.override

all.js is now safe :).	The whitespace was an artifact of something I'd tried
earlier and wasn't intentional, so I fixed it.	Thanks for pointing it out.
Comment 25 Christian :Biesinger (don't email me, ping me on IRC) 2003-11-22 14:36:01 PST
Comment on attachment 136130 [details] [diff] [review]
By popular demand, navigator.appname now set by general.appname.override

+  prefBranch->PrefHasUserValue("general.appname.override", &override);

why?

+    if (NS_FAILED(res))
+      return res;

why should this be fatal?
Comment 26 Greg Singer 2003-11-22 16:16:25 PST
Created attachment 136139 [details] [diff] [review]
It seems my persistence is rivalled by my incompetence.
Comment 27 Ben Giddings 2003-11-25 12:13:39 PST
An instance where this would really be helpful:

http://personal.fidelity.com/products/funds/mfl_frame.shtml?130347883

var browser = "";

if ( (parseInt(navigator.appVersion) >= 4) &&
	((navigator.appName.indexOf("Microsoft") != -1) ||
(navigator.appName.indexOf("Netscape") != -1)) &&
	((navigator.appVersion.indexOf("Win") != -1) ||
(navigator.appVersion.indexOf("Mac") != -1)) ) {
	browser = "okay";
}
else {
	browser = "other";
}

They don't even try the page when it's Mozilla on Linux, only browsers calling
themselves "Netscape" or "IE" on "Win" or "Mac" are tried.  I threw a filter in
Privoxy to overcome this, after calling their tech support dept. and
complaining.  I still would prefer to be able to set this from Mozilla though.


-----

And even worse:
alert('You are using ' + navigator.appName + ' ' + browser.versionMinor)

where "browser.versionMinor" is reported by "Browser Detect Lite  v2.1.3" as 1.5
and navigator.appName is reported as Netscape.

This makes them think I'm using Netscape 1.5 -- no big shock they don't think
the browser will work.
Comment 28 Doug Halamay 2003-12-20 11:43:39 PST
Any possibility of getting this patch reviewed/checked in soon?

Trying to sign in to online banking at www.bankofamerica.com is broken because
it checks navigator.appName, which we can't spoof/change until this is fixed.
Comment 29 Boris Zbarsky [:bz] 2003-12-20 12:02:31 PST
I assume there is an evangelism bug on that site?
Comment 30 Hadrien Nilsson 2004-01-07 09:26:32 PST
Btw, I think appName should now return « Mozilla » instead of « Netscape » by
default. Mozilla do not depend on Netscape anymore. It's strange to use Firebird
and get a « Netscape » string when accessing to navigator.appName.
Comment 31 Matthias Versen [:Matti] 2004-01-07 11:53:33 PST
comment#30 :
That is another bug and it's wontfix for good reasons
Comment 32 Doug Halamay 2004-01-11 01:35:03 PST
RE: Comment#29 :

I filed bug 230612 regarding the problem.
Comment 33 David P James 2004-04-17 08:37:51 PDT
(In reply to comment #31)
> comment#30 :
> That is another bug and it's wontfix for good reasons

Yes, bug 61071.

But what are the good reasons, especially in 2004? I've probably encountered
*more* problems with sites thinking that appName = Netscape == Netscape4 than
with some other name in the string (the site will serve up NS4 stuff that
Mozilla can't handle whilst Opera and Konqueror will happily display the site
since they use own names and not "Netscape", although Opera does default to IE).

Once this bug is fixed, the default appName should become "Mozilla" or "Mozilla
Firefox" (as the case may be) since a UA spoofing extension could change it back
to Netscape for those sites which may cause a problem.
Comment 34 Christopher Aillon (sabbatical, not receiving bugmail) 2004-04-21 10:53:11 PDT
Comment on attachment 136139 [details] [diff] [review]
It seems my persistence is rivalled by my incompetence.

IMO, this should only be able to be changed by embeddors, not users.  As should
the user agent.  But then again, in an ideal world, this would not matter. 
This whole thing stinks to me, but I'll offer up a review anyway.

>--- dom/src/base/nsGlobalWindow.cpp	2003-11-22 19:03:20.883448800 -0500
>+++ dom/src/base/nsGlobalWindow.cpp.new	2003-11-22 19:03:28.033361848 -0500
>@@ -5758,6 +5758,24 @@
> NavigatorImpl::GetAppVersion(nsAString& aAppVersion)
> {
>   nsresult res;

With your changes, you aren't using this...

>+
>+  nsCOMPtr<nsIPrefBranch> prefBranch(gPrefBranch);
>+  if (!prefBranch) {
>+    prefBranch = do_GetService(NS_PREFSERVICE_CONTRACTID);
>+    NS_ENSURE_STATE(prefBranch);
>+  }
>+
>+  nsXPIDLCString override;
>+

...until here, so declare and initialize all at once.

But while you are here, please rename the nsresult to |rv|.

>+  res = prefBranch->GetCharPref("general.appversion.override",
>+                                getter_Copies(override));
>+
>+  if (NS_SUCCEEDED(res))
>+  {

We use K&R bracing style, not BSD style.

>+    CopyASCIItoUCS2(override, aAppVersion);
>+    return res;
>+  }
>+
>   nsCOMPtr<nsIHttpProtocolHandler>
>     service(do_GetService(kHTTPHandlerCID, &res));
>   if (NS_SUCCEEDED(res) && service) {


The same exact comments apply to your GetAppName changes.

r=caillon I suppose, but only if jst superreviews.
Comment 35 Greg Singer 2004-05-12 16:36:54 PDT
Created attachment 148353 [details] [diff] [review]
Updated patch 

Bracing fixed, res changed to rv.  Whatever else that was using gPrefBranch
appears to have gone away in CVS over the past few months, so instead of
bringing it back, I'm just setting prefBranch with do_GetService and rv's
getting declared early anyway.
Comment 36 Peter Van der Beken [:peterv] - away till Aug 1st 2004-05-13 01:28:19 PDT
You want to use GetCharPref or GetStringPref on nsContentUtils.
Comment 37 Greg Singer 2004-05-13 10:44:36 PDT
Created attachment 148434 [details] [diff] [review]
Updated updated patch

Yes, it appears I do.  Simplifies things somewhat.
Comment 38 Owen Marshall 2004-05-17 07:22:31 PDT
(This is possibly beyond the scope of the current bug in name, but not in
spirit, I think, so ...)

To really spoof sites perfectly (such as HSBC's internet banking -
www.ebank.hsbc.com.au - see bug #205631) you need to allow an override to the
navigator.platform string as well.  Adding  

  nsXPIDLCString override =
  nsContentUtils::GetCharPref("general.platform.override");

  if (override) {
    CopyASCIItoUTF16(override, aPlatform);
    return NS_OK;
  }

to NavigatorImpl::GetPlatform(nsAString& aPlatform) solves this, and allows the
HSBC site to work perfectly (and presumably many others that rely on
navigator.platform to check whether the user is running Windows or Mac OS, with
no option for any other OS).  Any chance this could be added in as well?
Comment 39 Johnny Stenback (:jst, jst@mozilla.com) 2004-05-17 16:52:51 PDT
Comment on attachment 148434 [details] [diff] [review]
Updated updated patch

Drive-by review comment...

+  nsXPIDLCString override =
+    nsContentUtils::GetCharPref("general.appversion.override");

override should be a const nsAdoptingString& here, the current code will do an
extra malloc+string copy for no reason. Same thing in other places that uses
methods that return nsAdoptingString (except in odd cases that I don't see in
this patch).

   nsCOMPtr<nsIHttpProtocolHandler>
-    service(do_GetService(kHTTPHandlerCID, &res));
-  if (NS_SUCCEEDED(res) && service) {
+    service(do_GetService(kHTTPHandlerCID, &rv));
+  if (NS_SUCCEEDED(rv) && service) {

redundant error checking, either check NS_SUCCEEDED(rv) or that service is
non-null, checking both is a waste of code. Since you need rv here anyways,
check it, and don't bother checking that service is non-null if do_GetService()
succeeded.

+    rv = service->GetAppVersion(str);
+    CopyASCIItoUTF16(str, aAppVersion);

     aAppVersion.Append(NS_LITERAL_STRING(" ("));
-    res = service->GetPlatform(str);
-    if (NS_FAILED(res))
-      return res;
+    rv = service->GetPlatform(str);

rv at the top of this block is overwritten by the last line here, either check
it and return it at the top of this, or just don't assign into it at all...

     aAppVersion.Append(PRUnichar(')'));

Unnecessary cast.
Comment 40 Greg Singer 2004-05-19 09:51:06 PDT
Created attachment 148863 [details] [diff] [review]
Misc. fixes; also covers platform and appCodeName

Update fixes string type and checks rv before reassigning it, as well as
changing the whitespace in GetAppVersion to be a little more logical (lumping
pieces that get appended to aAppVersion together instead of grouping by where
the error checks went).  I also made it possible to override navigator.platform
(as requested) and navigator.appCodeName.  I'm not completely sure those should
be user-configurable, but I guess some sites do check them.  Hopefully, this'll
be useful :/.


>      aAppVersion.Append(PRUnichar(')'));
>
> Unnecessary cast.

If the ')' were assumed to be UTF-16, then it wouldn't be necessary, but on my
system at least, GCC sees

    aAppVersion.Append(')');

as appending an ASCII character to a UTF-16 string.  In addition to not really
being correct, it also doesn't compile (GCC 3.4).  Maybe I'm just
misunderstanding you, but the cast seems necessary to me.
Comment 41 Greg Singer 2004-05-19 16:29:04 PDT
Created attachment 148894 [details] [diff] [review]
Superfluous appCodeName override pulled.

Silly me, navigator.appCodeName is already configurable via
general.useragent.appName... platform appears to be hard-coded.  The override
has been removed, but I left changing |res| to |rv| in GetAppCodeName.	Would
it be worth it to similarly clean up the other functions (it's pretty much
uniformly |res| in this part of the file), or is it better to just leave that
stuff alone?
Comment 42 Michael van der Kolff 2004-06-05 23:49:50 PDT
Was just thinking, can this be put in the next release of mozilla?  It would be
really useful, and I think that the Status Whiteboard is saying that Opera has
already got it; Konqueror definitely does.  How do we go about getting it in the
next beta?
Comment 43 Steve Chapel 2004-07-11 18:21:32 PDT
*** Bug 245789 has been marked as a duplicate of this bug. ***
Comment 44 Greg Singer 2004-07-14 10:19:48 PDT
Created attachment 153170 [details] [diff] [review]
Update to apply cleanly against recent SeaMonkey CVS

The code above the patched area appears to have grown by about 80 lines or so;
patch updated for convenience.
Comment 45 Darin Fisher 2004-07-21 09:06:12 PDT
Comment on attachment 153170 [details] [diff] [review]
Update to apply cleanly against recent SeaMonkey CVS

asking jst to review this instead of me.  it's his module! ;-)
Comment 46 Johnny Stenback (:jst, jst@mozilla.com) 2004-07-21 11:07:05 PDT
Comment on attachment 153170 [details] [diff] [review]
Update to apply cleanly against recent SeaMonkey CVS

   nsCOMPtr<nsIHttpProtocolHandler>
-    service(do_GetService(kHTTPHandlerCID, &res));
-  if (NS_SUCCEEDED(res) && service) {
+    service(do_GetService(kHTTPHandlerCID, &rv));
+  if (NS_SUCCEEDED(rv) && service) {

I'd suggest replacing all occurances of the above if check with a check that
only tests the service pointer, no need to test both...

r+sr=jst. Let me know if you need help checking this in.
Comment 47 amano 2004-07-21 17:33:36 PDT
It would be nice if this fix could land on the Fx aviary branch as well. So a
completely functional User Agent Switch Extension - possibly developed by Chris
Pederick - would work with Fx 1.0.

Setting the blocking 1.0 flag to get the attention from the main firefox
developers. 
Comment 48 Johnny Stenback (:jst, jst@mozilla.com) 2004-07-21 20:26:02 PDT
Fix landed on the trunk (with some other minor related cleanup). Anyone wanna
port this to the aviary branch? I'll gladly check it in, but I'd need the patch
pretty soon to have time to get it in.

Marking bug FIXED.
Comment 49 Greg Singer 2004-07-22 11:32:15 PDT
Created attachment 154016 [details] [diff] [review]
Aviary port

Anything objectionable about this for the aviary branch?
Comment 50 Johnny Stenback (:jst, jst@mozilla.com) 2004-07-22 13:51:07 PDT
Not IMO. Landed on the aviary branch.
Comment 51 Asa Dotzler [:asa] 2004-12-01 16:25:01 PST
Comment on attachment 154016 [details] [diff] [review]
Aviary port

a=asa for 1.7.5.

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