Last Comment Bug 738298 - navigator.mozApps.mgmt.getAll() doesn't handle non-ASCII characters in the manifest
: navigator.mozApps.mgmt.getAll() doesn't handle non-ASCII characters in the ma...
Status: VERIFIED FIXED
[qa!]
:
Product: Core
Classification: Components
Component: DOM: Apps (show other bugs)
: unspecified
: All All
: -- major (vote)
: mozilla17
Assigned To: [:fabrice] Fabrice Desré
: Jason Smith [:jsmith]
: [:fabrice] Fabrice Desré
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-22 09:39 PDT by David Flanagan [:djf]
Modified: 2012-08-06 12:33 PDT (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
+


Attachments
patch (1.15 KB, patch)
2012-07-31 02:52 PDT, [:fabrice] Fabrice Desré
21: review+
Details | Diff | Splinter Review

Description David Flanagan [:djf] 2012-03-22 09:39:17 PDT
When I use navigator.mozApps.mgmt.getAll() in the Gaia homescreen, and set the local to French, accented characters in the app name are garbled.  It appears that UTF-8 strings in the manifest are being treated as 7-bit ASCII and not properly converted into JavaScript UTF-16 strings.
Comment 1 Jason Smith [:jsmith] 2012-03-22 09:43:07 PDT
As a point of similarity, this issue also occurs with desktop native installation (see bug 733482). Seeing that this happens in more than spot makes me think that this UTF-8 character issue could be more areas we have not looked at yet.
Comment 2 Ian Bicking (:ianb) 2012-03-22 09:50:14 PDT
We should definitely include some non-ASCII manifests in our testing (right now all our test manifests are strictly ASCII).
Comment 3 dclarke@mozilla.com [:onecyrenus] 2012-03-22 17:45:02 PDT
Hey jason I had written a manual test with japanese characters. 

In the interim, could you add this test to your smoketest. 

https://github.com/mozilla/openwebapps/tree/develop/site/tests/apps/app3
Comment 4 dclarke@mozilla.com [:onecyrenus] 2012-03-22 17:48:52 PDT
To do that you need to run the node test server, and load appstore.html

the above app will be listed to be installed, and you should be able to install to the dashboard, view on dashboard..etc
Comment 5 Jason Smith [:jsmith] 2012-03-22 18:20:35 PDT
Yup, I've added the UTF-8 character test on native install here on test case IA-15 (https://docs.google.com/spreadsheet/ccc?key=0AiZoGR-iOAlUdDdfMHo5aTI2WjVIeWxnNWxvV0ZVOWc#gid=0).
Comment 6 Ian Bicking (:ianb) 2012-03-22 20:09:17 PDT
I you post the manifest somewhere I can add it to the /appdir/ list
Comment 7 dclarke@mozilla.com [:onecyrenus] 2012-03-23 16:01:31 PDT
ian that touches on another issue which is where / how to host test apps ......

I think the staging server would be a good spot for this, the question is whether we can get ops to open some firewall ports so we can run the node.js test server. 

i think that would be optimal
Comment 8 Ian Bicking (:ianb) 2012-03-23 16:03:22 PDT
I think appengine or heroku might be better hosting options for samples.
Comment 9 Jason Smith [:jsmith] 2012-03-23 16:13:50 PDT
Filed bug 738845 for the hosting of test apps. Let's pull the conversation on the test case applications over to that bug.
Comment 10 dclarke@mozilla.com [:onecyrenus] 2012-04-01 19:17:46 PDT
Jason, do you have an eta on this bug fix ?
Comment 11 Jason Smith [:jsmith] 2012-04-02 09:52:47 PDT
Which bug are you referring to? This one? Bug 738845? If it's bug 738845, please comment in that bug.
Comment 12 dclarke@mozilla.com [:onecyrenus] 2012-04-02 10:07:38 PDT
I was referring to this bug.
Comment 13 Jason Smith [:jsmith] 2012-04-02 10:09:28 PDT
This is an implementation bug - I'm not the appropriate person to fix this bug.
Comment 14 dclarke@mozilla.com [:onecyrenus] 2012-04-02 10:50:56 PDT
k, wanted to determine if you had more information about the bug than I did... 

Fabrice: do you have an eta on this bug fix ?
Comment 15 [:fabrice] Fabrice Desré 2012-04-02 11:14:35 PDT
I'll probably have a fix by the end of next week
Comment 16 Jason Smith [:jsmith] 2012-05-14 21:53:54 PDT
Nominating for k9o - this affects localized b2g apps
Comment 17 [:fabrice] Fabrice Desré 2012-07-31 02:52:53 PDT
Created attachment 647477 [details] [diff] [review]
patch

Tested by changing some manifest.webapp files in gaia to use utf-8 instead of the current escaping.
Comment 18 Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) 2012-07-31 03:03:24 PDT
Comment on attachment 647477 [details] [diff] [review]
patch

Review of attachment 647477 [details] [diff] [review]:
-----------------------------------------------------------------

\o/
Comment 19 [:fabrice] Fabrice Desré 2012-07-31 03:07:31 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/2bef20dfc8fb
Comment 20 Ryan VanderMeulen [:RyanVM] 2012-07-31 19:20:20 PDT
https://hg.mozilla.org/mozilla-central/rev/2bef20dfc8fb
Comment 21 Jason Smith [:jsmith] 2012-08-02 23:27:28 PDT
Testing on desktop, this looks okay outside of the regression that was caught on Android. I'll want to test this on Firefox OS though to be sure.
Comment 22 [:fabrice] Fabrice Desré 2012-08-03 07:14:09 PDT
(In reply to Jason Smith [:jsmith] from comment #21)
> Testing on desktop, this looks okay outside of the regression that was
> caught on Android. I'll want to test this on Firefox OS though to be sure.

Which android regression?
Comment 23 Jason Smith [:jsmith] 2012-08-03 08:03:58 PDT
(In reply to Fabrice Desré [:fabrice] from comment #22)
> (In reply to Jason Smith [:jsmith] from comment #21)
> > Testing on desktop, this looks okay outside of the regression that was
> > caught on Android. I'll want to test this on Firefox OS though to be sure.
> 
> Which android regression?

bug 779490
Comment 24 Jason Smith [:jsmith] 2012-08-06 12:33:41 PDT
Verified on 8/6/2012 Firefox OS Daily Build with Ed Lee's app with Japanese characters.

Note You need to log in before you can comment on or make changes to this bug.