Bug 26375 – VC++ 2005 Express failed to build WebKit due to raw UTF-8 string in WebKit/win/WebCoreLocalizedStrings.cpp

• Component: New Bugs
• Status: RESOLVED FIXED

[Takeshi Yoshino]

VC++ 2005 Express failed to build WebKit due to raw UTF-8 string in WebKit/win/WebCoreLocalizedStrings.cpp . It shows warning that the file contains some non ASCII characters (in my Japanese environment, it said it found some characters not in Windows-932 codepage). By default VC++ treats warnings as errors and stops building.

Actually, WebKit/win/WebCoreLocalizedStrings.cpp contains a multiplication sign (U+00D7) encoded in UTF-8 (0xC3 0x97).

Checkout is ToT r44649

[Takeshi Yoshino]

Created attachment 31242 [details]
Proposed fix for 26375

[Adam Barth]

Comment on attachment 31242 [details]
Proposed fix for 26375

LGTM.  Thanks.

[Darin Adler]

Comment on attachment 31242 [details]
Proposed fix for 26375

This change is incompatible with the scripts that we uses to maintain the localized string files, so while it fixes compilation for the people using these development tools, it breaks WebKit development work flow for anyone using the localization scripts, unless we make some changes to the script used to extract localizable strings.

[Adam Barth]

Comment on attachment 31242 [details]
Proposed fix for 26375

Marking as R- based on comment #3.  I'm surprised we've been able to get away with non-ASCII source files thus far.  Maybe we should change the scripts?

[Darin Adler]

(In reply to comment #4)
> Marking as R- based on comment #3. I'm surprised we've been able to get away
> with non-ASCII source files thus far. Maybe we should change the scripts?

Until this point in the project, all the compilers we support were consistent in that in C strings (non-wide) they allowed non-ASCII characters, including the same raw bytes in the executable that were in the source file. This meant we were free to use UTF-8 for all strings, and not do any special escaping.

I'm disappointed to find we need to support compilers incompatible with this technique.

If we decide that this is no longer allowed, then we need to teach the scripts to deal with some sort of all-ASCII syntax. I think actual UTF-8 sequences with \x for each byte are a really ugly choice; it would be nice to have something that was aesthetically pleasing. I don't have any immediate good ideas at the moment about how to resolve this.

[Darin Adler]

Perhaps the issue is entirely with warnings. Maybe we can fix this by turning off the warning. Or is the compiler actually trying to transcode the string?

[Adam Barth]

Other projects I've been involved with have used XML as the source representations of localized strings (which, of course, is natively UTF-8).

[Takeshi Yoshino]

I see. So, for now, may I put some comments into the code to let the Windows-based developer know the workaround, e.g. URL to this bug entry? I'll send a new patch to do so, later.

[Takeshi Yoshino]

Created attachment 31405 [details]
Proposed fix for 26375 (rev 2)

[David Levin]

*** Bug 21849 has been marked as a duplicate of this bug. ***

[Eric Seidel (no email)]

Could you please post the warning number, and the description (even if it's in Japanese).

[Takeshi Yoshino]

(In reply to comment #11)
> Could you please post the warning number, and the description (even if it's in
> Japanese).
> 

Here's the warning.

5>WebCoreLocalizedStrings.cpp
5>..\WebCoreLocalizedStrings.cpp : error C2220: warning treated as error - no 'object' file generated
5>..\WebCoreLocalizedStrings.cpp : warning C4819: The file contains a character that cannot be represented in the current code page (932). S
ave the file in Unicode format to prevent data loss
5>Project : warning PRJ0018 : The following environment variables were not found:
5>$(PRODUCTION)
5>Build log was saved at "file://E:\wk1_svn_cygwin\WebKitBuild\obj\WebKit\Debug\BuildLog.htm"
5>WebKit - 1 error(s), 1 warning(s)
========== Build: 4 succeeded, 1 failed, 13 up-to-date, 0 skipped ==========

[Takeshi Yoshino]

Created attachment 31531 [details]
Proposed fix for 26375 (rev 3)

Included the warning messages to the patch.

[Eric Seidel (no email)]

What happens if we "Save the file in Unicode format"?  Does it just add a UTF8 BOM to the top of the file?

[Takeshi Yoshino]

(In reply to comment #14)
> What happens if we "Save the file in Unicode format"?  Does it just add a UTF8
> BOM to the top of the file?
> 

I couldn't find what the message "Save the file in Unicode format" in C4819 (http://msdn.microsoft.com/en-us/library/ms173715.aspx) exactly means. However, Visual Studio 2005 Professional has a "Advanced Save Options" for its editor. I saw five encoding options in the dialog with "Unicode" in them.

So, I've tested building WebKit by saving WebCoreLocalizedString.cpp with each of them. Whether a BOM is added or not is also described.

Build mode used: Release (because run-safari in debug mode on Windows fails too frequently)
Check: Navigate to some image (http://www.google.co.jp/intl/ja_jp/images/logo.gif) and check the title

Save mode: Unicode (UTF-8 with signature) - Codepage 65001
The source was encoded by UTF-8 w/ BOM
Build: Successful
Result: localized string not found

Save mode: Unicode - Codepage 1200
The source was encoded by UTF-16 Little Endian w/o BOM
Build: Successful
Result: localized string not found

Save mode: Unicode (Big-Endian) - Codepage 1201
The source was encoded by UTF-16 Big-Endian w/o BOM
Build: Successful
Result: localized string not found

Save mode: Unicode (UTF-7) - Codepage 65000
The source was encoded by UTF-7
Build: Failure
4>WebCoreLocalizedStrings.cpp
4>..\WebCoreLocalizedStrings.cpp(1) : error C2059: syntax error : '/'
4>..\WebCoreLocalizedStrings.cpp(7) : error C2059: syntax error : '+'
4>..\WebCoreLocalizedStrings.cpp(13) : error C2137: empty character constant
4>..\WebCoreLocalizedStrings.cpp(35) : error C2653: 'WebCore' : is not a class or namespace name

Save mode: Unicode (UTF-8 without signature) - Codepage 65001
The source was encoded by UTF-8 w/o BOM
Build: Failure. We need to remove /Wx option (take warnings as errors)
Build w/o /Wx: Successful
Result: localized string not found

None of them fixed the problem. I saw "logo.gif 276×110 pixels image" on the title bar only when I replace the character with escape sequences (\xC3\x97).

Thanks,

[Eric Seidel (no email)]

Comment on attachment 31531 [details]
Proposed fix for 26375 (rev 3)

Wow!  Thank you for the awesome investigation.

Unfortunately the current patch doesn't seem to actually have the code change, just a comment?

This is the only log message you need in the ChangeLog:
WebCoreLocalizedStrings.cpp : warning C4819: The file contains a character that cannot be represented in the current code page (932). Save the file in Unicode format to prevent data loss

The rest of the build output you can remove from the ChangeLog.

r- for lack of code change.  Otherwise I'd r+ this.  Thanks again for the investigation!

[Eric Seidel (no email)]

Comment on attachment 31531 [details]
Proposed fix for 26375 (rev 3)

oops, meant to r- it until a new patch is posted with the code change (and adjusted ChangeLog)

[Takeshi Yoshino]

(In reply to comment #16)
> (From update of attachment 31531 [details] [review])
> Wow!  Thank you for the awesome investigation.
> 
> Unfortunately the current patch doesn't seem to actually have the code change,
> just a comment?
> 
> This is the only log message you need in the ChangeLog:
> WebCoreLocalizedStrings.cpp : warning C4819: The file contains a character that
> cannot be represented in the current code page (932). Save the file in Unicode
> format to prevent data loss
> 
> The rest of the build output you can remove from the ChangeLog.
> 

OK. Thanks.

> r- for lack of code change.  Otherwise I'd r+ this.  Thanks again for the
> investigation!
> 

As Darin said, if I make code change on WebCoreLocalizedStrings.cpp without making change on extract-localized-strings script, I'll break Localized.strings generation process. Do you mean I should make change on extract-localized-strings instead of putting a comment?

Thanks,

[Takeshi Yoshino]

Created attachment 33143 [details]
Proposed fix for 26375 (rev 4)

Wrote a change on WebKitTools/Scripts/extract-localizable-strings so
that replacing a raw string in UI_STRING with hexadecimal escape
sequences won't break localized string generation process.

[David Levin]

Assign to levin for landing.

[David Levin]

Committed as http://trac.webkit.org/changeset/46245

[Takeshi Yoshino]

(In reply to comment #21)
> Committed as http://trac.webkit.org/changeset/46245

Thank you!