965211 - Show capital letters on keyboard when shift pressed

Status: RESOLVED FIXED

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

Description

[Dale Harvey (:daleharvey)]

Regressed it in https://bugzilla.mozilla.org/show_bug.cgi?id=860318, make it pretty impossible for me to type my keyboard, if we dont switch back the default at the least need an installable keyboard

Comment 1

[Dale Harvey (:daleharvey)]

s/make it pretty impossible for me to type my keyboard/make it pretty impossible for me to type my password

Comment 2

[Gabriele Svelto [:gsvelto] (PTO)]

+1

I only used older non-WP Nokia phones before Firefox OS and I find the ALLCAPS keyboard annoying and especially so when typing passwords. If providing an alternative keyboard still hits performance issues then let's fix those; I'm very willing to lend my profiling skills to this cause.

Comment 3

[Jan Jongboom [:janjongboom] (inactive)]

Invalid as per https://bugzilla.mozilla.org/show_bug.cgi?id=883340#c18

Comment 4

[Dale Harvey (:daleharvey)]

This isnt resolved, I dont have a keyboard which shows the correct case

If the default sticks with copying ios, thats fine, this bug will track the availability of a 3rd party keyboard that does it

Comment 5

[Jan Jongboom [:janjongboom] (inactive)]

Well, bug 883340 is still open and describes the exact same issue, so that's why I was closing it as a duplicate.

If you still think this bug should be kept open, please ping me on IRC.

Comment 6

[Dale Harvey (:daleharvey)]

883340 has been closed, as said "this bug will track the availability of a 3rd party keyboard that does it" which I still dont have, so its an open bug

Comment 7

[Gabriele Svelto [:gsvelto] (PTO)]

To add my 2p, while dogfooding on master I've noticed that when long-pressing keys to get accents for certain letters you get them in lowercase. This is doubly confusing as you have an uppercase keyboard, writing lowercase characters and with lowercase overlays when using accents. Ugh.

Comment 8

[Klaatu]

Really need lower-case and capital letters. Capitalization is bafflingly confusing without it.

Running 2.0 on a Flame.

Comment 9

[Gabriele Svelto [:gsvelto] (PTO)]

(In reply to Klaatu from comment #8)
> Really need lower-case and capital letters. Capitalization is bafflingly
> confusing without it.

Yep, it's still pretty bad though the specific issue I described in comment 7 has been fixed.

Comment 11

[Martin Creutziger [:MMx]]

Although this behaviour is intended as by the Keyboard UX Specification, attached to bug 983043, I also disagree with this spec. 
It is terribly annoying to type lowercase on an uppercase keyboard, especially if you are used to Android or SailfishOS devices, which both show the correct case.

So actually, a bug against the specification needs to be filed, I guess...

Comment 13

[Dale Harvey (:daleharvey)]

Hey, So this has been very consistently been brought up as an annoyance from user feedback, can we rethink this product decision? Cheers

Comment 14

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Redirect ni to UX owner.

Comment 15

[Harly Hsu[:harly]]

This issue has a long story, basically the All Capital keyboard design was due to better performance and more visual appealing. However, from an UX standpoint, I agree it will be much easier for user to differentiate if we have a keyboard that can display lower letter and capital letter keys. But I only agree to change the design if the performance problem isn't an issue anymore.

Comment 16

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Harly Hsu from comment #15)
> But I
> only agree to change the design if the performance problem isn't an issue
> anymore.

I guess that means people can do the patch and send you for ui-review?

Comment 17

[jezra]

(In reply to Harly Hsu from comment #15)
> the All Capital keyboard design was
> due to better performance and more visual appealing. 

What performance issue did changing to ALLCAPS resolve?  I have asked before but was never responded to. please see https://bugzilla.mozilla.org/show_bug.cgi?id=883340#c32

Comment 18

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Instead of putting this into my even longer backlog, I am setting this as a mentor bug for experienced Gaia contributor to work on. You would need a phone and have previous experience to flash Gaia. To fix this bug, identify the keyboard rendering code that involve this behavior. WebIDE will be helpful in terms of identifying the DOM.

Comment 19

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to jezra from comment #17)
> What performance issue did changing to ALLCAPS resolve?  I have asked before
> but was never responded to. please see
> https://bugzilla.mozilla.org/show_bug.cgi?id=883340#c32

My recollection is that fixing the layout to all caps helps the visual performance, since that reduce the area of repaint needed when switching cases from the entire layout to just the shift button.

Comment 20

[Harly Hsu[:harly]]

(In reply to Tim Guan-tin Chien [:timdream] (slow response; please ni? to queue) from comment #16)
> (In reply to Harly Hsu from comment #15)
> > But I
> > only agree to change the design if the performance problem isn't an issue
> > anymore.
> 
> I guess that means people can do the patch and send you for ui-review?

Yes, please flag me for ui-review if there are any patches. Thanks :)

Comment 21

[jezra]

> My recollection is that fixing the layout to all caps helps the visual
> performance, since that reduce the area of repaint needed when switching
> cases from the entire layout to just the shift button.
At the time, I was a real world user of FirefoxOS on the original ZTE Open, and I can honestly say that on that low end hardware there was no noticeable difference from when the WYSIWYG keyboard was replaced by the iPhone style ALL CAPS keyboard. If there is any documentation about the performance issue, I would certainly like to read it.

Comment 22

[GitHub Autolander Bot]

Attachment: [gaia] nblumoe:fix/965211-capital-letters-default-for-keyboard > mozilla-b2g:master

Comment 23

[nils]

Attachment: Pull request to fix this bug

Comment 24

[Michael Kohler [:mkohler]]

Comment on attachment 8632554 [details] [review]
Pull request to fix this bug

Also flaging Harly for ui-review

Comment 25

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8632554 [details] [review]
Pull request to fix this bug

Stealing review. Thanks for contributing!

Comment 26

[Michael Kohler [:mkohler]]

Hi Tim

Thanks for the fast review! Can this be landed so it doesn't bit-rot?

Would you mind suggesting a "good next bug" for Nils in case he'd like to continue contributing to Gaia? (I hope Nils wants to :)

Cheers,
Michael

Comment 27

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Michael Kohler [:mkohler] from comment #26)
> Hi Tim
> 
> Thanks for the fast review! Can this be landed so it doesn't bit-rot?

There is no other Keyboard developers left except me; I will surely get this landed before other patches cause conflicts.

> Would you mind suggesting a "good next bug" for Nils in case he'd like to
> continue contributing to Gaia? (I hope Nils wants to :)

Maybe try http://www.joshmatthews.net/bugsahoy/?b2g=1 :D

A few DOM APIs for B2G are classified in Core::DOM and written in JS (A _lot_ of things in Gecko are written in JS), do look up these bugs too if you are interested.

Comment 28

[Harly Hsu[:harly]]

Comment on attachment 8632554 [details] [review]
Pull request to fix this bug

Looks good to me! Nice work :)

Comment 29

[Tim Guan-tin Chien [:timdream] (please needinfo)]

master: https://github.com/mozilla-b2g/gaia/commit/364df10a3c1852c01fc1674fb9b0594819d0992f

Thanks for the contribution!

Comment 30

[Gareth Aye [:gaye] (back from PTO)]

This broke  apps/settings/test/marionette/tests/sound_settings_test.js  (see https://s3-us-west-2.amazonaws.com/taskcluster-public-artifacts/iXkdzsPGR-ucIBA3eFpiVA/0/public/logs/live_backing.log). Going to backout for now.

Comment 31

[Tim Guan-tin Chien [:timdream] (please needinfo)]

I don't think they can be related; I can verify that locally and re-land if this is clean.

Comment 32

[Gareth Aye [:gaye] (back from PTO)]

Yeah nevermind :(. apps/settings/test/marionette/tests/sound_settings_test.js is still broken when I try reverting this patch. Ugh

Comment 33

[Gareth Aye [:gaye] (back from PTO)]

False alarm!

Comment 34

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Hi Nils,

What's your timezone and available hours for contribution? If you are interested, would like to talk to you over IRC and know which part of Gaia (and maybe FxOS) you would be interested in working on.

I do have some mentor bug tagged but they may be too trivial for you ...
https://bugzilla.mozilla.org/buglist.cgi?quicksearch=bug_mentor%3Atimdream

We could take this off list; shoot me an e-mail at timdream at mozilla dot com.