Provide a link to Langpacks from Settings

VERIFIED FIXED in Firefox OS v2.2

Status

Firefox OS
Gaia::Settings
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: gandalf, Assigned: stas)

Tracking

unspecified
2.2 S4 (23jan)
Dependency tree / graph

Firefox Tracking Flags

(b2g-v2.2 verified, b2g-master verified)

Details

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
Settings should provide a link to Marketplace to install additional languages.
(Reporter)

Updated

3 years ago
Blocks: 1107341
(Assignee)

Updated

3 years ago
Blocks: 1107346
(Assignee)

Updated

3 years ago
No longer blocks: 1107341
(Assignee)

Comment 1

3 years ago
Created attachment 8546556 [details] [review]
Pull request

Arthur, I'd like to add a Marionette test but I'm not sure how to verify that tapping on the link resulted in the Marketplace app being launched.  Any pointers?
Assignee: nobody → stas
Status: NEW → ASSIGNED
Attachment #8546556 - Flags: feedback?(arthur.chen)
(Assignee)

Comment 2

3 years ago
Comment on attachment 8546556 [details] [review]
Pull request

Wil, in https://github.com/stasm/gaia/commit/2ed38e1871ef9267b3bc3f99c2625cfebe6e8ea0 I changed the activity to marketplace-langpacks.  Will this work with the changes you're planning to the Marketplace?
Attachment #8546556 - Flags: feedback?(wclouser)
Attachment #8546556 - Flags: feedback?(wclouser) → feedback+
(Assignee)

Comment 3

3 years ago
We might want to only show this link on phones for now and hide it on other device types.  The final decision hasn't been made yet but I'd like to start thinking about it.  Arthur, do you know if there's a way to get the current device type in the Settings app?  Or is it a buildtime option exclusively?
Flags: needinfo?(arthur.chen)
(Assignee)

Comment 4

3 years ago
Comment on attachment 8546556 [details] [review]
Pull request

EJ, can you take a look at this patch in Arthur's stead?  I added a unit test but I'm not sure if I follow good practices wrt. configuring MockSettingsCache.

Also, any thoughts on my question in comment 3?  Thanks!
Attachment #8546556 - Flags: feedback?(arthur.chen) → feedback?(ejchen)
Comment on attachment 8546556 [details] [review]
Pull request

Sorry for the late response. I was on PTO for the past two weeks. The patch looks good to me!
Flags: needinfo?(arthur.chen)
Attachment #8546556 - Flags: feedback?(ejchen) → feedback+
What kind of device type are you referring to? Like phone, tv, tablet?
Flags: needinfo?(stas)
Hey stats, I left some comments on Github for code part, and because Arthur is back, he will try to go through the other parts in the future. (Feel free to ni? me if you guys need any help !)

And also, I think we need some UX's helps here for visual part. 

Thanks !!
(Assignee)

Comment 8

3 years ago
(In reply to Arthur Chen [:arthurcc] from comment #6)
> What kind of device type are you referring to? Like phone, tv, tablet?

Yes, exactly.

(In reply to EJ Chen [:eragonj][:小龍哥][ni? if you need me] from comment #7)
> Hey stats, I left some comments on Github for code part, and because Arthur
> is back, he will try to go through the other parts in the future. (Feel free
> to ni? me if you guys need any help !)

Thanks, EJ, great comments in the PR.  I'll update the code and will ask Arthur to review again.

> And also, I think we need some UX's helps here for visual part. 

The UX was done in bug 1107346, attachment 8537621 [details].
Flags: needinfo?(stas)
(Assignee)

Comment 9

3 years ago
Comment on attachment 8546556 [details] [review]
Pull request

Arthur, can you take another look at this now that I applied EJ's comments?  Thanks!

The latest try build is:  https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=e0608a34e768
Attachment #8546556 - Flags: review?(arthur.chen)
(In reply to Staś Małolepszy :stas from comment #8)
> (In reply to Arthur Chen [:arthurcc] from comment #6)
> > What kind of device type are you referring to? Like phone, tv, tablet?
> 
> Yes, exactly.
> 
You could set the "data-device-type" attribute to elements. Only elements with the value of "data-device-type" exactly the same as the specified GAIA_DEVICE_TYPE exist in the DOM tree, or they are removed in the build process.

Are you going to implement it in this patch? If so, please also handle the case that the show more language item is not available, thanks!
Flags: needinfo?(stas)
Comment on attachment 8546556 [details] [review]
Pull request

The code looks great. Cancel the review request first per comment 10.
Attachment #8546556 - Flags: review?(arthur.chen)
(Assignee)

Comment 12

3 years ago
(In reply to Arthur Chen [:arthurcc] from comment #10)
> You could set the "data-device-type" attribute to elements. Only elements
> with the value of "data-device-type" exactly the same as the specified
> GAIA_DEVICE_TYPE exist in the DOM tree, or they are removed in the build
> process.

This is perfect!  Thanks for the pointer.

> Are you going to implement it in this patch? If so, please also handle the
> case that the show more language item is not available, thanks!

Yes, I'm updating the PR right now.
Flags: needinfo?(stas)
(Assignee)

Comment 13

3 years ago
Comment on attachment 8546556 [details] [review]
Pull request

Arthur, could you take another look?  I used data-device-type="phone" as you suggested.
Attachment #8546556 - Flags: review?(arthur.chen)
Sorry, I was wrong about data-device-type.

For this time being:
1. It only supports link element for this time being (we use it to remove non-required styles).
2. It does not support elements in a template.

This requires changes of build script (webapp-optimize.js). Wondering if you would like to implement it in this bug or a follow-up bug?
Flags: needinfo?(stas)
(Assignee)

Updated

3 years ago
Blocks: 1124098
(Assignee)

Comment 15

3 years ago
Arthur, thanks for providing more information about data-device-type.  I filed bug 1124098 as a follow-up to deal with that.  I'll update the PR to show the link on all devices for now.
Flags: needinfo?(stas)
(Assignee)

Comment 16

3 years ago
Arthur, this is now ready for you review.  The latest try build is:

https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=b815f44903d6
Comment on attachment 8546556 [details] [review]
Pull request

r=me, thanks!
Attachment #8546556 - Flags: review?(arthur.chen) → review+
(Assignee)

Comment 18

3 years ago
Master: https://github.com/mozilla-b2g/gaia/commit/e9377e2d12c8ec01796043ea29b22962758cac35
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Assignee)

Comment 19

3 years ago
Comment on attachment 8546556 [details] [review]
Pull request

[Approval Request Comment]

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

[User impact] if declined:  users won't have a way to access the special Marketplace landing page which lists available langpacks

[Testing completed]: on the device;  unit test

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

[String changes made]: 1 new string, more-languages
Attachment #8546556 - Flags: approval-gaia-v2.2?(bbajaj)

Updated

3 years ago
Attachment #8546556 - Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+
v2.2: https://github.com/mozilla-b2g/gaia/commit/4fec73f3141d7c122b8f260ce1d2db4b0545953a
status-b2g-v2.2: --- → fixed
status-b2g-master: --- → fixed
Target Milestone: --- → 2.2 S4 (23jan)
Created attachment 8636460 [details]
Flame_V2.2_Verify1.3gp

This bug has been verified as "pass" on latest build of Flame 2.2/master & Nexus5 2.2/master.

STR.
1. Open settings.
2. Tap "Language" tab.
3. Tap "Get More Languages"
4. Tap "Marketplace".

Actually result: Settings provide a link to Marketplace to install additional languages.
Reproduce rate: 0/10
See attachment: Flame_V2.2_Verify1.3gp

Device: Flame v2.2 build (pass)
Build ID               20150720002503
Gaia Revision          e1e6317f17a840b19af9dbb25f5a771d8d9fa161
Gaia Date              2015-07-15 21:05:11
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/e2d1f1f55803
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150720.042247
Firmware Date          Mon Jul 20 04:22:58 EDT 2015
Bootloader             L1TC000118D0

Device: Flame master (pass)
Build ID               20150720010206
Gaia Revision          3fac3ed7b8c887351098ffc677769ddc36abb3d0
Gaia Date              2015-07-17 17:53:41
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/202e9233d130
Gecko Version          42.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150720.044158
Firmware Date          Mon Jul 20 04:42:10 EDT 2015
Bootloader             L1TC000118D0

Device: N5 v2.2 build (pass)
Build ID               20150720002503
Gaia Revision          e1e6317f17a840b19af9dbb25f5a771d8d9fa161
Gaia Date              2015-07-15 21:05:11
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/e2d1f1f55803
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150720.041924
Firmware Date          Mon Jul 20 04:19:43 EDT 2015
Bootloader             HHZ12f

Device: N5 master build (pass)
Build ID               20150720160206
Gaia Revision          4fe0507781f3ed56c8ae5e66dd9489165d1ff68e
Gaia Date              2015-07-20 16:09:12
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/d00e4167b482
Gecko Version          42.0a1
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150720.191854
Firmware Date          Mon Jul 20 19:19:13 EDT 2015
Bootloader             HHZ12f
Status: RESOLVED → VERIFIED
QA Whiteboard: [MGSEI-Triage+]
status-b2g-v2.2: fixed → verified
status-b2g-master: fixed → verified
You need to log in before you can comment on or make changes to this bug.