910702 - [Keyboard] Inserting a space in the middle of a string that will disable the uppercase function

Status: VERIFIED DUPLICATE of bug 1031561

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

Description

[William Hsu [:whsu]]

* Description:
  This is a legacy problem.
  If you enable uppercase function and input a string (like "AAABBBCCC") then insert a space in middle of the string, the uppercase function will be disabled.

* Reproduction steps:
  1. Launch the contact app
  2. Tap "↑" twice
  3. Tap the "Name" field
  4. Input "ABCDEF"
  5. Insert a space in the middle of the string.("ABC DEF")

* Expected result:
  The uppercase function is still enabled

* Actual result:
  The uppercase function is disabled

* Reproduction build:(Mozilla-Central)
  + Mercurial-Information
    - Gecko revision="416075f77249"
  + Git-information
    - Gaia revision="94c3dde50cb8da6e9d9ed137bfbf895f8699f8e7"(From Rudy's repository)
  + Gecko version: 26.0a1


Thanks!

Comment 1

[Ivan Tsay (:ITsay)]

Functional issue. it is blocker for koi.

Comment 2

[Luke Chang [:lchang] (inactive)]

This issue results from the "UpperCaseLocked" state resetting when the cursor position is changed. Since the same behavior appears in v1-train branch also, I'm not sure if it is under UX spec.

After discussing with Rudy, I think we need more information from UX team.

Comment 3

[Carrie Wang [New account: [e-mail]]]

Hi guys,

If Uppercase key is locked, it means the keyboard is in "All Uppercase" mode. Thus, the uppercase function should be enabled until users tap the key again to unlock it.
Thanks!

(In reply to Luke Chang [:lchang] from comment #2)
> This issue results from the "UpperCaseLocked" state resetting when the
> cursor position is changed. Since the same behavior appears in v1-train
> branch also, I'm not sure if it is under UX spec.
> 
> After discussing with Rudy, I think we need more information from UX team.

Comment 4

[Ivan Tsay (:ITsay)]

Minor issue and move it to 1.3

Comment 5

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

Attachment: Pull Request #13102 - Don't reset locked uppercase

This seems to be quite a quick fix that involves not resetting the upperCaseLocked state when changing the position of the cursor.

Comment 6

[Jan Jongboom [:janjongboom] (inactive)]

Comment on attachment 822546 [details]
Pull Request #13102 - Don't reset locked uppercase

Stealing review

Comment 7

[Jan Jongboom [:janjongboom] (inactive)]

Comment on attachment 822546 [details]
Pull Request #13102 - Don't reset locked uppercase

Hi mihai, still something flaky going on. Following situation.

* This is in SMS app, French keyboard.
* Put uppercase lock on.
* Type: QSD FDS JG. J
* Disable upper case lock [1]
* Type: an is tof
* Move cursor to FD|S (position 6)
* Upper case is activated

So we still fiddle with this on cursor movement. Think we should completely ignore it on cursor movement (this is what Android 4.3 does).

[1]: It goes from locked -> uppercase, should go to lowercase. Probably out of scope for this issue. But still.

Comment 8

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

Minor issue, push to backlog.

Please do ask for a+ if you think the patch is safe enough to land on the production branch.

Comment 9

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

(In reply to Jan Jongboom [:janjongboom] (PTO until Nov 5) from comment #7)
> Comment on attachment 822546 [details]
> Pull Request #13102 - Don't reset locked uppercase
> 
> Hi mihai, still something flaky going on. Following situation.
> 
> * This is in SMS app, French keyboard.
> * Put uppercase lock on.
> * Type: QSD FDS JG. J
> * Disable upper case lock [1]
> * Type: an is tof
> * Move cursor to FD|S (position 6)
> * Upper case is activated
> 
> So we still fiddle with this on cursor movement. Think we should completely
> ignore it on cursor movement (this is what Android 4.3 does).
> 
> [1]: It goes from locked -> uppercase, should go to lowercase. Probably out
> of scope for this issue. But still.

Hi Jan, investigated this behavior further and found the following code in the latin IME: https://github.com/mozilla-b2g/gaia/blob/master/apps/keyboard/js/imes/latin/latin.js#L832-L863, which changes the upperCase state based on the cursor position.

Given that there is a comment explaining the logic, it might be the case that the context-aware uppercase switching is intentional (i.e. by design). In any case, adding David in the loop to provide some context on this, since it appears he wrote that code.

Comment 10

[David Flanagan [:djf]]

That behavior was intentional: if you position the cursor after a string of more than one uppercase letter, I assume you are typing an acronym and want to continue in uppercase.  I can't think of a case (in English, at least, and assuming that you are not programming) where you would want to type two or three uppercase letters and then follow with lowercase letters.

I suppose that if the user has explicitly double-tapped the shift key to lock caps, then we should not do any auto-capitalization until they user taps the shift key again.  But if caps is not locked, then we should leave auto-capitalization as it is.  Does that sound right?

IIRC, the logic in that area of the code is confusing and it could stand to be strengthened and refactored.

Comment 11

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

The behavior of 2.2 keyboard app works as expected in comment 0. We should close this bug as fixed.

That said, I am not sure the behavior change is intentional or not based on the above comment from :djf. The behavior with STR in comment 7 is changed too.

Jan, could you find the dup? :djf, have you changed your position on the behavior since? Just to make sure.

Comment 12

[Jan Jongboom [:janjongboom] (inactive)]

> Jan, could you find the dup?

I assume it is because we fixed selection handling, which we never had in place properly. But if you need specific bug number I can bisect?

Comment 13

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

(In reply to Jan Jongboom [:janjongboom] (Telenor) from comment #12)
> > Jan, could you find the dup?
> 
> I assume it is because we fixed selection handling, which we never had in
> place properly. But if you need specific bug number I can bisect?

Never mind, we could bisect if we really need to. Thanks!

Comment 14

[David Flanagan [:djf]]

Tim,

I suspect this was resolved by bug 1031561 which removed the auto-capitalization when you put the cursor next to two or more capital letters.

Comment 15

[William Hsu [:whsu]]

Thanks David and Tim.
You got that right.
After Bug 1031561 is fixed, this bug cannot be reproduced.

* Build information:
 - Gaia      94638b51a5a5d513e26247e9b207aa54e7bb0568
 - Gecko     https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/bc9e3fa37e63
 - BuildID   20141008160201
 - Version   32.0