Filing this because Alec says he's already got it in his tree
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla0.9.6
ack, I had the whole tree, but I ran my script once more and it whacked half my files.. more tomorrow.
I can't get the platform testing on this done by 0.9.6
Priority: P3 → P1
Target Milestone: mozilla0.9.6 → mozilla0.9.7
Alec, when do you think you'll get to this?
its pretty low on my radar right now :( I welcome anyone else to take this over... I just have too many other bugs to deal with..
Assignee: alecf → jaggernaut
Status: ASSIGNED → NEW
Created attachment 61012 [details] [diff] [review] Replace a bunch of WithConversion(" with NS_LITERAL_STRING
Attachment #54812 - Attachment is obsolete: true
Comment on attachment 61012 [details] [diff] [review] Replace a bunch of WithConversion(" with NS_LITERAL_STRING rs=alecf, assuming you've built on all 3 platforms. If you haven't built on all 3 platforms, then sr=alecf on the code you have built (i.e. land it asap) and then sr=alecf on the code as you test it.. The only reason I bring this up is that I made a run at this before, and found that there were a few goofy uses of AssignWithConversion that required me to hand-tweak a line or two. If that code is in Mac/Win and you've only tested Linux, you might bust Mac or Win. Anyway... this is great!
Attachment #61012 - Flags: superreview+
Did that, I now have Windows building, and as soon as I'm done wrestling my Mac I'll attach an updated patch.
Comment on attachment 61012 [details] [diff] [review] Replace a bunch of WithConversion(" with NS_LITERAL_STRING And I have r=bryner
Attachment #61012 - Flags: review+
Target Milestone: mozilla0.9.7 → mozilla0.9.8
Created attachment 61888 [details] [diff] [review] Patch that was checked in.
Attachment #61012 - Attachment is obsolete: true
I really hate to say this, but according to the tinderbox logs, it looks like this slowed down pageloading time..:(
Yes it did, by approx. 10ms on Linux, since it has to use NS_ConvertASCIItoUCS2 there. On Mac and Windows it won't have, since they use nsDependentString.
Checked in the third patch, I'll keep this bug open for now for any stragglers I run across between now and getting rid of *WithConversion.
jag: I am getting error message with today's pull UnicharSelfTest.cpp E:\raptor\mozilla\intl\unicharutil\tests\UnicharSelfTest.cpp(391) : error C2002 invalid wide-character constant E:\raptor\mozilla\intl\unicharutil\tests\UnicharSelfTest.cpp(391) : error C2002 invalid wide-character constant E:\raptor\mozilla\intl\unicharutil\tests\UnicharSelfTest.cpp(438) : error C2002 invalid wide-character constant E:\raptor\mozilla\intl\unicharutil\tests\UnicharSelfTest.cpp(438) : error C2002 invalid wide-character constant NMAKE : fatal error U1077: 'C:\MICROS~1\VC98\BIN\cl.exe' : return code '0x2' Stop. NMAKE : fatal error U1077: '"C:\MICROSOFT VISUAL STUDIO\VC98\BIN\NMAKE.exe"' : eturn code '0x2' Stop. I am not certain why MS compiler doesn't like inString.Assign(NS_LITERAL_STRING("\x61\x62\x80\xA0\x63")); with Japanese system locale setting. I changed my DOS command shell to 'us' to use the en locale; but it didn't help. I think MSVC++ is doing something wacky. I may be the only NS developer (occationally) compiling moz under Win98-Japanese; however, we need to get this resolved for Japanese Moz contributors. :) Note, this error *may* occur for other OS languages as well.
what version of msvc? have you installed any VC++ service packs? you might try upgrading to VC++ service pack 5...
If that still doesn't work, replace those two lines with this: 437 nsString inString(NS_ConvertASCIItoUCS2("\x61\x62\x80\xA0\x63")); I don't see why L"\x61\x62\x80\xA0\x63" (one of the steps in NS_LITERAL_STRING on Windows) wouldn't work, though.
See bug 115786.
cc myself I'm not sure if applying the service pack fixes the problem. (waiting from momoi-san's upgrade :) However, while I was compiling Moz with -UNICODE flag, http://bugzilla.mozilla.org/attachment.cgi?id=60387&action=view I had the similar problem with L"\x61\x62\x80\xA0\x63" I was forced to use TEXT("\x61\x62\x80\xA0\x63"); to work around the compilation error.
jag: I havent' tried TEXT() with Win98-JA build yet......
Roy, what does TEXT() do? Will it work on all platforms?
>Will it work on all platforms? No. I think it's only for Win32. :( After reading the online help on TEXT() and the header file, it may not help us after all. :( Do you think we can fool the compiler by doing the 2)? I don't think so; but I am not very much of a compiler guru. 1) MS Online Help: The TEXT macro identifies a string as Unicode when the UNICODE is defined during compilation. Otherwise, it identifies a string as an ANSI string. TEXT( LPTSTR string // ANSI or Unicode string ); Parameters string Pointer to the string to be interpreted as either Unicode or ANSI. 2) Header file #ifdef UNICODE #define __TEXT(quote) L##quote #else #define __TEXT(quote) quote #endif #define TEXT(quote) __TEXT(quote)
Hmmm... That is kinda what NS_LITERAL_STRING does internally, see nsLiteralString.h, except it's based on whether wchar_t is unsigned 16 bits. This is what NS_LITERAL_STRING("\x61\x62\x80\xA0\x63") expands to: NS_STATIC_CAST(const nsAFlatString&, nsDependentString(NS_REINTERPRET_CAST(const PRUnichar*, L"\x61\x62\x80\xA0\x63"), PRUint32((sizeof(L"\x61\x62\x80\xA0\x63") / sizeof(wchar_t))-1))) Is the L" failing here because the compiler for some reason has already turned it into a |const wchar_t|? Can you add the url to the online info on TEXT, or e-mail it to me?
>Is the L" failing here because the compiler for some reason has already >turned it into a |const wchar_t| Possibly. Can we change L" to TEXT(" for Win32 only to see if that helps? Meanwhile here is the online help: http://msdn.microsoft.com/library/default.asp?url=/library/en-us/intl/unicode_13lb.asp
Can you try this then: Index: mozilla/string/public/nsLiteralString.h =================================================================== RCS file: /cvsroot/mozilla/string/public/nsLiteralString.h,v retrieving revision 1.13 diff -u -r1.13 nsLiteralString.h --- mozilla/string/public/nsLiteralString.h 26 Nov 2001 05:59:35 -0000 1.13 +++ mozilla/string/public/nsLiteralString.h 19 Dec 2001 04:04:46 -0000 @@ -64,7 +64,11 @@ #endif #ifdef HAVE_CPP_2BYTE_WCHAR_T +#if defined(XP_WIN) && defined(UNICODE) + #define NS_LL(s) TEXT(s) +#else #define NS_LL(s) L##s +#endif #define NS_MULTILINE_LITERAL_STRING(s) nsDependentString(NS_REINTERPRET_CAST(const nsAString::char_type*, s), PRUint32((sizeof(s)/sizeof(wchar_t))-1)) #define NS_NAMED_MULTILINE_LITERAL_STRING(n,s) nsDependentString n(NS_REINTERPRET_CAST(const nsAString::char_type*, s), PRUint32((sizeof(s)/sizeof(wchar_t))-1)) #else See if this fixes your build.
Your suggestion may work once we make Moz an Unicode app (ie. compile with -DUNICODE). However, we are not doing that just yet. So your |#ifdef| patch doesn't make any differences; thus we still have a compiler error. Make use of |NS_ConvertASCIItoUCS2("\x61\x62\x80\xA0\x63"));| get around of the compiler error. I suggest we use NS_ConvertASCIItoUCS2() for the time being.
Ah, I thought you were already compiling with -DUNICODE. So yeah, use NS_ConvertASCIItoUCS2 for now in this one specific place.
Created attachment 62393 [details] [diff] [review] Fixing a build bustage on non-ASCII OS here the patch to fix the compiler error on non-English os. jag: please check this in. thanks
Comment on attachment 62393 [details] [diff] [review] Fixing a build bustage on non-ASCII OS sr=jag
Attachment #62393 - Flags: superreview+
Comment on attachment 62393 [details] [diff] [review] Fixing a build bustage on non-ASCII OS I am not sure if this is a valid review (since I put the patch together); but here is /r=yokoyama
Attachment #62393 - Flags: review+
*** Bug 115786 has been marked as a duplicate of this bug. ***
Patch "Fixing a build bustage on non-ASCII OS" checked in.
roy, if thia has been checked in, do you want to mark it fixed?
momoi-san: I'll leave it to jag until he get rid of all *WithConversion. This bug has never been closed (by jag) so I am assuming he has something else to do.
Target Milestone: mozilla0.9.8 → mozilla0.9.9
Target Milestone: mozilla0.9.9 → Future
14 years ago
Depends on: 248687
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.