reduce narrow windows API calls.

RESOLVED FIXED

Status

()

RESOLVED FIXED
11 years ago
9 years ago

People

(Reporter: dougt, Assigned: crowderbt)

Tracking

Trunk
x86
Windows NT
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 -
wanted1.9.0.x +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [not needed for 1.9])

Attachments

(4 attachments, 23 obsolete attachments)

9.72 KB, patch
Details | Diff | Splinter Review
26.62 KB, patch
Details | Diff | Splinter Review
172.02 KB, patch
pavlov
: review-
pavlov
: superreview-
Details | Diff | Splinter Review
29.95 KB, patch
Biesinger
: review+
Details | Diff | Splinter Review
(Reporter)

Description

11 years ago
We want to be using wide API calls where ever possible.
(Reporter)

Comment 1

11 years ago
Created attachment 304599 [details] [diff] [review]
ugly wip patch

lots of problem with this patch, but it shows what we are attempting.

note that this also have windows mobile changes which are not for this bug.
Created attachment 304695 [details] [diff] [review]
minimal changes to build with unicode defined

more to come
also note that I'm building with plugins and printing disabled for this patch
Created attachment 304822 [details] [diff] [review]
works with or without unicode defined

the question being how to enable building with unicode defined.  Right now there are 3 lines at the end of config.mk in config and nsprpub/config that add the define.  Should it be a mozconfig flag?
Attachment #304695 - Attachment is obsolete: true
OS: Mac OS X → Windows NT
Created attachment 305598 [details] [diff] [review]
more changes in gfx, toolkit and others
Attachment #304822 - Attachment is obsolete: true
Created attachment 305626 [details] [diff] [review]
xpcom changes
Attachment #305598 - Attachment is obsolete: true
Created attachment 305646 [details] [diff] [review]
fixes a linux build issue
Attachment #305626 - Attachment is obsolete: true
Created attachment 305767 [details] [diff] [review]
fixed some stuff that bsmedberg pointed out

This patch seems to break the extension search in the add-on manager.
Attachment #305646 - Attachment is obsolete: true
Created attachment 305774 [details] [diff] [review]
fixes errors that show up in debug build

The issue with the add-on manger appears to be a dependency that the add-on manager has on @mozilla.org/plugin/host;1.  There is going to need to be some configure magic to undefine UNICODE in plugins and printing.
Attachment #305767 - Attachment is obsolete: true
Attachment #305774 - Flags: review?
Attachment #305774 - Flags: review? → review?(benjamin)
This shouldn't be "turned on" (i.e. actually defining UNICODE) yet.  Just found some places in nspr where char* is being passed to system calls, which for some reason is only a compiler warning and not an error.
(Reporter)

Comment 11

11 years ago
did you define MOZ_UNICODE?
(Reporter)

Comment 12

11 years ago
Created attachment 305862 [details] [diff] [review]
patch to modules/plugin

need to verify if this builds without UNICODE defined.
Attachment #304599 - Attachment is obsolete: true
(Reporter)

Updated

11 years ago
Attachment #305862 - Attachment is patch: true
Attachment #305862 - Attachment mime type: application/octet-stream → text/plain
NSS isn't Unicode safe on Windows either, see bug 413171.
Created attachment 305913 [details] [diff] [review]
builds without UNICODE defined
Attachment #305774 - Attachment is obsolete: true
Attachment #305774 - Flags: review?(benjamin)
Created attachment 305964 [details] [diff] [review]
removed most of the #ifdef UNICODE's
Attachment #305913 - Attachment is obsolete: true
Created attachment 305989 [details] [diff] [review]
changes based on stuart's comments
Attachment #305964 - Attachment is obsolete: true
Attachment #305989 - Flags: review?
Attachment #305989 - Flags: review? → review?(benjamin)
Created attachment 306068 [details] [diff] [review]
switched from wchar_t to PRUnichar and integrated plugin changes
Attachment #305862 - Attachment is obsolete: true
Attachment #305989 - Attachment is obsolete: true
Attachment #306068 - Flags: review?
Attachment #305989 - Flags: review?(benjamin)
Attachment #306068 - Flags: review? → review?(benjamin)

Comment 18

11 years ago
Comment on attachment 305989 [details] [diff] [review]
changes based on stuart's comments

>Index: browser/components/migration/src/nsIEProfileMigrator.cpp
>@@ -1410,7 +1410,7 @@
> {
>   HRESULT result;
> 
>-  IUniformResourceLocator* urlLink = nsnull;
>+  IUniformResourceLocatorW* urlLink = nsnull;
>   result = ::CoCreateInstance(CLSID_InternetShortcut, NULL, CLSCTX_INPROC_SERVER,
>                               IID_IUniformResourceLocator, (void**)&urlLink);

I think you need to modify IID_IUniformResourceLocator also.

>-          *aOutURL = PL_strdup(lpTemp);
>-
>+          *aOutURL = *aOutURL = (char*)ToNewUTF8String(nsDependentString(lpTemp));

Note, this changes the allocator for the outparam (was NSPR, now XPCOM)... but the old code was incorrect, and I've verified that the (one) caller handles the XPCOM allocator correctly (passed to getter_Copies).

>Index: browser/components/shell/src/nsWindowsShellService.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/browser/components/shell/src/nsWindowsShellService.cpp,v
>retrieving revision 1.51
>diff -u -w -r1.51 nsWindowsShellService.cpp
>--- browser/components/shell/src/nsWindowsShellService.cpp	26 Nov 2007 19:56:52 -0000	1.51
>+++ browser/components/shell/src/nsWindowsShellService.cpp	27 Feb 2008 11:18:58 -0000
>@@ -409,7 +409,7 @@
>   STARTUPINFO si = {sizeof(si), 0};
>   PROCESS_INFORMATION pi = {0};
> 
>-  BOOL ok = CreateProcess(NULL, (LPSTR)appHelperPath.get(), NULL, NULL,
>+  BOOL ok = CreateProcess(NULL, (LPTSTR)appHelperPath.get(), NULL, NULL,

This is incorrect, you're casting something that's always a char* to LPTSTR, which could be a WCHAR*.

It should be pretty trivial to convert this code to use CreateProcessW.

>Index: embedding/browser/activex/src/plugin/XPCDocument.cpp

>@@ -1915,7 +1915,7 @@
>                 NS_SUCCEEDED(baseURI->GetSpec(spec)))
>             {
>                 USES_CONVERSION;
>-                if (FAILED(CreateURLMoniker(NULL, T2CW(spec.get()), &baseURLMoniker)))
>+                if (FAILED(CreateURLMoniker(NULL, A2CW((char*)spec.get()), &baseURLMoniker)))

T2CW is correct, but I don't like the cast. Why is it necessary?

>Index: gfx/src/thebes/nsSystemFontsWin.cpp

>-  if (aIsWide)
>     memcpy(name, ptrLogFont->lfFaceName, LF_FACESIZE*2);
>-  else {
>-    MultiByteToWideChar(CP_ACP, 0, ptrLogFont->lfFaceName,
>-      strlen(ptrLogFont->lfFaceName) + 1, name, sizeof(name)/sizeof(name[0]));
>-  }

The "aIsWide" parameter is now unused, and I'm pretty sure it should be removed altogether to avoid confusion.

>Index: gfx/thebes/src/gfxWindowsSurface.cpp

>+    wchar_t *title = (wchar_t*)(nsPromiseFlatString(titleStr).get());

This code won't work. The nsPromiseFlatString temporary goes out of scope immediately, while you're storing the pointer for later use. You need to make the temporary last as long as "title". Same thing below with "docName"

Secondly, .get() returns a const PRUnichar* and you're casting to wchar_t*... is that necessary? can't title be "const wchar_t*"? 

>Index: intl/uconv/src/nsWinCharset.cpp

>Index: modules/libpr0n/decoders/icon/win/nsIconChannel.cpp

>-  char fileNativePath[MAX_PATH];
>-  nsCAutoString fileNativePathStr;
>-  aFile->GetNativePath(fileNativePathStr);
>-  ::GetShortPathName(fileNativePathStr.get(), fileNativePath, sizeof(fileNativePath));
>+  wchar_t fileNativePath[MAX_PATH];
>+  nsAutoString fileNativePathStr;
>+  aFile->GetPath(fileNativePathStr);
>+  ::GetShortPathNameW(fileNativePathStr.get(), fileNativePath, sizeof(fileNativePath));

This code was already broken, so you don't need to fix it here, but on systems with 8.3 shortnames disabled the GetShortPathName function will fail... i'm not sure why we need the shortname here, but if we really do, we should have a fallback or error check. Can you file a bug about this?

>@@ -342,7 +342,9 @@
> 
>   // Not a special folder, or something else failed above.
>   if (!shellResult)
>-    shellResult = ::SHGetFileInfo(filePath.get(), FILE_ATTRIBUTE_ARCHIVE, &sfi, sizeof(sfi), infoFlags);
>+      shellResult = ::SHGetFileInfoW(
>+                                    NS_ConvertUTF8toUTF16(filePath).get(),
>+                                    FILE_ATTRIBUTE_ARCHIVE, &sfi, sizeof(sfi), infoFlags);

This hunk is incorrect. Either we should make "filePath" a unicode string, or we should be doing NS_ConvertNativeToUTF16... filePath is definitely not UTF8.

>Index: netwerk/base/src/nsSocketTransport2.cpp

>-            tryAgain = nsNativeConnectionHelper::OnConnectionFailed(SocketHost().get());
>+            tryAgain = nsNativeConnectionHelper::OnConnectionFailed(
>+		    NS_ConvertUTF8toUTF16(SocketHost()).get()
>+);

This change needs r=biesi or a necko peer... I'm not sure whether SocketHost is always UTF8.

>Index: nsprpub/pr/include/md/_win95.h

I am not a NSPR peer. Please separate out the NSPR changes into a separate patch for review by wtc.

>Index: security/nss/cmd/shlibsign/shlibsign.c

I am not an NSS peer, please separate out NSS changes for review by wtc/kaie/nelsonb.

>Index: toolkit/components/startup/src/nsUserInfoWin.cpp

>-    *aUsername = nsCRT::strdup(username);
>-    
>+    *aUsername = (char*)NS_ConvertUTF16toUTF8( nsCRT::strdup(username)).get();
>     if (*aUsername) return NS_OK;

This is incorrect. I think you want

*aUsername = ToNewUTF8String(username);

>Index: toolkit/xre/nsNativeAppSupportWin.cpp

> // Simple Win32 mutex wrapper.
> struct Mutex {
>-    Mutex( const char *name )
>+    Mutex( const wchar_t *name )
>         : mName( name ),
>           mHandle( 0 ),
>           mState( -1 ) {
>-        mHandle = CreateMutexA( 0, FALSE, mName.get() );
>+        mHandle = CreateMutex( 0, FALSE, mName.get() );

This is where we get into concerns about compatibility. This mutex is supposed to work with old versions of FF also. If an old version does CreateMutexA("foo") and a new version does CreateMutexW("foo"), are those the same mutex? If not, then we'll need an explicit WINCE ifdef and keep the narrow versions of this code on the desktop.

I'm going to need to review the toolkit/xre changes separately... can you create a patch just for them?


>Index: uriloader/exthandler/win/nsMIMEInfoWin.cpp

>-        sinfo.lpVerb   = (LPCSTR)&cmdVerb;
>+        sinfo.lpVerb   = (LPTSTR)&cmdVerb;

This cast is incorrect unless cmdVerb is WCHAR, and I'm a little confused by the use of & on a type that is already an array. const_cast<LPCSTR>(cmdVerb) is better code.

>Index: widget/src/windows/nsBidiKeyboard.cpp

>-  NS_ASSERTION((strlen(mCurrentLocaleName) < KL_NAMELENGTH), 
>+  NS_ASSERTION((_tcslen(mCurrentLocaleName) < KL_NAMELENGTH), 
>     "GetKeyboardLayoutName return string length >= KL_NAMELENGTH");

wcslen here

>-  NS_ASSERTION((strlen(mRTLKeyboard) < KL_NAMELENGTH), 
>+  NS_ASSERTION((_tcslen(mRTLKeyboard) < KL_NAMELENGTH), 
>     "mLTRKeyboard has string length >= KL_NAMELENGTH");
>-  NS_ASSERTION((strlen(mLTRKeyboard) < KL_NAMELENGTH), 
>+  NS_ASSERTION((_tcslen(mLTRKeyboard) < KL_NAMELENGTH), 
>     "mRTLKeyboard has string length >= KL_NAMELENGTH");

and here

>-  NS_ASSERTION((strlen(localeName) < KL_NAMELENGTH), 
>+  NS_ASSERTION((_tcslen(localeName) < KL_NAMELENGTH), 
>     "GetKeyboardLayout return string length >= KL_NAMELENGTH");

and here

>Index: widget/src/windows/nsClipboard.cpp

>     // Display the string.
>-    MessageBox( NULL, (const char *)lpMsgBuf, "GetLastError", MB_OK|MB_ICONINFORMATION );
>+    MessageBoxW( NULL, (LPCWSTR)lpMsgBuf, L"GetLastError", MB_OK|MB_ICONINFORMATION );

This change is incorrect without also changing to FormatMessageW directly above.

>Index: widget/src/windows/nsDeviceContextSpecWin.h

>-  void GetDriverName(char *&aDriverName) const   { aDriverName = mDriverName;     }
>-  void GetDeviceName(char *&aDeviceName) const   { aDeviceName = mDeviceName;     }
>+  void GetDriverName(TCHAR *&aDriverName) const   { aDriverName = mDriverName;     }
>+  void GetDeviceName(TCHAR *&aDeviceName) const   { aDeviceName = mDeviceName;     }

Can these and below be WCHAR?

>Index: widget/src/windows/nsSound.cpp

>+#include "nsString.h"
>+#include "nsXPCOMStrings.h"

You shouldn't need nsXPCOMStrings.h in internal code, see below

>-    nsCAutoString nativeSoundAlias;
>-    NS_CopyUnicodeToNative(aSoundAlias, nativeSoundAlias);
>-    ::PlaySound(nativeSoundAlias.get(), nsnull, SND_ALIAS | SND_ASYNC);
>+    const PRUnichar* data;
>+    NS_StringGetData(aSoundAlias, &data);
>+    ::PlaySoundW(data, nsnull, SND_ALIAS | SND_ASYNC);

::PlaySoundW(PromiseFlatString(aSoundAlias).get(), ...)

>Index: xpcom/glue/nsGREGlue.cpp

Pretty much all the code in xpcom/glue is incorrect: you are doing utf8->utf16 conversions instead of native->utf16 conversions, and you aren't allowed to use mozilla's string functions to find out where Mozilla is. Please separate this part out and we can discuss the proper solution.

>Index: xpcom/io/SpecialSystemDirectory.cpp

>+    gShell32DLLInst = LoadLibraryA("Shell32.dll");

I'll bet you meant LoadLibraryW(L"Shell32.dll");

>Index: xpcom/io/nsLocalFileWin.cpp

>Index: xpcom/stub/nsXPComStub.cpp

> EXPORT_XPCOM_API(nsresult)
>-NS_GetFrozenFunctions(XPCOMFunctions *functions, const char* /* libraryPath */)
>+NS_GetFrozenFunctions(XPCOMFunctions *functions, const char* libraryPath)

The old way prevents a strict warning about unused variables.

>Index: xpcom/threads/nsProcessCommon.cpp

More tchar stuff... we should convert all the way to unicode if we're going to. It's not clear what effect you're trying to achieve or what casts might end up bad from this change.
Attachment #305989 - Attachment is obsolete: false

Comment 19

11 years ago
Comment on attachment 306068 [details] [diff] [review]
switched from wchar_t to PRUnichar and integrated plugin changes 

Clearing review: I'd like to get the changes from the last patch separately in so I don't have to wade through a bunch of stuff I've already reviewed.
Attachment #306068 - Flags: review?(benjamin)
Created attachment 306167 [details] [diff] [review]
changes based on bsmedberg's comments
Attachment #305989 - Attachment is obsolete: true
Attachment #306068 - Attachment is obsolete: true
Attachment #306167 - Flags: review?
Attachment #306167 - Flags: review? → review?(benjamin)
Attachment #306170 - Flags: review? → review?(wtc)
Attachment #306173 - Flags: review? → review?(wtc)

Comment 24

11 years ago
Comment on attachment 306170 [details] [diff] [review]
nss changes

This patch contains only a whitespace change.  Did you attach the wrong patch?
Attachment #306170 - Attachment is obsolete: true
Attachment #306170 - Flags: review?(wtc)
Created attachment 306219 [details] [diff] [review]
fixes some compiler warnings in nspr
Attachment #306173 - Attachment is obsolete: true
Attachment #306219 - Flags: review?
Attachment #306173 - Flags: review?(wtc)

Comment 26

11 years ago
Comment on attachment 306167 [details] [diff] [review]
changes based on bsmedberg's comments

>Index: config/config.mk

Please just leave this out for now.

>Index: embedding/browser/activex/src/plugin/XPConnect.cpp

>-        HMODULE hlib = ::LoadLibrary("xpcom.dll");
>+      HMODULE hlib = ::LoadLibraryW(L"xpcom.dll");

nit, untabify

>Index: netwerk/base/src/nsSocketTransport2.cpp


>-        if (autodialEnabled)
>-            tryAgain = nsNativeConnectionHelper::OnConnectionFailed(SocketHost().get());
>+        if (autodialEnabled){
>+          // SocketHost returns a nsCString, which is 8-bit
>+          tryAgain = nsNativeConnectionHelper::OnConnectionFailed(
>+		    NS_ConvertUTF8toUTF16(SocketHost()).get());
>+	}

Please attach these bits as a separate patch for review by biesi or a necko peer.

>Index: netwerk/protocol/file/src/nsFileProtocolHandler.cpp


>-    IUniformResourceLocator* urlLink = nsnull;
>+    IUniformResourceLocatorW* urlLink = nsnull;
>     result = ::CoCreateInstance(CLSID_InternetShortcut, NULL, CLSCTX_INPROC_SERVER,
>                                 IID_IUniformResourceLocator, (void**)&urlLink);

Whoops, didn't catch this the first time: need to use the W form of the IID here.

>Index: rdf/datasource/src/nsFileSystemDataSource.cpp

>     PRInt32         driveType;
>-    char            drive[32];
>+    PRUnichar            drive[32];
>     PRInt32         volNum;

please line these up.


>Index: widget/src/windows/nsBidiKeyboard.cpp


>   if (IsRTLLanguage(locale)) {
>-    strncpy(mRTLKeyboard, localeName, KL_NAMELENGTH);
>+      swprintf(mRTLKeyboard, localeName, KL_NAMELENGTH);
>     mRTLKeyboard[KL_NAMELENGTH-1] = '\0'; // null terminate
>   }
>   else {
>-    strncpy(mLTRKeyboard, localeName, KL_NAMELENGTH);
>+      swprintf( mLTRKeyboard, localeName, KL_NAMELENGTH);
>     mLTRKeyboard[KL_NAMELENGTH-1] = '\0'; // null terminate

nit, these should be two-space indented

>Index: widget/src/windows/nsClipboard.cpp


>-  else
>-    format = ::RegisterClipboardFormat(aMimeStr);
>+  else{
>+
>+  format = ::RegisterClipboardFormatW(NS_ConvertASCIItoUTF16(aMimeStr).get());
>+}

nit, follow existing style, avoid braces around the single-line else clause, and indent the clause properly

>Index: widget/src/windows/nsDataObj.h

> #ifndef CFSTR_INETURLA
>-#define CFSTR_INETURLA    "UniformResourceLocator"
>+#define CFSTR_INETURLA    TEXT("UniformResourceLocator")
> #endif
> #ifndef CFSTR_INETURLW
>-#define CFSTR_INETURLW    "UniformResourceLocatorW"
>+#define CFSTR_INETURLW    TEXT("UniformResourceLocatorW")

No TEXT here... either "" or L""

>Index: widget/src/windows/nsDeviceContextSpecWin.cpp


>+      PRUnichar* deviceName;
>+      PRUNichar* driverName;

typo

>Index: widget/src/windows/nsDeviceContextSpecWin.h

>-  void GetDriverName(char *&aDriverName) const   { aDriverName = mDriverName;     }
>-  void GetDeviceName(char *&aDeviceName) const   { aDeviceName = mDeviceName;     }
>+  void GetDriverName(TCHAR *&aDriverName) const   { aDriverName = mDriverName;     }
>+  void GetDeviceName(TCHAR *&aDeviceName) const   { aDeviceName = mDeviceName;     }

no tchars

>Index: widget/src/windows/nsFilePicker.cpp


>-      MessageBox(ofn.hwndOwner,
>+      MessageBoxW(ofn.hwndOwner,
>                  0,
>-                 "The filepicker was unexpectedly closed by Windows.",
>+                 L"The filepicker was unexpectedly closed by Windows.",
>                  MB_ICONERROR);

nit, please re-indent the secondary lines


>Index: widget/src/windows/nsWindow.h
>===================================================================
>RCS file: /cvsroot/mozilla/widget/src/windows/nsWindow.h,v
>retrieving revision 3.246
>diff -u -1 -0 -r3.246 nsWindow.h
>--- widget/src/windows/nsWindow.h	19 Dec 2007 19:40:20 -0000	3.246
>+++ widget/src/windows/nsWindow.h	28 Feb 2008 01:08:25 -0000
>@@ -98,20 +98,27 @@
> const LPCWSTR kWClassNameContent      = L"MozillaContentWindowClass";
> const LPCWSTR kWClassNameContentFrame = L"MozillaContentFrameWindowClass";
> const LPCWSTR kWClassNameGeneral      = L"MozillaWindowClass";
> const LPCWSTR kWClassNameDialog       = L"MozillaDialogClass";
> const LPCSTR kClassNameHidden         = "MozillaHiddenWindowClass";
> const LPCSTR kClassNameUI             = "MozillaUIWindowClass";
> const LPCSTR kClassNameContent        = "MozillaContentWindowClass";
> const LPCSTR kClassNameContentFrame   = "MozillaContentFrameWindowClass";
> const LPCSTR kClassNameGeneral        = "MozillaWindowClass";
> const LPCSTR kClassNameDialog         = "MozillaDialogClass";
>+const LPCTSTR kTClassNameHidden       = TEXT("MozillaHiddenWindowClass");
>+const LPCTSTR kTClassNameUI           = TEXT("MozillaUIWindowClass");
>+const LPCTSTR kTClassNameContent      = TEXT("MozillaContentWindowClass");
>+const LPCTSTR kTClassNameContentFrame = TEXT("MozillaContentFrameWindowClass");
>+const LPCTSTR kTClassNameGeneral      = TEXT("MozillaWindowClass");
>+const LPCTSTR kTClassNameDialog       = TEXT("MozillaDialogClass");

I don't understand why we need both style here... if the explanation is complicated, then Neil or a widget peer should review all this code under widget/src/windows


>Index: xpcom/build/nsXPCOMPrivate.h

>+#define LXPCOM_SEARCH_KEY L"PATH"
>+#define LGRE_CONF_NAME    L"gre.config"
>+#define LGRE_WIN_REG_LOC  L"Software\\mozilla.org\\GRE"
>+#define LXPCOM_DLL        L"xpcom.dll"
>+#define LXUL_DLL          L"xul.dll"

Leave this change out until we understand the scope/plan of the xpcom/glue changes.

>Index: xpcom/glue/nsVersionComparator.cpp

as well as everything under xpcom/glue


>Index: xpcom/threads/nsProcessCommon.cpp

>-    retVal = CreateProcess(NULL,
>+    retVal = CreateProcessW(NULL,
>                            // const_cast<char*>(mTargetPath.get()),
>                            cmdLine,
>                            NULL,  /* security attributes for the new

nit, reindent

>Index: xpcom/windbgdlg/Makefile.in

>+OS_LIBS		+= Shell32.lib

all-lowercase so we don't break the mingw build

>Index: xpcom/windbgdlg/windbgdlg.cpp

>+#include <Shellapi.h>

Probably also lowercase for mingw
Attachment #306167 - Flags: review?(benjamin) → review+
Attachment #306167 - Flags: approval1.9?
(Reporter)

Updated

11 years ago
Flags: blocking1.9?
Comment on attachment 306167 [details] [diff] [review]
changes based on bsmedberg's comments

clearing this nom as there seems to be a new patch ...
Attachment #306167 - Flags: approval1.9?
Comment on attachment 306167 [details] [diff] [review]
changes based on bsmedberg's comments

a=beltzner
Attachment #306167 - Flags: approval1.9+
Discussed this with Doug.  Not blocking 1.9, but wanted-1.9.0.x.
Flags: wanted1.9.0.x+
Flags: blocking1.9?
Flags: blocking1.9-
This seems like a really, really bad thing to take this late in the 1.9 cycle.  Why did we want this?
Comment on attachment 306167 [details] [diff] [review]
changes based on bsmedberg's comments

>Index: browser/components/migration/src/nsIEProfileMigrator.cpp
>+          *aOutURL = *aOutURL = (char*)ToNewUTF8String(nsDependentString(lpTemp));

As pointed out on IRC, this has two problems:
 * you're assigning twice, unnecessarily
 * you have an unneeded cast that could just mask errors in the future

Comment 32

11 years ago
there are several files in here that aren't actually built anymore.  old gfx for example.

Comment 33

11 years ago
also, there are a lot of weird indention changes in here where it looks like you search and replaced without changing spacing.
This caused compilation errors in debug builds:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1204755960.1204758040.12955.gz

Also, should windows.h really be included in nsNativeConnectionHelper.h ?

And, for the record, the test failure in
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1204754076.1204756364.8959.gz
was (when viewing the log as ISO-8859-1 and pasting it here, which is UTF-8):
*** 40118 ERROR FAIL | parse+compute+serialize(font-family) should be idempotent for 'font: caption' | got "serif", expected "ååŒ æ•¨æ±¬ä æ¬ïˆ€ïˆ¸ï‰ˆï‰ˆæ®á€€Ð¾ê”ïˆ˜à¾€á\\ā" | /tests/layout/style/test/test_value_storage.html
Created attachment 308503 [details] [diff] [review]
same patch that I checked in, but it doesn't fail the mochitests now
Attachment #306167 - Attachment is obsolete: true
(Reporter)

Comment 36

11 years ago
Comment on attachment 308503 [details] [diff] [review]
same patch that I checked in, but it doesn't fail the mochitests now

Index: db/mork/src/morkFile.cpp

do we even care about mork anymore?

Index: extensions/auth/nsAuthSSPI.cpp

 static HINSTANCE                 sspi_lib; 
-static PSecurityFunctionTable    sspi;
+static PSecurityFunctionTableW    sspi;

Match he spacing


Index: gfx/src/thebes/nsSystemFontsWin.cpp

   name[0] = 0;
-  if (aIsWide)
     memcpy(name, ptrLogFont->lfFaceName, LF_FACESIZE*2);
-  else {

fix spacing


Index: gfx/src/windows/nsFontMetricsWin.cpp

   memset(aGlyphMetrics, 0, sizeof(GLYPHMETRICS)); // UMR: bug 46438
-  if (eGlyphAgent_UNKNOWN == mState) { // first time we have been in this function
-    // see if this platform implements GetGlyphOutlineW()
-    DWORD len = GetGlyphOutlineW(aDC, aChar, GGO_METRICS, aGlyphMetrics, 0, nsnull, &mMat);
-    if (GetLastError() == ERROR_CALL_NOT_IMPLEMENTED) {
-      // next time, we won't bother trying GetGlyphOutlineW()
-      mState = eGlyphAgent_ANSI;
-    }
-    else {
-      // all is well with GetGlyphOutlineW(), we will be using it from now on
       mState = eGlyphAgent_UNICODE;
-      return len;



Index: intl/uconv/src/nsWinCharset.cpp

   nsAutoString acp_key(NS_LITERAL_STRING("acp."));
-  acp_key.AppendWithConversion(acp_name);
+  acp_key.Append(acp_name);


Is this the right conversion?  shoult it be NS_ConvertUTF8toUTF16 or something?  



Index: modules/libpr0n/decoders/icon/win/nsIconChannel.cpp


-    shellResult = ::SHGetFileInfo(filePath.get(), FILE_ATTRIBUTE_ARCHIVE, &sfi, sizeof(sfi), infoFlags);
+    shellResult = ::SHGetFileInfoW(
+                                    NS_ConvertUTF8toUTF16(filePath).get(),
+                                    FILE_ATTRIBUTE_ARCHIVE, &sfi, sizeof(sfi), infoFlags);




Index: modules/libpr0n/decoders/icon/win/nsIconChannel.h


Not sure why you made this change.  why isn't using the abstract class correct?


modules/plugin/*

Did not review since I authored most of it. :-)




Index: uriloader/exthandler/win/nsOSHelperAppService.cpp


      HKEY hKey;
-     LONG err = ::RegOpenKeyEx(HKEY_CLASSES_ROOT, aProtocolScheme, 0,
+     LONG err = ::RegOpenKeyExA(HKEY_CLASSES_ROOT, aProtocolScheme, 0,
                                KEY_QUERY_VALUE, &hKey);


we want wide here, me thinks.



Index: widget/src/windows/nsBidiKeyboard.cpp


 
   if (IsRTLLanguage(locale)) {
-    strncpy(mRTLKeyboard, localeName, KL_NAMELENGTH);
+      swprintf(mRTLKeyboard, localeName, KL_NAMELENGTH);
     mRTLKeyboard[KL_NAMELENGTH-1] = '\0'; // null terminate
   }
   else {
-    strncpy(mLTRKeyboard, localeName, KL_NAMELENGTH);
+      swprintf( mLTRKeyboard, localeName, KL_NAMELENGTH);
     mLTRKeyboard[KL_NAMELENGTH-1] = '\0'; // null terminate
   }



Spacing is off a bit



Index: widget/src/windows/nsLookAndFeel.cpp

-  gShell32DLLInst = LoadLibrary("Shell32.dll");
+  gShell32DLLInst = LoadLibraryW(L"Shell32.dll");

and

Index: xpcom/io/SpecialSystemDirectory.cpp
     // so that we need to use GetProcAddress to get the pointer.
-    gShell32DLLInst = LoadLibrary("Shell32.dll");
+    gShell32DLLInst = LoadLibraryW(L"Shell32.dll");


^^^
in other places we use shell32.dll -- lets clean it up while we are here.



Index: widget/src/windows/nsSound.cpp


-    ::PlaySound(reinterpret_cast<const char*>(data), 0, flags);
+    ::PlaySoundA(reinterpret_cast<const char*>(data), 0, flags);

Could you add a comment stating the reason why the A version is used here (because |data| can be a string or blob)
Attachment #308503 - Flags: review+
>    name[0] = 0;
> -  if (aIsWide)
>      memcpy(name, ptrLogFont->lfFaceName, LF_FACESIZE*2);
> -  else {
> 
> fix spacing

Fixed, as well as switching to LOGFONTW
 
> 
> Index: gfx/src/windows/nsFontMetricsWin.cpp
> 
>    memset(aGlyphMetrics, 0, sizeof(GLYPHMETRICS)); // UMR: bug 46438
> -  if (eGlyphAgent_UNKNOWN == mState) { // first time we have been in this
> function
> -    // see if this platform implements GetGlyphOutlineW()
> -    DWORD len = GetGlyphOutlineW(aDC, aChar, GGO_METRICS, aGlyphMetrics, 0,
> nsnull, &mMat);
> -    if (GetLastError() == ERROR_CALL_NOT_IMPLEMENTED) {
> -      // next time, we won't bother trying GetGlyphOutlineW()
> -      mState = eGlyphAgent_ANSI;
> -    }
> -    else {
> -      // all is well with GetGlyphOutlineW(), we will be using it from now on
>        mState = eGlyphAgent_UNICODE;
> -      return len;
> 

Is there a comment here?

> 
> Index: intl/uconv/src/nsWinCharset.cpp
> 
>    nsAutoString acp_key(NS_LITERAL_STRING("acp."));
> -  acp_key.AppendWithConversion(acp_name);
> +  acp_key.Append(acp_name);
> 
> Is this the right conversion?  shoult it be NS_ConvertUTF8toUTF16 or something? 
> 

I believe this is the right thing, no conversion is needed, we're appending a wide char string to a wide char string

> 
> 
> 
> Index: modules/libpr0n/decoders/icon/win/nsIconChannel.h
> 
> 
> Not sure why you made this change.  why isn't using the abstract class correct?
> 
> 
> modules/plugin/*
> 
> Did not review since I authored most of it. :-)
> 
> 
> 
> 
> Index: uriloader/exthandler/win/nsOSHelperAppService.cpp
> 
> 
>       HKEY hKey;
> -     LONG err = ::RegOpenKeyEx(HKEY_CLASSES_ROOT, aProtocolScheme, 0,
> +     LONG err = ::RegOpenKeyExA(HKEY_CLASSES_ROOT, aProtocolScheme, 0,
>                                 KEY_QUERY_VALUE, &hKey);
> we want wide here, me thinks.
> 

Eventually, yes.  Will need to do a conversion, for now we can do this in the shunt I think.


> 
> 
> Index: widget/src/windows/nsBidiKeyboard.cpp
> 
> 
> 
>    if (IsRTLLanguage(locale)) {
> -    strncpy(mRTLKeyboard, localeName, KL_NAMELENGTH);
> +      swprintf(mRTLKeyboard, localeName, KL_NAMELENGTH);
>      mRTLKeyboard[KL_NAMELENGTH-1] = '\0'; // null terminate
>    }
>    else {
> -    strncpy(mLTRKeyboard, localeName, KL_NAMELENGTH);
> +      swprintf( mLTRKeyboard, localeName, KL_NAMELENGTH);
>      mLTRKeyboard[KL_NAMELENGTH-1] = '\0'; // null terminate
>    }
> 
> 
> 
> Spacing is off a bit
> 
> 
> 
> Index: widget/src/windows/nsLookAndFeel.cpp
> 
> -  gShell32DLLInst = LoadLibrary("Shell32.dll");
> +  gShell32DLLInst = LoadLibraryW(L"Shell32.dll");
> 
> and
> 
> Index: xpcom/io/SpecialSystemDirectory.cpp
>      // so that we need to use GetProcAddress to get the pointer.
> -    gShell32DLLInst = LoadLibrary("Shell32.dll");
> +    gShell32DLLInst = LoadLibraryW(L"Shell32.dll");
> 
> 
> ^^^
> in other places we use shell32.dll -- lets clean it up while we are here.
> 

I assume you mean it should be lowercase, I think that's needed for mingw32.

> 
> Index: widget/src/windows/nsSound.cpp
> 
> 
> -    ::PlaySound(reinterpret_cast<const char*>(data), 0, flags);
> +    ::PlaySoundA(reinterpret_cast<const char*>(data), 0, flags);
> 
> Could you add a comment stating the reason why the A version is used here
> (because |data| can be a string or blob)
> 

no, data is a string.  I've added the conversion. 
Status: NEW → ASSIGNED
Created attachment 308560 [details] [diff] [review]
updated based on dougt's comments [missed 1.9]

again, passes all mochitests
Attachment #308503 - Attachment is obsolete: true
Attachment #308560 - Flags: review?
(Reporter)

Updated

11 years ago
Attachment #308560 - Attachment is patch: true
Attachment #308560 - Attachment mime type: application/octet-stream → text/plain
(Reporter)

Comment 39

11 years ago
Comment on attachment 308560 [details] [diff] [review]
updated based on dougt's comments [missed 1.9]

discussed on aim; carrying flag forward.
Attachment #308560 - Flags: review? → review+
Attachment #308560 - Flags: review?(roc)
Created attachment 308564 [details] [diff] [review]
necko changes
Attachment #308564 - Flags: review?(cbiesinger)
(Reporter)

Comment 41

11 years ago
Comment on attachment 308560 [details] [diff] [review]
updated based on dougt's comments [missed 1.9]

carrying bsmedberg sr flag forward.
Attachment #308560 - Flags: review?(roc) → superreview?(benjamin)
(Reporter)

Updated

11 years ago
Attachment #308560 - Flags: superreview?(benjamin) → superreview+
Attachment #308560 - Flags: approval1.9?
Comment on attachment 308560 [details] [diff] [review]
updated based on dougt's comments [missed 1.9]

a1.9+=damons

/me runs for cover in the nearest ditch.  Pops an iodine pill and pulls on radiation suit.
Attachment #308560 - Flags: approval1.9? → approval1.9+
I skimmed through the patch and didn't see anything wrong.

But no-one in this bug ever mentioned *why* we're doing this and I'm curious why.
(Reporter)

Comment 44

11 years ago
roc:
currently we have a "shunt" library that provides an implementation for narrow APIs on Windows Mobile.  As you may know, there are no narrow windows apis defined on windows mobile.  In this shunt, most of the implementations are simple text conversion, but some more involved.  To reduce this extra layer of conversation, as well as avoid #ifdefs outside of the shunt layer, we would like to use the wide version of windows apis. this patch also fixes uses of tchar (whos type vary based on the define of UNICODE).
OK, that makes sense, thanks. I did not know that Windows Mobile only defines the W APIs. Strange they don't ship Uniscribe then...
(Reporter)

Comment 46

11 years ago
don't get me started....  although FreeType+HarfBuzz might be the right way to go.
Comment on attachment 308564 [details] [diff] [review]
necko changes

netwerk/base/src/nsAutodialWin.cpp
+    PRUnichar* key = nsnull;
+    PRUnichar* val = nsnull;

those should really be const, can you change that? (even though that's not your fault)

              ::GetProcAddress(mhRASapi32, "RasEnumConnectionsA");
              ::GetProcAddress(mhRASapi32, "RasEnumEntriesA");

shouldn't those get changed to the W versions?

you need to change the callers of GetDefaultEntryName. the size argument of RegQueryValueExW is in bytes, not in characters.

+++ netwerk/base/src/nsSocketTransport2.cpp	11 Mar 2008 05:26:57 -0000
+          // SocketHost returns a nsCString, which is 8-bit

that's kind of obvious...

Why are you including windows.h from nsNativeConnectionHelper.h?

+++ netwerk/protocol/file/src/nsFileProtocolHandler.cpp	11 Mar 2008 05:26:58 -0000
                                 IID_IUniformResourceLocator, (void**)&urlLink);

you have to make that IID_IUniformResourceLocatorW right?
Attachment #308564 - Flags: review?(cbiesinger) → review-
(In reply to comment #47)
> (From update of attachment 308564 [details] [diff] [review])
> netwerk/base/src/nsAutodialWin.cpp
> +    PRUnichar* key = nsnull;
> +    PRUnichar* val = nsnull;
> 
> those should really be const, can you change that? (even though that's not your
> fault)

sure thing

> 
>               ::GetProcAddress(mhRASapi32, "RasEnumConnectionsA");
>               ::GetProcAddress(mhRASapi32, "RasEnumEntriesA");
> 
> shouldn't those get changed to the W versions?

There actually is no W version of this call on desktop windows.  as a side note, there is no A version of this call on windows mobile, we need to do some ugly undef'ing and def'ing as a work around....


> 
> you need to change the callers of GetDefaultEntryName. the size argument of
> RegQueryValueExW is in bytes, not in characters.
> 
> +++ netwerk/base/src/nsSocketTransport2.cpp     11 Mar 2008 05:26:57 -0000
> +          // SocketHost returns a nsCString, which is 8-bit
> 
> that's kind of obvious...
> 
> Why are you including windows.h from nsNativeConnectionHelper.h?
> 
> +++ netwerk/protocol/file/src/nsFileProtocolHandler.cpp 11 Mar 2008 05:26:58
> -0000
>                                  IID_IUniformResourceLocator,
> (void**)&urlLink);
> 
> you have to make that IID_IUniformResourceLocatorW right?
> 

good catch

Comment 49

11 years ago
Comment on attachment 308560 [details] [diff] [review]
updated based on dougt's comments [missed 1.9]

please get rid of the changes to files that are no longer built

Index: gfx/src/* for example
Attachment #308560 - Flags: review+ → review-
Get actual review and superreview next time, please, instead of carrying it over from several iterations ago.

Comment 51

11 years ago
Comment on attachment 308560 [details] [diff] [review]
updated based on dougt's comments [missed 1.9]

this also was checked in with various C casts that dbaron asked you to remove last time.

I also don't see other module owners reviewing this.
Attachment #308560 - Flags: superreview+ → superreview-
(Reporter)

Comment 52

11 years ago
stuart is seeing problems with the assertion dialog in debug.  I think the problem is that we need to build the windbgdlg with the UNICODE flag enabled so that the command line (which is the text to be display in the debug dialog) is wide.  

http://lxr.mozilla.org/mozilla/source/xpcom/windbgdlg/windbgdlg.cpp#48

CreateProcessW

Comment 53

11 years ago
A little problem in nsDeviceContextWin.cpp (but this code is not used anyway):
nsresult nsDeviceContextWin::CopyLogFontToNSFont(HDC* aHDC, const LOGFONTW* ptrLogFont, nsFont* aFont) const
{
  name[0] = 0;
  if (aIsWide)
    memcpy(name, ptrLogFont->lfFaceName, LF_FACESIZE*sizeof(PRUnichar));
  else {
    MultiByteToWideChar(CP_ACP, 0, ptrLogFont->lfFaceName,
      strlen(ptrLogFont->lfFaceName) + 1, name, sizeof(name)/sizeof(name[0]));
  }

The parameter aIsWide is removed, so this code won't compile anymore.

Better is to first remove the old code in gfx/src/windows (bug 418104), and do this wide (and large) Windows patch.
Depends on: 418104
Nothing like this patch should go in until its changes are tested, which means actually _running_ the code that's being changed.  In some cases that'll be difficult because the code isn't used any more, but that should probably be a tip-off that the patch isn't ready to be committed.
Comment on attachment 308560 [details] [diff] [review]
updated based on dougt's comments [missed 1.9]

It appears that this patch missed landing in 1.9.
Attachment #308560 - Attachment description: updated based on dougt's comments → updated based on dougt's comments [missed 1.9]
Whiteboard: [missed 1.9 landing]
Comment on attachment 308560 [details] [diff] [review]
updated based on dougt's comments [missed 1.9]

Removing approval1.9+ per schrep's request.
Attachment #308560 - Flags: approval1.9+
Whiteboard: [missed 1.9 landing] → [missed 1.9 checkin]

Updated

11 years ago
Whiteboard: [missed 1.9 checkin] → [not needed for 1.9]
(Assignee)

Comment 57

10 years ago
Created attachment 332958 [details] [diff] [review]
necko patch updates

This is an interdiff of the changes between attachment 308564 [details] [diff] [review]: necko changes, and the patch I am about to upload.
Assignee: blassey → crowder
(Assignee)

Comment 58

10 years ago
Created attachment 332959 [details] [diff] [review]
v2: necko changes

Will request review after a round of testing.
Attachment #308564 - Attachment is obsolete: true
(Assignee)

Comment 59

10 years ago
Created attachment 332983 [details] [diff] [review]
v3: necko changes

Another round of changes; mostly what biesi asked for and a few little tweaks.
Attachment #332958 - Attachment is obsolete: true
Attachment #332959 - Attachment is obsolete: true
Attachment #332983 - Flags: review?

Updated

10 years ago
Blocks: 432792
(Assignee)

Updated

10 years ago
Attachment #332983 - Flags: review? → review?(cbiesinger)
Comment on attachment 332983 [details] [diff] [review]
v3: necko changes

+++ mozilla/netwerk/base/src/nsAutodialWin.cpp	Fri Aug  8 12:37:40 2008
+    nsresult result = GetDefaultEntryName(mDefaultEntryName, sizeof(mDefaultEntryName));

can you limit your lines to 80 characters?

     LOGD(("Autodial: Added address %s to RAS autodial db for entry %s.",
-     hostName, autodialEntry.szEntry));
+         hostName, autodialEntry.szEntry));

szEntry is a wide string. this will not work.

+++ mozilla/netwerk/base/src/nsAutodialWin.h	Fri Aug  8 12:37:40 2008
 typedef struct tagRASAUTODIALENTRYA {
     DWORD dwSize;
     DWORD dwFlags;
     DWORD dwDialingLocation;
-    CHAR szEntry[RAS_MaxEntryName + 1];
+    PRUnichar szEntry[RAS_MaxEntryName + 1];
 } RASAUTODIALENTRYA, *LPRASAUTODIALENTRYA;
 typedef RASAUTODIALENTRYA RASAUTODIALENTRY, *LPRASAUTODIALENTRY;

you should change those names to *W instead of *A

+++ mozilla/netwerk/base/src/nsSocketTransport2.cpp	Fri Aug  8 12:37:40 2008
+        if (autodialEnabled){

missing space before {

+                    rv = NS_NewURI(aURI, NS_ConvertUTF16toUTF8(lpTemp).get());

you don't need the .get() here
Attachment #332983 - Flags: review?(cbiesinger) → review+
(Assignee)

Comment 61

10 years ago
Created attachment 333249 [details] [diff] [review]
v4: necko changes

Sorry about the 80-column violation; a lot of other code in this file breaks that rule, so I had figured it was a different width for this module.  I've restored this line to fitting, though.

Fixed the rest of your suggestions, asking for another round of review, despite your + on last patch, since this patch is large and largely generated.
Attachment #332983 - Attachment is obsolete: true
Attachment #333249 - Flags: review?(cbiesinger)
(Assignee)

Comment 62

10 years ago
Comment on attachment 333249 [details] [diff] [review]
v4: necko changes

Woops, found a buffer overflow here, one more go needed.
Attachment #333249 - Attachment is obsolete: true
Attachment #333249 - Flags: review?(cbiesinger)
(Assignee)

Comment 63

10 years ago
Created attachment 333253 [details] [diff] [review]
v5: necko changes

This fixes a potential buffer overflow in the previous version of nsRASAutodial::GetFirstEntryName, to do with the change from strncpy to wcsncpy.  All instances of changes like this should be audited (I hope you spot some I've missed).
Attachment #333253 - Flags: review?(cbiesinger)
Attachment #333253 - Flags: review?(cbiesinger) → review+
(Assignee)

Comment 64

10 years ago
For the landing of the necko/ fixes:

changeset:   16594:f788da9b2165

Thanks for the careful and fast reviews!

This bug remains open for now, to track its dependencies.
(Assignee)

Comment 65

10 years ago
I had to back out 16594:f788da9b2165 this morning, because the tree was closed.  Will take another swing we it reopens.
(Assignee)

Comment 66

10 years ago
changeset:   16626:45df6b91be5a

Relanded.

Comment 67

10 years ago
I took a look at the patch "V5 : necko changes":

 // Add the specified address to the autodial directory.
-PRBool nsRASAutodial::AddAddressToAutodialDirectory(const char* hostName)
+PRBool nsRASAutodial::AddAddressToAutodialDirectory(const PRUnichar* hostName)
 {
     // Need to load the DLL if not loaded yet.
     if (!LoadRASapi32DLL())
         return PR_FALSE;
 
     // First see if there is already a db entry for this address. 
-    RASAUTODIALENTRY autodialEntry;
+    RASAUTODIALENTRYW autodialEntry;
     autodialEntry.dwSize = sizeof(RASAUTODIALENTRY);
     DWORD size = sizeof(RASAUTODIALENTRY);
     DWORD entries = 0;
 

I looks like it should read "sizeof(RASAUTODIALENTRYW)", I don't know if the sizes are the same. I would recommend to use "sizeof(autodialEntry)" though.

(Assignee)

Comment 68

10 years ago
And backed-out again.

changeset:   16627:5365113ba1fb
(Assignee)

Comment 69

10 years ago
Bernard:  Thanks, I'll fix that.
(Assignee)

Comment 70

10 years ago
Created attachment 333599 [details] [diff] [review]
v6: necko changes

Sorry to ask for review again, but I wanted to cleanup some sizeof() usages throughout the autodial module.  I'll check next to make sure bugzilla interdiff works.
Attachment #333253 - Attachment is obsolete: true
Attachment #333599 - Flags: review?(cbiesinger)
(Assignee)

Comment 71

10 years ago
Alas, bugzilla interdiff fails and I can't get a good interdiff otherwise, either...  Don't want to spend a lot of time on it, so I won't unless you need it.
Attachment #333599 - Flags: review?(cbiesinger) → review+
(Assignee)

Comment 72

10 years ago
changeset:   16689:f134ac75b142

Landed again (on a truly open tree this time!)

Comment 73

10 years ago
Comment on attachment 333599 [details] [diff] [review]
v6: necko changes

>-                    rv = NS_NewURI(aURI, lpTemp);
>-
>+                    rv = NS_NewURI(aURI, NS_ConvertUTF16toUTF8(lpTemp));
NS_NewURI has an nsAString& overload, no change needed here...
(Assignee)

Comment 74

10 years ago
(In reply to comment #73)
> NS_NewURI has an nsAString& overload, no change needed here...

This did need a change, the LPWSTR doesn't find an implicit conversion.  Instead I used an nsDependentString:

changeset:   18484:e3d348de9ee2

If the rest of this landing sticks, I'll attach the changeset info shortly.
(Assignee)

Comment 75

10 years ago
http://hg.mozilla.org/mozilla-central/rev/9698ed0a3e43

This landing finally stuck, with the one fix mentioned in comment #74.  Still keeping the bug open to track the remaining fixes.
Depends on: 452775
No longer depends on: 452775
Depends on: 452775

Comment 76

10 years ago
Did this cause bug 453156?
(Assignee)

Updated

9 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.