Bug 27288 - Inspector: Missing UIString and other localizedString.js fixes
Summary: Inspector: Missing UIString and other localizedString.js fixes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-14 20:03 PDT by Joseph Pecoraro
Modified: 2009-07-24 00:52 PDT (History)
4 users (show)

See Also:


Attachments
Localized String Fixes (28.14 KB, patch)
2009-07-14 20:06 PDT, Joseph Pecoraro
timothy: review+
Details | Formatted Diff | Diff
plain/text version of the new file (10.17 KB, application/x-javascript)
2009-07-15 16:16 PDT, Joseph Pecoraro
no flags Details
Manual Diff (262 bytes, application/octet-stream)
2009-07-18 08:13 PDT, Joseph Pecoraro
no flags Details
Manual Diff (262 bytes, text/plain)
2009-07-18 08:15 PDT, Joseph Pecoraro
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2009-07-14 20:03:53 PDT
localizedStrings.js was missing "This storage is empty." and had two improperly sorted strings.
Comment 1 Joseph Pecoraro 2009-07-14 20:06:22 PDT
Created attachment 32763 [details]
Localized String Fixes

Added the missing string and correctly sorted two other keys ("COOKIES" and "Time").
Comment 2 Joseph Pecoraro 2009-07-14 20:09:33 PDT
I just realized COOKIES is not yet committed, its something that I've been working on.  Should I submit a new patch that doesn't have COOKIES in it but fixes the other issues?  Or should I leave in COOKIES which will fall into place when that bug gets resolved:
https://bugs.webkit.org/show_bug.cgi?id=27202
Comment 3 Joseph Pecoraro 2009-07-14 20:13:52 PDT
Last Comment, something I forgot to mention: I ran a grep to search for all the currently used UIString() calls and verified that they all exist in localizedStrings.js.  This means that all calls to UIString(), in WebCore/inspector/front-end/*.js are properly in localizedString.js.  I didn't look for strings that didn't have UIString but might need it, or in any other files.  Are there other places to hunt for mistake?

Cheers.
Comment 4 Eric Seidel (no email) 2009-07-15 16:05:11 PDT
Hard to review this type of patch w/o being able to see it.
Comment 5 Joseph Pecoraro 2009-07-15 16:16:58 PDT
Created attachment 32819 [details]
plain/text version of the new file

Hmm, when I do a `file WebCore/English.lproj/localizedStrings.js` I get no mime/type.  Do you think if I change the mime type by deleting and recreating the file it would be okay. Then the patches would apply as though it was text.  Or is this unusual format required for English.lproj to recognize it?
Comment 6 Jan Alonzo 2009-07-18 01:22:05 PDT
(In reply to comment #5)
> Created an attachment (id=32819) [details]
> plain/text version of the new file
> 
> Hmm, when I do a `file WebCore/English.lproj/localizedStrings.js` I get no
> mime/type.  Do you think if I change the mime type by deleting and recreating
> the file it would be okay. Then the patches would apply as though it was text. 
> Or is this unusual format required for English.lproj to recognize it?

Are you able to create a plain/text version of the diff? You can do this using iconv and just do a manual diff of the two files.
Comment 7 Joseph Pecoraro 2009-07-18 08:10:22 PDT
Okay a few things.  First the issue I think is causing the assumed binary.

  shell> file localizedStrings.js
  localizedStrings.js: 

Then iconv didn't actually work =/.

  shell> iconv -t UTF8 localizedStrings.js 
  iconv: localizedStrings.js:1:0: cannot convert

`cat`, `cp`, or even opening the file in an editor and saving it didn't change that information for me.  What I actually had to do was open it in an editor, copy the text over to a new document and save a new file.  I did this for both the new and old, attahed is the manual diff.
Comment 8 Joseph Pecoraro 2009-07-18 08:13:05 PDT
Created attachment 33024 [details]
Manual Diff

NOTES:

- I thought "Time" was incorrectly sorted, but that was just because I had just added two strings next to "Time" in apparently the wrong spot.
- This still has the new strings for "Cookies" which have yet to be added to the inspector.  https://bugs.webkit.org/show_bug.cgi?id=27202 So maybe you want me to just revert and then resubmit a patch with the 1 line fix.
Comment 9 Joseph Pecoraro 2009-07-18 08:15:56 PDT
Created attachment 33025 [details]
Manual Diff

Wow... that was frustrating.  Its clearly plain/text but that mime type wasn't working.  Take 2.
Comment 10 Patrick Mueller 2009-07-22 12:56:06 PDT
Anyone know why English.lproj/localizedStrings.js is maintained as UTF16 encoded?  I think I noticed Trac does the right thing with the file, but diffs and other tools don't.  The actual content of the file doesn't require it to be UTF16, even it has (or had or could have) non-ascii chars in it, because those would be in the JS strings and so could be escaped.

My only guess is some tool during a translation or build step is assuming UTF16.

In that case, perhaps we can keep the "development" version of the file as a plain old ascii file, using unicode escapes as needed.  For whatever tool needs the UTF16 version, we can write a filter invoked during build that will read the ascii version and convert to UTF16.
Comment 11 Adam Barth 2009-07-24 00:52:27 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/English.lproj/localizedStrings.js
Committed r46332
	M	WebCore/ChangeLog
	M	WebCore/English.lproj/localizedStrings.js
r46332 = 7995bbbd08706997f37891031a3b2ea353b5bcaf (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/46332