1126247 - Hide the Get More Languages link until Marketplace supports langpacks

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Settings, defect)

Description

[Staś Małolepszy :stas]

Bug 1115798 added the Get More Languages link and bug 1124098 made it only show up on phones.

However, until bug 1105530 is fixed and an update to the Marketplace app is deployed, we're missing a major part of the functionality, and the link looks broken.

Let's hide it entirely for now and enable it only once bug 1105530 is deployed.

Comment 1

[Staś Małolepszy :stas]

Attachment: Pull request

With this patch, the link is hidden unless the l10n.more.showOnAllDevices setting is set to true.  Combined with build/config/custom-settings.json, this can be used to enable the link for testing.

Once the Marketplace supports langpacks, I'll file another bug to re-add the link to settings_phone.css.  This will override the setting and always make the link show up on phones.  The setting will continue to control the link on other device types.

Arthur, does this sound fine? 

Also, I'm not sure what the guidelines for naming settings are.  Any pointers?  I'm not very happy with "l10n.more.showOnAllDevices".

Comment 2

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8556171 [details] [review]
Pull request

It makes less sense to introduce a new key as it is temporary and we still have to modify the code (adding the css back) to show the menu item.

How about simply remove the css in this patch?

Comment 3

[Staś Małolepszy :stas]

Sure, I can do that.  I was just wondering if there was a way to easily show the link for testing purposes.  The l10n and the marketplace team would benefit from this during the current dev cycle.

Can I comment the CSS out instead of removing it?

Comment 4

[Arthur Chen [:arthurcc][inactive after 6/30]]

Got it. I was not aware that you need to tweak the menu item for development. To me the settings key is used to pref off the language pack feature, so maybe we can call it "l10n.language-pack.enabled"?

Comment 5

[Arthur Chen [:arthurcc][inactive after 6/30]]

btw, please make sure this patch gets uplifted to v2.2.

Comment 6

[Staś Małolepszy :stas]

Comment on attachment 8556171 [details] [review]
Pull request

Arthur, I changed my mind on this:  adding a new setting is an overkill.  As per the IRC chat, the updated pull request uses data-device-type="none" to hide the link temporarily.  I tested on my Flame and it works fine.  Thanks!

Comment 7

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8556171 [details] [review]
Pull request

r=me, thanks!

Comment 8

[Staś Małolepszy :stas]

Master: https://github.com/mozilla-b2g/gaia/commit/5479269eb26c006124437c9973e4916aec69d17a

Comment 9

[Staś Małolepszy :stas]

Comment on attachment 8556171 [details] [review]
Pull request

[Approval Request Comment]

Requesting approval as per comment 5.

[Bug caused by] (feature/regressing bug #): langpacks

[User impact] if declined:  the Marketplace implementation of langpacks isn't ready yet and the Get More Languages link in Settings > Language looks broken because it doesn't do anything.

[Testing completed]:  on the device

[Risk to taking this patch] (and alternatives if risky):  low

[String changes made]: none

Comment 10

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/be528ed492362dbbcb802ffe7a65e03fcb263dfc