[B2G][L10n][Settings][Internet Sharing] Security field shows "wpa2-psk" by default, not loading translated strings

RESOLVED FIXED

Status

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: petercpg, Assigned: mihai)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: LocRun1.3)

Attachments

(2 attachments, 1 obsolete attachment)

Steps to Reproduce:
1. Switch to English (US) locale
2. Go to Settings -> Internet Sharing, Security field shows "wpa2-psk"
3. Tap Hotspot settings, switch to other security protocols, tap OK
4. Security field shows "open" or "WPA (TKIP)"
5. Go to Hotspot settings again and switch to WPA2 (AES)
6. Security field now shows "WPA2 (AES)"

Expected Results: The initial security level should be the same to the option name, "WPA2 (AES)"

Actual Results: "wpa2-psk"


Device: Geeksphone Keon
Build: 20140205010019
Gaia: 3405205
Platform Version: 28.0
Created attachment 8370603 [details]
2014-02-05-16-52-28.png
(Assignee)

Updated

5 years ago
Assignee: nobody → mihai
Created attachment 8371136 [details] [review]
Pull Request #16010 - Localize WiFi hotspot security type string
Attachment #8371136 - Flags: review?(arthur.chen)
Comment on attachment 8371136 [details] [review]
Pull Request #16010 - Localize WiFi hotspot security type string

Thanks for the patch, Mihai!

The root cause of this issue is that we used `data-name="tethering.wifi.security.type"` and there is a very bad trick here. The trick is, we check whether the key specified with the data-name attribute is associated with a selector. If yes, we show the localization result instead of the value of the key. Please see [1] for the code. However, the selector of wifi security only exist after users navigate to the hotspot settings panel, that's why it fails.

I would suggest to remove the data-name attribute and use SettingsListener on "tethering.wifi.security.type" in hotspot.js, which is less magic and easier to trace. 

[1]: https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/js/settings.js#L509
Attachment #8371136 - Flags: review?(arthur.chen)
Comment on attachment 8371136 [details] [review]
Pull Request #16010 - Localize WiFi hotspot security type string

Thanks for the feedback, Arthur! Updated the PR with your suggestion, let me know if it looks good.
Attachment #8371136 - Flags: review?(arthur.chen)
Comment on attachment 8371136 [details] [review]
Pull Request #16010 - Localize WiFi hotspot security type string

r=me. Thanks!
Attachment #8371136 - Flags: review?(arthur.chen) → review+
Thanks Arthur! Landed on master:
https://github.com/mozilla-b2g/gaia/commit/3f195048fb801620c9c294d3062ee9c9e7eacdba
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
I had to revert this in https://github.com/mozilla-b2g/gaia/commit/0358dbe1b4b13a186f87d1fc5c8a8a29ab898a92 because it broke gaia unit tests on b2g-inbound: https://tbpl.mozilla.org/php/getParsedLog.php?id=34522345&tree=B2g-Inbound
Status: RESOLVED → REOPENED
Flags: needinfo?(mihai)
Resolution: FIXED → ---
That is weird, Travis was all green (https://travis-ci.org/mozilla-b2g/gaia/builds/18687931). Arthur, do you have any idea why the test failed on b2g-inbound?
Flags: needinfo?(mihai) → needinfo?(arthur.chen)
Not sure how it got green in the first place. We should add the mock of SettingsListener in hotspot_test.js.
Flags: needinfo?(arthur.chen)
Created attachment 8375172 [details] [review]
Pull Request #16232 - Localize WiFi hotspot security type string

Thanks Arthur, I updated the initial patch to use the mock for SettingsListener in the test, thus avoiding the failing unit test. Let me know if it is good.
Attachment #8371136 - Attachment is obsolete: true
Attachment #8375172 - Flags: review?(arthur.chen)
Comment on attachment 8375172 [details] [review]
Pull Request #16232 - Localize WiFi hotspot security type string

r=me. Thank you.
Attachment #8375172 - Flags: review?(arthur.chen) → review+
Landed on master:
https://github.com/mozilla-b2g/gaia/commit/55c4094e5d5a44420b742e0959e961459dc6b1c4
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.