939445 - "general.appname.override" pref is ignored (Firefox 25 regression)

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Adam Moore]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/32.0.1678.0 Safari/537.36

Steps to reproduce:

Set "general.appname.override" pref to "Hello", then visited a test page that reports the value of navigator.appName.


Actual results:

navigator.appName = "Netscape"


Expected results:

navigator.appName = "Hello"

This was caused by a changeset for https://bugzilla.mozilla.org/show_bug.cgi?id=838146

https://hg.mozilla.org/mozilla-central/rev/7c7ef42d1fe1

general.appname.override no longer is honored due to the removal of the return:

   1.714 -nsresult
   1.715 +void
   1.716  NS_GetNavigatorAppName(nsAString& aAppName)
   1.717  {
   1.718    if (!nsContentUtils::IsCallerChrome()) {
   1.719      const nsAdoptingString& override =
   1.720        mozilla::Preferences::GetString("general.appname.override");
   1.721  
   1.722      if (override) {
   1.723        aAppName = override;
   1.724 -      return NS_OK;
   1.725      }
   1.726    }
   1.727  
   1.728    aAppName.AssignLiteral("Netscape");
   1.729 -  return NS_OK;
   1.730  }

Comment 1

[Robert Longson [:longsonr]]

Attachment: patch

Comment 2

[Robert Longson [:longsonr]]

Thanks for the pointer to the code to fix Adam.

https://hg.mozilla.org/integration/mozilla-inbound/rev/56791576d790

Comment 3

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/56791576d790

Comment 4

[Boris Zbarsky [:bzbarsky]]

Robert, thanks for picking this up!  Should we get this backported?

Comment 6

[[e-mail]]

I've hit this bug with one app which where testing IE browser using !navigator.appName!="netscape" method and I was not able to bypass it using a plugin.

+1
Please backport it.

Comment 7

[Robert Longson [:longsonr]]

Comment on attachment 8333558 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 838146
User impact if declined: At least one extension uses this: https://addons.mozilla.org/en-US/firefox/addon/user-agent-switcher/reviews/
Testing completed (on m-c, etc.):  landed on m-c with a test some time ago.
Risk to taking this patch (and alternatives if risky): low, it's only one line.
String or IDL/UUID changes made by this patch: none

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b718ba279b1a
https://hg.mozilla.org/releases/mozilla-beta/rev/ff4c59861cd4

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/ff4c59861cd4

Comment 10

[Petruta Horea [:phorea], Desktop Test Engineering (back on Sep 5th)]

I have modified the pref and then ran the script from 
http://www.w3schools.com/jsref/tryit.asp?filename=tryjsref_nav_appname. The navigator.appName was the configured pref or Netscape if the pref wasn't set.

Verified using Fx 26 beta 8 (20131125215016), latest Aurora (20131126004001), latest Nightly (20131126030201) on Win 7 64-bit, Ubuntu 13.04 32-bit and Mac OS 10.8.5. 
Considering that the issue is also covered by automated tests, I'm marking these versions as verified.

Comment 11

[Petruta Horea [:phorea], Desktop Test Engineering (back on Sep 5th)]

Removing the "verifyme" keyword as per Comment 10.