1155568 - [RTL][First Time Experience]The text "open" at select network view in FTE is overlapped by divider.

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::First Time Experience, defect, P2)

Description

[Sue]

Attachment: FTE_wifi_view.png

[1.Description]:
[RTL][Flame v2.2 & v3.0][N5 v2.2 & v3.0][First Time Experience]The bottom of "Open" is overlapped by divider at select network view in FTE.
See attachment:FTE_wifi_view.png

[2.Testing Steps]: 
Prerequisite: Have a open Wi-Fi.
1. Flash a device or reset device.
2. Set system language as Arabic at FTE.
3. Tap next button to select network view.
4. Observe the text "Open" under Wi-Fi name.

[3.Expected Result]: 
4. Nothing should overlap.

[4.Actual Result]: 
4. The bottom of "Open" is overlapped by divider.

[5.Reproduction build]: 
Device: Flame 2.2 (affected)
Build ID               20150416162504
Gaia Revision          d50b8a3919a7b4d8d289f150d3b9bed704ebafa9
Gaia Date              2015-04-16 21:46:57
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/5ebf32030512
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150416.195720
Firmware Date          Thu Apr 16 19:57:29 EDT 2015
Bootloader             L1TC000118D0

Device: Flame 3.0 (affected)
Build ID               20150416160206
Gaia Revision          3cd0a9facce26c2acc7be3755a17131a6358e33f
Gaia Date              2015-04-16 16:33:22
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/51e3cb11a258
Gecko Version          40.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150416.191700
Firmware Date          Thu Apr 16 19:17:10 EDT 2015
Bootloader             L1TC000118D0

Device: Nexus 5 2.2 (affected)
Build ID               20150416162504
Gaia Revision          d50b8a3919a7b4d8d289f150d3b9bed704ebafa9
Gaia Date              2015-04-16 21:46:57
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/5ebf32030512
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150416.201018
Firmware Date          Thu Apr 16 20:10:32 EDT 2015
Bootloader             HHZ12f

Device: Nexus 5 3.0 (affected)
Build ID               20150416160206
Gaia Revision          3cd0a9facce26c2acc7be3755a17131a6358e33f
Gaia Date              2015-04-16 16:33:22
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/51e3cb11a258
Gecko Version          40.0a1
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150416.192355
Firmware Date          Thu Apr 16 19:24:11 EDT 2015
Bootloader             HHZ12f

[6.Reproduction Frequency]: 
Always Recurrence,5/5

[7.TCID]: 
Free Test

Comment 1

[Delphine Lebédel [:delphine - Use needinfo]]

Since this is FTE, this is going to be the 1st impression people get when they use their phone for the 1st time... if it were in an other area, it might not have been so bad, but this is FTE
I'm therefore tempted to block on this. 
triage P2 -- nominating

Comment 2

[Josh Cheng [:josh]]

Hi Sam,
Could you please help to check the problem? Thanks!

Comment 3

[Sam Foster [:sfoster] (he/him)]

Yeah I can look into this. P2/blocker though? Either way should be a trivial patch

Comment 4

[Sam Foster [:sfoster] (he/him)]

Not a blocker IMO, will get a patch in though and we can uplift

Comment 5

[Delphine Lebédel [:delphine - Use needinfo]]

Yeah I wasn't too sure to nominate it or not. But given this is FTE and and will be the 1st impression a user will get (and he'll have to see this), I think it was better to get this on triage's team radar. Thanks for getting a fix in!

Comment 6

[GitHub Autolander Bot]

Attachment: [gaia] sfoster:ftu-wifi-line-height-bug-1155568 > mozilla-b2g:master

Comment 7

[Sam Foster [:sfoster] (he/him)]

I actually had trouble reproducing this - I tried a few different languages. However, I see in the code how it could come about. Attachment #8596265 [details] should fix the issue. Could you check it, and if possible narrow down the STR so I can also confirm? 

Also, note that this fix is specific to the wifi list in FTU app. If there are similar issues elsewhere in the FTE they will need to be filed and addressed separately.

Comment 8

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8593827 [details]
FTE_wifi_view.png

The PR tweaks the CSS a bit for the wifi list entries. Note we use very different markup here vs. Settings. I think the 2rem line-height was trying to give each entry a bit of breathing room, with variable results in different languages. The bottom padding should do a more consistent job of that.

Comment 9

[Delphine Lebédel [:delphine - Use needinfo]]

Taking out the ni off me and leaving QA wanted for QA to look at. thanks

Comment 10

[Pi Wei Cheng [:piwei] (inactive)]

This issue will occur on: Arabic + FTE + an open wifi network (one that doesn't require a password).

I've tried to test the patch but I couldn't figure out a way to add Arabic language into the device after applying the patch. After patch it always only gives me three languages (English, Accented English, and another non-standard one I forgot the name). So I'm afraid I can't help on testing the patch. Hopefully the criteria I mentioned on first paragraph helps you repro locally.

Comment 11

[Sam Foster [:sfoster] (he/him)]

> I've tried to test the patch but I couldn't figure out a way to add Arabic
> language into the device after applying the patch. 

You need to patch a build that has the Arabic (ar) locale. It may be easier to test once the patch gets review and I land on master - no rush until then. I'll try again myself meantime.

Comment 12

[Pi Wei Cheng [:piwei] (inactive)]

The build I tested does have all the languages in there PRIOR to applying the patch. I do git checkout of that patch, and then make reset-gaia. After that I lose all the languages, except the three I mentioned.

Comment 13

[Sam Foster [:sfoster] (he/him)]

:piwei, you can try:
 
$ GAIA_DEFAULT_LOCALE=ar make reset-gaia

But I think reset-gaia is replacing the locales from the build with the contents of your GAIA/locales directory - which is probably empty. You can follow the directions on https://developer.mozilla.org/en-US/Firefox_OS/Developing_Gaia/Localizing_Firefox_OS to populate your locales directory (which is what I did), but I think that's not worth your time. There must be a better way to apply the patch and preserve the existing locales in there, but I'm afraid I dont know it.

Comment 14

[Pi Wei Cheng [:piwei] (inactive)]

Thank you :sfoster. I will give that a try later. I want to learn how to do this in case we need to do it again for future reference, and I can pass this information on to our team if I make it work.

Comment 15

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8593827 [details]
FTE_wifi_view.png

Doh, wrong attachment :)

Comment 16

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8596265 [details] [review]
[gaia] sfoster:ftu-wifi-line-height-bug-1155568 > mozilla-b2g:master

See comment #8, whichever of you has a moment to look at this

Comment 17

[Fernando Campo (:fcampo)]

Comment on attachment 8596265 [details] [review]
[gaia] sfoster:ftu-wifi-line-height-bug-1155568 > mozilla-b2g:master

took me some time to test with the locales, but it looks good.

Comment 18

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8596265 [details] [review]
[gaia] sfoster:ftu-wifi-line-height-bug-1155568 > mozilla-b2g:master

Thanks :fcampo. Cancelling r? for Alberto

Comment 19

[GitHub Autolander Bot]

http://docs.taskcluster.net/tools/task-graph-inspector/#GpEJVTSFRbu0zjnocNRevw

The pull request failed to pass integration tests. It could not be landed, please try again.

Comment 20

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/4975c03cc2ae56cc4c24db6501a7be0fc1674804

Comment 21

[Sam Foster [:sfoster] (he/him)]

I would like to get this verified on master before requesting 2.2 uplift. We'll need to check Arabic and spot-check a few other languages with varying character heights, diacritics and leading. For example: ar, en-US, ko, ru, bn-BD, zh-TW.

Comment 22

[Pi Wei Cheng [:piwei] (inactive)]

Today's 3.0 nightly doesn't have the fix. I'll try to verify this tomorrow.

Comment 23

[Yeojin Chung [:YeojinC]]

Attachment: NotFixedMaster2015-04-30-15-25-44.png

This bug is NOT fixed on today's Flame Master (nightly, user build). The "Open" string is cut off at the bottom. I checked the locales mentioned on comment 21, and it seems that only Arabic has the issue.

Environmental Variables:
Device: Flame 3.0 (KK, 319mb, full flash)
Build ID: 20150430010201
Gaia: db8ea705c0fd1b1684807f5a8e837bb9a36a6f96
Gecko: 4b9b12c248dc
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 40.0a1 (Master)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Comment 24

[Sue]

Hi Josh,
According to comment 23, this issue was verified fail, could you help with it? Thanks!

Comment 25

[Josh Cheng [:josh]]

Hi Sam,
The issue seems still exist per comment 23. Could you please have a check?
Thanks!

Comment 26

[Sam Foster [:sfoster] (he/him)]

(In reply to Josh Cheng [:josh] from comment #25)
> Hi Sam,
> The issue seems still exist per comment 23. Could you please have a check?

Yeah this is on my list for this week.

Comment 27

[Sam Foster [:sfoster] (he/him)]

Attachment: ftu-wifi-dividers-ar.png

Gaia-Rev        f13e3c923381ddca055b56d66360781964b858a8
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/946ac85af8f4
Build-ID        20150422010202
Version         40.0a1
Device-Name     flame

This looks good to me on master. The relevant CSS rule from the patch is line-height: 1.33; when I remove this property I get something that looks more like attachment #8600031 [details] Is it possible you tested without the patch? Otherwise we need to figure out what other factors are in play here as I'm not able to reproduce on master currently

Comment 28

[Sam Foster [:sfoster] (he/him)]

I just flashed gaia db8ea705c0fd1b1684807f5a8e837bb9a36a6f96, and it still works-for-me. Maybe something in gecko since then made this stick? Can we re-test on master?

Comment 29

[Yeojin Chung [:YeojinC]]

This issue is fixed on Flame Master.

Result: The "Open" string is displayed properly.

Environmental Variables:
Device: Flame 3.0 (KK, 319mb, full flash)
Build ID: 20150508010203
Gaia: bc5bfa18f795919b56b952bbf3637c235d0e13dc
Gecko: 356e735fa908
Gonk: a9f3f8fb8b0844724de32426b7bcc4e6dc4fa2ed
Version: 40.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

========================
Adding verifyme for 2.2 uplift/verification

Comment 30

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8596265 [details] [review]
[gaia] sfoster:ftu-wifi-line-height-bug-1155568 > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Wifi network list in FTU (Arabic)
[User impact] if declined: In some languages, the lower part of the names of Wifi networks may be obscured in FTU; names may be ambiguous and potentially unreadble 
[Testing completed]: Tested on device in various languages including Arabic. Verified by QA on master
[Risk to taking this patch] (and alternatives if risky): Very low - CSS-only patch with minor tweaks to restore proper handling of taller characters
[String changes made]: None

Comment 31

[Sam Foster [:sfoster] (he/him)]

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

Comment 32

[Sue]

Attachment: verify_v2.2_pass.png

This issue has been verified as pass on latest nightly build of Flame 2.2 and Nexus 5 2.2 by STRs in comment 0.
Result: The "Open" string is displayed properly.
See attachment:verify_v2.2_pass.png
Rate:0/3

Device: Flame 2.2 (pass)
Build ID               20150519162501
Gaia Revision          63e9eeec3032318f8a240f80b6a184fa4b50b6e1
Gaia Date              2015-05-19 17:52:15
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/4e078e1364d3
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150519.200807
Firmware Date          Tue May 19 20:08:18 EDT 2015
Bootloader             L1TC000118D0

Device: Nexus 5 2.2 (pass)
Build ID               20150519162501
Gaia Revision          63e9eeec3032318f8a240f80b6a184fa4b50b6e1
Gaia Date              2015-05-19 17:52:15
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/4e078e1364d3
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150519.195445
Firmware Date          Tue May 19 19:55:01 EDT 2015
Bootloader             HHZ12f