Closed Bug 766604 Opened 9 years ago Closed 9 years ago
Installing an app with UTF-8 characters does not show correct characters to the user on the shortcut, shortcut message, and app in about:apps
Steps: 1. Go to http://ed.agadak.net/app/ 2. Install Ed's app that should contain Japanese characters in the name Expected: The Japanese characters should be shown correctly on the shortcut message, the shortcut itself, and the app in about:apps. Actual: See screenshots. Garbled incorrect characters are shown instead of the correct Japanese characters.
I think we're mangling these when we save them to disk. Need to dig more.
This converts the title to UTF-8 before sending it to Java. That works, but I'm not exactly sure why. Hoping maybe you know mbrubeck? The downloaded manifest can be in any encoding. That's used to show the install prompt, which works just fine. We save it as UTF-8: http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm#248 Then, we read that back from the disk. http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm#166 AFAICT, the string should be UTF-8 at that point, but I'm not sure how to query it exactly (the nsIChannel reports nothing). http://mxr.mozilla.org/mozilla-central/source/netwerk/base/src/NetUtil.jsm#116 When we finally send this to Java, it expects UTF-16 which may be our problem? Except this patch just converts to UTF-8....
Comment on attachment 645468 [details] [diff] [review] WIP Patch Wrong patch....
Comment on attachment 645469 [details] [diff] [review] WIP Patch Review of attachment 645469 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/chrome/content/browser.js @@ +5962,5 @@ > > + // The manifest is stored as UTF-8. This is a responseText value from reading the manifest file, which > + // should be UTF-8. sendMessageToJava expects UTF-16 strings, so I'm not sure why UTF-8 fixes this > + let converter = Cc["@mozilla.org/intl/scriptableunicodeconverter"].createInstance(Ci.nsIScriptableUnicodeConverter); > + converter.charset = "UTF-8"; If I understand correctly, this is converting *from* UTF-8 to UTF-16. Our internal Unicode strings are always UTF-16, so ConvertToUnicode always converts *to* UTF-16, and ConvertFromUnicode always converts *from* UTF-16. So this code looks exactly correct, except for the comment.
Attachment #645469 - Flags: feedback?(mbrubeck) → feedback+
Whoops. Forgot about this. Here's the same patch then, with the comment fixed.
Attachment #647320 - Flags: review?(mbrubeck) → review+
Comment on attachment 647320 [details] [diff] [review] Patch [Approval Request Comment] Bug caused by (feature/regressing bug #): Always present User impact if declined: Apps using UTF-8 characters will have them scrambled Testing completed (on m-c, etc.): landed on mc today Risk to taking this patch (and alternatives if risky: Low risk. Mobile only. Should only help. String or UUID changes made by this patch: None.
Attachment #647320 - Flags: approval-mozilla-aurora?
Attachment #647320 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 17
You need to log in before you can comment on or make changes to this bug.