Closed Bug 444770 Opened 12 years ago Closed 10 years ago

Include WOW64/x64/ia64 in the User Agent string's platform when run on a 64-bit Windows OS

Categories

(Core :: General, enhancement)

x86_64
Windows Vista
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.3a5

People

(Reporter: jason_upton, Assigned: timeless)

References

(Blocks 1 open bug, )

Details

Attachments

(2 files, 3 obsolete files)

User-Agent:       Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 6.0; WOW64; SLCC1; .NET CLR 2.0.50727; .NET CLR 3.0.04506; Media Center PC 5.0; Zune 2.5)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9) Gecko/2008052906 Firefox/3.0

Normal DOM code doesn't know whether or not I'm running a 64-bit version of Windows or a 32-bit version of Windows because FF doesn't register WOW64 in the UA string.  See my script below in the repro steps.

Reproducible: Always

Steps to Reproduce:
1. Open 32-bit IE (either 7 or 8) on a 64-bit Vista or XP system
2. Visit http://www.useragentstring.com and see that they have WOW64 in their UA string
3. Visit http://www.useragentstring.com and see that they have WOW64 in their UA string

Alternatively, you can write this script that won't work right on FF:  
    //Check for XP
    if(navigator.userAgent.indexOf("Windows NT 5.1")>-1)
    {
      if(navigator.userAgent.indexOf("SV1")>-1)
      {
        bIsXPSP2=true;
      }
    }
   
    //Are they running the 64-bit version of the browser on some version of Windows?
      if(navigator.userAgent.indexOf("Win64")>-1)
      {
        bIs64BitBrowser=true;
      }    
      
    //WS03 and XP x64 case
    if(navigator.userAgent.indexOf("Windows NT 5.2")>-1)
    {      
      if(navigator.userAgent.indexOf("WOW64")>-1)
      {
        bIsXWS03X64=true;
        //While it's 64-bits, it's running the default 32-bit browser
      }
      else
      {
        bIsWS03X86=true;
      }
    } //NT5.2
   
    //Vista cases
    if(navigator.userAgent.indexOf("Windows NT 6.0")>-1)
    {      
      if(navigator.userAgent.indexOf("WOW64")>-1)
      {
        bIsVistaX64=true;
        document.write("Windows Vista (64-bit)")
        //While it's 64-bits, it's running the default 32-bit browser
      }
      else
      {
        bIsVistaX86=true;
        document.write("Windows Vista (32-bit)")
      }
    }//NT6.0
Actual Results:  
FF doesn't add WOW64 to the same part of the UA string.  

As a result, my driver download page can't detect whether the person is running a 32 or 64-bit version of Windows and offer them the right drivers.  This problem is going to get worse quickly.  

Expected Results:  
I'd expect that when you install FF, you'd detect the processor architecture (or even if you're being forced into Program Files(x86) instead of Program Files) and add this to the platform portion of the User Agent string 

I talked to Mike Schroepfer and Mike Shaver about this a couple of weeks ago.  They told me to file a bug so the issue doesn't get lost.
Severity: major → enhancement
Status: UNCONFIRMED → NEW
Component: OS Integration → General
Ever confirmed: true
Product: Firefox → Core
QA Contact: os.integration → general
Just found this old bug still having "new" status... I'm having the same problem, not being able to present a 64-bit download to Windows Firefox users, while this is no problem in Internet Explorer.

Unfortunately I don't have the spare time to setup a complete Mozilla build environment and create a patch myself, but I did have a look at the source code and it seems to me this is an easy fix. I included some code below and hope one of the devs will incorporate it into the source.

The User-Agent is constructed identically for Windows NT platforms in both the mozilla 1.9.1 (Firefox 3.5) and mozilla 1.9.2 (Firefox 3.6) branches, namely in nsHttpHandler::InitUserAgentComponents() :
http://mxr.mozilla.org/mozilla1.9.1/source/netwerk/protocol/http/src/nsHttpHandler.cpp#646
http://mxr.mozilla.org/mozilla1.9.2/source/netwerk/protocol/http/src/nsHttpHandler.cpp#647

I propose to add these two code fragments:
1. At the top, right after #include-ing <windows.h>, defining the IsWow64Process function type (the function needs to be called dynamically since not all NT platforms have this function - it exists since XP SP2 / Server 2003 SP1):
----------------
#if defined(XP_WIN)
#include <windows.h>
typedef BOOL (WINAPI *LPFN_ISWOW64PROCESS)(HANDLE, PBOOL);
#endif
----------------

2. Change the Windows NT User-Agent code, adding a few lines to get the function pointer, check if we're running WOW64 and if so, adding the WOW64 token to mOscpu, comparable to what is done a few lines further for UNIX systems, where " i686 (x86_64)" is added.
----------------
if (info.dwPlatformId == VER_PLATFORM_WIN32_NT) {
    if (info.dwMajorVersion      == 3)
        mOscpu.AssignLiteral("WinNT3.51");
    else if (info.dwMajorVersion == 4)
        mOscpu.AssignLiteral("WinNT4.0");
    else {
        char *buf = PR_smprintf("Windows NT %ld.%ld",
                                info.dwMajorVersion,
                                info.dwMinorVersion);
        if (buf) {
            BOOL isWow64 = FALSE;
            LPFN_ISWOW64PROCESS fnIsWow64Process = (LPFN_ISWOW64PROCESS)GetProcAddress(GetModuleHandle("kernel32"), "IsWow64Process");
            mOscpu = buf;
            PR_smprintf_free(buf);
            if (fnIsWow64Process != NULL)
                if (fnIsWow64Process(GetCurrentProcess(), &isWow64) != 0)
                    if (isWow64 == TRUE)
                        mOscpu.AppendLiteral("; WOW64");
        }
    }
} else {
--------------------------------

I'm hoping someone more familiar with mozilla development can turn this into a patch, get it tested, and hopefully incorporated in Firefox.
Hardware: x86 → x86_64
Blocks: 530811
Attached patch based on comment 1 (obsolete) — Splinter Review
Personally, I'm not really sure we want to do this. However, here it is.
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #430881 - Flags: review?(cbiesinger)
Comment on attachment 430881 [details] [diff] [review]
based on comment 1

I think we should do this, we do it for unix too. That said, we don't indicate whether we run as 64-bit, and I think we should do that too before checking this in.

+        if (fnIsWow64Process &&
+            !fnIsWow64Process(GetCurrentProcess(), &isWow64)) {
+            isWow64 = FALSE;

you never set isWow64 to true. I think you should make your if check the return value for true and set isWow64 to true in the if.
Attachment #430881 - Flags: review?(cbiesinger) → review-
Summary: Need to add WOW64 to the User Agent string's platform when installing on a 64-bit Windows OS → Include WOW64/x64/ia64 in the User Agent string's platform when run on a 64-bit Windows OS
Attached patch handle 64bit browsers (obsolete) — Splinter Review
biesi didn't read the winapi spec for that function :)

biesi insists that i support more than the original summary and instead try to honor the spirit of the long description. so this patch tries to do that.
Attachment #430881 - Attachment is obsolete: true
Attachment #432412 - Flags: review?(cbiesinger)
Comment on attachment 432412 [details] [diff] [review]
handle 64bit browsers

+        format = WNT_BASE "; x64";

do you want to be compatible with MSIE and make this "; Win64; x64"? (similar for IA64)
Attachment #432412 - Flags: review?(cbiesinger) → review+
also cc'ing gerv in case he has an opinion on this change to the user-agent string
I think that as long as the new string still meets the user agent spec:
https://developer.mozilla.org/en/User_Agent_Strings_Reference
and we are doing something that IE is already doing, then it's probably OK :-)

Does it still meet the UA spec?

Gerv
Attached patch we can try thatSplinter Review
gerv: this doesn't change anything outside the supposedly "free-form" comment section, so it should satisfy your requirements too
Attachment #432412 - Attachment is obsolete: true
Attachment #442981 - Flags: review+
http://hg.mozilla.org/mozilla-central/rev/4c43764259af
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a5
Version: unspecified → Trunk
Reopening this.
I rebuilt from tip on Win32 and nsHTTPHandler.cpp fails to build with this error message : 

d:/.../nsHttpHandler.cpp(740) : error C2664: 'GetModuleHandleW' : cannot convert parameter 1 from 'const nsAFlatString' to 'LPCWSTR' No user-defined-conversion operator available that can perform this conversion, or the operator cannot be called. I've got a fix.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch quick fix for LPCWSTR type (obsolete) — Splinter Review
Fix for the above.
Please, I'd need someone to push that to m-c for me. Thanks !
Attachment #444861 - Flags: review?(cbiesinger)
Comment on attachment 444861 [details] [diff] [review]
quick fix for LPCWSTR type

use GetModuleHandleW, please.

(but that's a really strange error message here...)
Attachment #444861 - Flags: review?(cbiesinger) → review-
(In reply to comment #12)
> (From update of attachment 444861 [details] [diff] [review])
> use GetModuleHandleW, please.

Done.
> 
> (but that's a really strange error message here...)
yeah, it's telling me I'm using the "W" version already !
Attachment #444861 - Attachment is obsolete: true
Attachment #444864 - Flags: review?(cbiesinger)
(In reply to comment #13)
> > (but that's a really strange error message here...)
> yeah, it's telling me I'm using the "W" version already !

That's not the strange part; that's a consequence of defining UNICODE. The strange part is that it suggests you had an nsAFlatString when in fact it was a const char[].
Attachment #444864 - Flags: review?(cbiesinger) → review+
ha! Mystery solved, case closed. I had previously attempted to cast using NS_LITTERAL, thus the nsAFlatString...So copy/paste mistake. But the real error message indeed says :

error C2664: 'GetModuleHandleW' : cannot convert parameter 1 from 'const char[9]' to 'LPCWSTR'.

thanks.
Keywords: checkin-needed
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
looks like this hasn't been pushed yet, so the bug shouldn't be marked fixed.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
http://hg.mozilla.org/mozilla-central/rev/ab47b2b089ad
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
See Also: → 1556223
Depends on: 1559747
You need to log in before you can comment on or make changes to this bug.