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

Categories

(Firefox for Android Graveyard :: Web Apps (PWAs), defect, P1)

ARM
Android
defect

Tracking

(firefox16 verified, firefox17 verified)

VERIFIED FIXED
Firefox 17
Tracking Status
firefox16 --- verified
firefox17 --- verified

People

(Reporter: jsmith, Assigned: wesj)

Details

Attachments

(3 files, 2 obsolete files)

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.
Attached image UTF 8 Weird Chars 1
Attached image UTF 8 Weird Chars 2
QA Contact: aaron.train
Priority: -- → P1
I think we're mangling these when we save them to disk. Need to dig more.
Attached patch WIP Patch (obsolete) — Splinter Review
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....
Attachment #645468 - Flags: feedback?(mbrubeck)
Comment on attachment 645468 [details] [diff] [review]
WIP Patch

Wrong patch....
Attachment #645468 - Flags: feedback?(mbrubeck)
Attached patch WIP Patch (obsolete) — Splinter Review
Attachment #645468 - Attachment is obsolete: true
Attachment #645469 - Flags: feedback?(mbrubeck)
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+
Status: NEW → ASSIGNED
tracking-fennec: --- → ?
Attached patch PatchSplinter Review
Whoops. Forgot about this. Here's the same patch then, with the comment fixed.
Assignee: nobody → wjohnston
Attachment #645469 - Attachment is obsolete: true
Attachment #647320 - Flags: review?(mbrubeck)
Attachment #647320 - Flags: review?(mbrubeck) → review+
Whiteboard: [qa+]
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+
https://hg.mozilla.org/mozilla-central/rev/8d2206c84e31
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 17
Status: RESOLVED → VERIFIED
Keywords: checkin-needed
Whiteboard: [qa+]
tracking-fennec: ? → ---
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.