1021500 - [Keyboard] Keyboard for Email shouldn't have '_' in the second row

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Keyboard, defect, P1)

Description

[Omega Feng [:Omega] [:馮於懋]]

Device: Flame
Gaia: 2014-06-05 23:34:17
BuildID: 20140602160205 
Version: 2.0.0.0-prerelease

STR:
1) Tap any "email" input to show keyboard

Actual:
There is '_' key in the second row

Expected:
There shouldn't be '_' key in the second row

Comment 1

[Jan Jongboom [:janjongboom] (inactive)]

Based on? Is there a UI document that says this should be removed?

Comment 2

[Omega Feng [:Omega] [:馮於懋]]

(In reply to Jan Jongboom [:janjongboom] (Telenor) from comment #1)
> Based on? Is there a UI document that says this should be removed?

Hi Jan,
There is no UI spec that says '_' should be there, so we should remove it to keep the layouts (normal text and email) consistent.

Comment 3

[Raniere Silva]

Attachment: patch-remove-extra-keys-from-second-row

Comment 4

[Omega Feng [:Omega] [:馮於懋]]

Comment on attachment 8435773 [details] [review]
patch-remove-extra-keys-from-second-row

Good job! Thanks!

Comment 5

[Jan Jongboom [:janjongboom] (inactive)]

Comment on attachment 8435773 [details] [review]
patch-remove-extra-keys-from-second-row

Clearing review for now. This only solves it for English but all other layouts have the same configuration. We should fix it for all layouts.

Comment 6

[Raniere Silva]

Some questions, although they are very similar.

1. For Spanish, the second row for email and url should have "ñ"?

2. For Catalan, Croatian, Hungarian and Serbian, the second row for email and url should have "'"?

3. For Portuguese, the second row for email and url should have "ç"?

Note: The patch was update by preserving this keys at email and url input type.

Comment 7

[Omega Feng [:Omega] [:馮於懋]]

(In reply to Raniere Silva from comment #6)
> Some questions, although they are very similar.
> 1. For Spanish, the second row for email and url should have "ñ"?
> 2. For Catalan, Croatian, Hungarian and Serbian, the second row for email
> and url should have "'"?
> 3. For Portuguese, the second row for email and url should have "ç"?

Yes to all three questions. "ñ", "'" and "ç" are valid (or limited supported) in URL and email address, so we should keep them.

Comment 8

[Raniere Silva]

Hi Jan, could you review the patch again? It should fix this issue for all layouts now.

Comment 9

[Jan Jongboom [:janjongboom] (inactive)]

Comment on attachment 8435773 [details] [review]
patch-remove-extra-keys-from-second-row

r=me. Looks good. Thanks!

Comment 10

[Raniere Silva]

Jan, could you merge this?

Comment 12

[Jan Jongboom [:janjongboom] (inactive)]

Landed https://github.com/mozilla-b2g/gaia/commit/78fd21bd35a9b466a40b1988e367784fd7638d66

Comment 13

[Omega Feng [:Omega] [:馮於懋]]

It's a major issue in new feature. Nominating as 2.0 blocking.
A consistent layout is really important for a large number of users.

Comment 14

[Stephany Wilkes]

I discussed this bug (and others like it) with Jason in IRC today. This looks like a 2.0 feature that was not correctly or fully implemented to spec, and should not ship as-is as a result (per Omega's comment #13). To that end, I am removing the blocking flag but adding the feature-b2g flag for 2.0, which it looks like this bug should have had as it was on the work list for 2.0. If this is not correct, let me know and we can discuss making it a blocking bug if it was not indeed a feature. 

I am not sure why the attachment has ui-review+ if the UI is not correct. When there is a major issue like this with a patch, the patch should get ui-review- and only get a ui-review+ when the submitted patch is 100% correct and ready to ship. 

Omega, please update the ui-review flags based on your understanding of this issue. Thank you!

Omega and Carol, please work on the necessary ui-review flag issues here. Thanks!

Comment 15

[Omega Feng [:Omega] [:馮於懋]]

Hi Steph,
Sorry for misleading. (Maybe I used the wrong flagging I guess.)
The patch is ui-review+. Since it's already done and ui-review+/review+, I hope it can be landed on 2.0 if there is low risk.

Comment 16

[Stephany Wilkes]

No problem, Omega! I just want to be sure everything is actually OK to ship. 

Rudy, since this is OK to land (if it is), can you mark this approval?/+ or should it just ride the train to release?

Comment 17

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

For me, I would prefer to make this change into the next release, though the patch here may look like a low-risk one.
This is because we modify some of the layouts but not all of them, so I would like to see more testers/users, especially l10n communities, take a look at these changes.
I think this would take some time and if we make this change into v2.0 in a rush, I'm afraid there will no time to make any changes if there is any potential issue.

Please let me know if you don't agree.
Thanks.

Comment 18

[Stephany Wilkes]

Rudy, that sounds like a good risk assessment. Omega, what will the keyboard look like if it ships? Will it be acceptable or no? If it will be a poor experience, is there something else we could do? Please advise and thank you!

Comment 19

[Omega Feng [:Omega] [:馮於懋]]

It's fine not to land on 2.0.
I'll review all other related layouts and write them down in the UX spec, so that we can have a reference for the next release (2.1).

Comment 20

[howie [:howie]]

This is not committed in 2.0, removing feature-b2g tag.

Comment 21

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

This has landed, see Comment 12.

Comment 22

[Sue]

Could you provide a video for me to verify this bug? Thanks.

Comment 23

[Sue]

Attachment: verify_video.MP4

This issue has been verified successfully on Flame v2.1
STR:
1. Launch E-Mail app.
2. New a mail.
3. Tap any email input box to show keyboard.
**There is no '_' key in the second row.
See attachment: verify_video.MP4
Reproducing rate: 0/5
Flame 2.1 versions:
Gaia-Rev        dbaf3e31c9ba9c3436e074381744f2971e15c7bf
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/ebce587d2194
Build-ID        20141203001205
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141203.034907
FW-Date         Wed Dec  3 03:49:18 EST 2014
Bootloader      L1TC00011880

Comment 24

[Omega Feng [:Omega] [:馮於懋]]

Sue, thanks for your verification!