1046798 - Predictive text doesn't handle caps lock correctly

Status: RESOLVED FIXED

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

Description

[Eric Rescorla (:ekr)]

STR:

0. Go to mail app
1. Put the phone in caps lock.
2. Type a regular word in (e.g., BRAIN)
3. Watch it autocorrect to regular case (Brain)


Expected behavior:
If you are in caps lock, words should end up as all upper case.

Comment 1

[Jan Jongboom [:janjongboom] (inactive)]

Attachment: Patch

Only downside I see is that if you have caps locked on the keyboard, and type one char, you get lcase suggestions, but it never autocorrects anyway, so just a minor thing. We don't keep this state in latin.js at the moment.

Comment 2

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

Comment on attachment 8466115 [details] [review]
Patch

I think David would be a better reviewer for this part of code.

David, 
Could you help on this?
Please re-direct to me if you're not available to review it.
Thanks.

Comment 3

[David Flanagan [:djf]]

Comment on attachment 8466115 [details] [review]
Patch

The patch seems technically fine, but I don't think it should land without ui-review.  The reporter wrote: "Expected behavior: If you are in caps lock, words should end up as all upper case". I'm not entirely sure I agree.

This issue came up before and someone submitted a patch that would have matched the case of the suggestions exactly to the case of the suggestions so that "UpperCasd" would auto-correct to "UpperCase".  I rejected that, but I thought that we decided in that bug (I don't recall the number) to do something about the all uppercase case.

I tried this out on Android and I found something interesting.  If I type "HELLP" instead of HELLO, it suggests HELP an HELLO, but does not autocorrect.  It assumes, I guess, that in all caps maybe I'm typing an acronym that it does not know about. I suspect that this might be a good rule for us to follow as well, and I think it would be an easy addition to this patch.

Surprisingly, on Android if I then type backspace (to convert HELLP to HELL) it starts suggesting words in lowercase. But maybe that is just an Android bug.

Comment 4

[Eric Rescorla (:ekr)]

(In reply to David Flanagan [:djf] from comment #3)
> Comment on attachment 8466115 [details] [review]
> Patch
> 
> The patch seems technically fine, but I don't think it should land without
> ui-review.  The reporter wrote: "Expected behavior: If you are in caps lock,
> words should end up as all upper case". I'm not entirely sure I agree.
> 
> This issue came up before and someone submitted a patch that would have
> matched the case of the suggestions exactly to the case of the suggestions
> so that "UpperCasd" would auto-correct to "UpperCase".  I rejected that, but
> I thought that we decided in that bug (I don't recall the number) to do
> something about the all uppercase case.
> 
> I tried this out on Android and I found something interesting.  If I type
> "HELLP" instead of HELLO, it suggests HELP an HELLO, but does not
> autocorrect.  It assumes, I guess, that in all caps maybe I'm typing an
> acronym that it does not know about. I suspect that this might be a good
> rule for us to follow as well, and I think it would be an easy addition to
> this patch.

That would be satisfactory as well. Autocorrecting the case is not.


> Surprisingly, on Android if I then type backspace (to convert HELLP to HELL)
> it starts suggesting words in lowercase. But maybe that is just an Android
> bug.

Comment 5

[Jan Jongboom [:janjongboom] (inactive)]

Comment on attachment 8466115 [details] [review]
Patch

Changing ui-review flag because no reply in a week.

Comment 6

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

Comment on attachment 8466115 [details] [review]
Patch

I think this patch works fine. It helps a lot on typing an all-uppercase sentence, but has a little bit inconvenience on typing a single unknown acronym (the user has to tap X to ignore autocorrection). The pros look more than cons, so I decide to give it a ui-review+ here.

Comment 7

[Jan Jongboom [:janjongboom] (inactive)]

Landed: https://github.com/mozilla-b2g/gaia/commit/494dc6f6aa589b2dd676f93a63220187b58abeb4

If after using this patch we happen to think that we should only suggest and not autocorrect, let's open a follow up.