Localized blackberry.app.name and blackberry.app.description #277

jamesjhedley · 2012-11-20T17:51:31Z

Testing scenarios;
-Test blackberry.app API's and ensure nothing is broken.
-Test blackberry.app.name and blackberry.app.description returns localized values when language is changed on the device
-Confirm tests are passing in functional test app

jamesjhedley · 2012-11-20T17:51:59Z

@jeffheifetz Code review please.

jamesjhedley · 2012-11-20T17:55:02Z

Going to update the automatic tests as well, so this doesn't happen again. But putting pull request up now so it can begin being reviewed etc.

jeffheifetz · 2012-11-20T18:23:25Z

Looks good James, r+ with the automated changes

jamesjhedley · 2012-11-20T19:15:24Z

@jeffheifetz Updated blackberry.app functional tests to include localization tests for name&description
Please review and r+.

nukulb · 2012-11-20T19:21:04Z

kills our non-localized test. :(

Can this be included in as a separate app? hate that we are not testing the most common case of blackberry.app

jamesjhedley · 2012-11-20T19:25:29Z

@nukulb Why does it "kill our non-localized test"? The existing name&description elements that contain the non-localized values are still there. I'm simply including additional localized values as well.

nukulb · 2012-11-20T19:53:28Z

@jkeshavarzi you are correct, I didn't read your tests properly, its a good test but I don't know why it works.
If you set the language to {} in the webview out of process why does it on the controller side, which is where I assume you check navigator.language, right?

nukulb · 2012-11-20T19:53:59Z

@tracyli - can you give this a quick test, we need respin the SCM bundle with this fix

tracyli · 2012-11-20T20:09:16Z

tested in 10.0.9.1103: blackberry.app works as before;blackberry.app.name and blackberry.app.description change with localized values; app test suite got passed in functional test app.

There's an observation:
1.in config.xml, there's xml:lang="fr-fr" for app name 'App-FR'
expected: app name shows up in screen as 'App-FR' and 'blackberry.app.name' returns 'App-FR' when the localized language is Fr
fact: app name shows up in screen as expected but 'blackberry.app.name' returns default value
question: I need to change xml:lang="fr-fr" to be xml:lang="fr-FR" to make it work, is it what expected?

jamesjhedley · 2012-11-20T20:14:45Z

@tracyli I think you mean " I need to change xml:lang="fr" to be xml:lang="fr-FR" to make it work, is it what expected?".
Currently there is no logic to default to the parent locale [fr] if the specific locale isn't available [fr-CA].

Working on it.

jamesjhedley · 2012-11-20T21:37:30Z

Fixed two issues;

Case sensitive locale strings
Default to language locale data [fr] if region isn't available [fr-CA]

Please use blackberry-webworks/BB10-Webworks-Packager#111 when testing.
As it contains part of the fix for 1) and uses this Framework.

@jeffheifetz Can you please take a look at the new changes?
When he's done, can you please retest @tracyli.

jeffheifetz · 2012-11-20T22:34:53Z

Please add unit tests to cover the locale generaliztion (fr when fr-CA is not available)

jeffheifetz · 2012-11-21T00:05:14Z

r+

jeffheifetz · 2012-11-21T00:06:57Z

ext/app/client.js

+        return data[locale];
+    } else if (data[locale.split("-")[0]]) {
+        //Region specific locale [i.e. fr-CA]
+        return data[locale.split("-")[0]];


Isn't this comment misleading, isn't this the "region unspefic" locale?

this returns fr for fr-CA?

Yup

From: Eric Pearson [mailto:notifications@github.com]
Sent: Tuesday, November 20, 2012 07:21 PM
To: blackberry-webworks/BB10-WebWorks-Framework BB10-WebWorks-Framework@noreply.github.com
Cc: James Keshavarzi
Subject: Re: [BB10-WebWorks-Framework] Localized blackberry.app.name and blackberry.app.description (#277)

In ext/app/client.js:

value = readOnlyValues ? readOnlyValues[field] : null; Object.defineProperty(_self, field, {"value": value, "writable": false});

}

+function getLocalizedText(data) {

var locale = navigator.language.toLowerCase();

if (data[locale]) {

return data[locale];

} else if (data[locale.split("-")[0]]) {

//Region specific locale [i.e. fr-CA]

return data[locale.split("-")[0]];

this returns fr for fr-CA?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/277/files#r2189016.

This transmission (including any attachments) may contain confidential information, privileged material (including material protected by the solicitor-client or other applicable privileges), or constitute non-public information. Any use of this information by anyone other than the intended recipient is prohibited. If you have received this transmission in error, please immediately reply to the sender and delete this information from your system. Use, dissemination, distribution, or reproduction of this transmission by unintended recipients is not authorized and may be unlawful.

You're right. I shall fix it.

From: Jeffrey Heifetz [mailto:notifications@github.com]
Sent: Tuesday, November 20, 2012 07:07 PM
To: blackberry-webworks/BB10-WebWorks-Framework BB10-WebWorks-Framework@noreply.github.com
Cc: James Keshavarzi
Subject: Re: [BB10-WebWorks-Framework] Localized blackberry.app.name and blackberry.app.description (#277)

In ext/app/client.js:

value = readOnlyValues ? readOnlyValues[field] : null; Object.defineProperty(_self, field, {"value": value, "writable": false});

}

+function getLocalizedText(data) {

var locale = navigator.language.toLowerCase();

if (data[locale]) {

return data[locale];

} else if (data[locale.split("-")[0]]) {

//Region specific locale [i.e. fr-CA]

return data[locale.split("-")[0]];

Isn't this comment misleading, isn't this the "region unspefic" locale?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/277/files#r2188880.

This transmission (including any attachments) may contain confidential information, privileged material (including material protected by the solicitor-client or other applicable privileges), or constitute non-public information. Any use of this information by anyone other than the intended recipient is prohibited. If you have received this transmission in error, please immediately reply to the sender and delete this information from your system. Use, dissemination, distribution, or reproduction of this transmission by unintended recipients is not authorized and may be unlawful.

Are you sure this is ok? If this is portuguese and you return pt for pt-BR a lot of people are going to be confused as the language is pretty different. Same goes for mandarin and cantonese with zh-CN and zh-HK.

Nevermind disregard my comments, it's probably good enough

Would be interesting to see what the os does in these situations. I know, tracy and I were seeing it default to the language locale (fr) when the region wasn't available.

From: Eric Pearson [mailto:notifications@github.com]
Sent: Tuesday, November 20, 2012 07:27 PM
To: blackberry-webworks/BB10-WebWorks-Framework BB10-WebWorks-Framework@noreply.github.com
Cc: James Keshavarzi
Subject: Re: [BB10-WebWorks-Framework] Localized blackberry.app.name and blackberry.app.description (#277)

In ext/app/client.js:

value = readOnlyValues ? readOnlyValues[field] : null; Object.defineProperty(_self, field, {"value": value, "writable": false});

}

+function getLocalizedText(data) {

var locale = navigator.language.toLowerCase();

if (data[locale]) {

return data[locale];

} else if (data[locale.split("-")[0]]) {

//Region specific locale [i.e. fr-CA]

return data[locale.split("-")[0]];

Are you sure this is ok? If this is portuguese and you return pt for pt-BR a lot of people are going to be confused as the language is pretty different. Same goes for mandarin and cantonese with zh-CN and zh-HK.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/277/files#r2189066.

This transmission (including any attachments) may contain confidential information, privileged material (including material protected by the solicitor-client or other applicable privileges), or constitute non-public information. Any use of this information by anyone other than the intended recipient is prohibited. If you have received this transmission in error, please immediately reply to the sender and delete this information from your system. Use, dissemination, distribution, or reproduction of this transmission by unintended recipients is not authorized and may be unlawful.

tracyli · 2012-11-21T00:58:07Z

tested in 10.0.9.1103 with packager build http://ci0000003863287:8080/hudson/job/BB10-Webworks-Packager-next-fix-case-sensitive/3/ and framework build http://ci0000003863287:8080/hudson/job/BB10-Webworks-Framework-next-issue378/4/: blackberry.app works as before;blackberry.app.name and blackberry.app.description change with localized values; app test suite got passed in functional test app

nukulb · 2012-11-21T03:26:42Z

@jkeshavarzi can I get a squash so we can commit this?

Fixes blackberry#378 Reviewed By: Jeffrey Heifetz <jheifetz@rim.com> Tested By: Tracy Li <tli@rim.com>

jamesjhedley · 2012-11-21T06:12:56Z

-Comment fixed
-Squashed commits
-Merged manually [https://github.com/blackberry-webworks/BB10-WebWorks-Framework/commit/9d756e5f29e2df4ed0aafbe3672e773a784d5f4a]

jeffheifetz closed this Nov 20, 2012

jeffheifetz reopened this Nov 20, 2012

jamesjhedley mentioned this pull request Nov 20, 2012

Locale information for name/description incorrectly case sensitive blackberry-webworks/BB10-Webworks-Packager#111

Closed

jeffheifetz reviewed Nov 21, 2012
View reviewed changes

Localized blackberry.app.name and blackberry.app.description

12b58de

Fixes blackberry#378 Reviewed By: Jeffrey Heifetz <jheifetz@rim.com> Tested By: Tracy Li <tli@rim.com>

jamesjhedley closed this Nov 21, 2012

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Localized blackberry.app.name and blackberry.app.description #277

Localized blackberry.app.name and blackberry.app.description #277

jamesjhedley commented Nov 20, 2012

jamesjhedley commented Nov 20, 2012

jamesjhedley commented Nov 20, 2012

jeffheifetz commented Nov 20, 2012

jamesjhedley commented Nov 20, 2012

nukulb commented Nov 20, 2012

jamesjhedley commented Nov 20, 2012

nukulb commented Nov 20, 2012

nukulb commented Nov 20, 2012

tracyli commented Nov 20, 2012

jamesjhedley commented Nov 20, 2012

jamesjhedley commented Nov 20, 2012

jeffheifetz commented Nov 20, 2012

jeffheifetz commented Nov 21, 2012

jeffheifetz Nov 21, 2012

ericpearson Nov 21, 2012

jamesjhedley Nov 21, 2012

jamesjhedley Nov 21, 2012

ericpearson Nov 21, 2012

ericpearson Nov 21, 2012

jamesjhedley Nov 21, 2012

tracyli commented Nov 21, 2012

nukulb commented Nov 21, 2012

jamesjhedley commented Nov 21, 2012

Localized blackberry.app.name and blackberry.app.description #277

Localized blackberry.app.name and blackberry.app.description #277

Conversation

jamesjhedley commented Nov 20, 2012

jamesjhedley commented Nov 20, 2012

jamesjhedley commented Nov 20, 2012

jeffheifetz commented Nov 20, 2012

jamesjhedley commented Nov 20, 2012

nukulb commented Nov 20, 2012

jamesjhedley commented Nov 20, 2012

nukulb commented Nov 20, 2012

nukulb commented Nov 20, 2012

tracyli commented Nov 20, 2012

jamesjhedley commented Nov 20, 2012

jamesjhedley commented Nov 20, 2012

jeffheifetz commented Nov 20, 2012

jeffheifetz commented Nov 21, 2012

jeffheifetz Nov 21, 2012

Choose a reason for hiding this comment

ericpearson Nov 21, 2012

Choose a reason for hiding this comment

jamesjhedley Nov 21, 2012

Choose a reason for hiding this comment

jamesjhedley Nov 21, 2012

Choose a reason for hiding this comment

ericpearson Nov 21, 2012

Choose a reason for hiding this comment

ericpearson Nov 21, 2012

Choose a reason for hiding this comment

jamesjhedley Nov 21, 2012

Choose a reason for hiding this comment

tracyli commented Nov 21, 2012

nukulb commented Nov 21, 2012

jamesjhedley commented Nov 21, 2012