869879 - [Settings] No cursor Display in Simpin input textfield on focus

Status: RESOLVED FIXED

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

Description

[Leo]

1. Title : No cursor Display in Simpin input textfield on focus .
2. Precondition : 
3. Tester's Action : Settings >> Sim Security >> Change SimPin >> fill text fields of Simpin, create new Simpin                       
4. Detailed Symptom (ENG.) : There is no cursor to display in which textfield box,the number is getting typed.
5. GAIA version : 0d2b50d1c7083e5c19d7f0d0eebc019b7a7e8374
6. Expected : A cusor should be displayed in the textfield in which the number is getting typed
7.Reproducibility: Y
1)Frequency Rate : 100%
8.Comparison Results : 
1)Model Comparing : 
9. Attached files: 
1)Log : 
2)Test Contents : 
3)Video file :

Comment 1

[Leo]

Attachment: screenshot of the view

The current field will not be focused and creates confusion to user in finding the current field.

Comment 2

[Leo]

The action on focus should be to display cursor.
Can you give some information about whether cusor is not dispaying due to
SIM Pin customized text fields or some other issue.
please give your comment.

Comment 3

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

Hi,

I guess this is due to our little trick to deal number+password input field case,

e.g. from the following section
 <div class="input-wrapper">
  <input type="number" x-inputmode="digits" name="simpin" size="8" maxlength="8" />
  <input type="text" name="simpinVis" size="8" maxlength="8" />
</div>

the shown input is type="text" while the focused input is type="number".
Since the focused one (with the cursor) is hidden, you could not see the cursor either.

Thanks.

Comment 4

[hyuna.cho]

(In reply to Rudy Lu [:rudyl] from comment #3)
> Hi,
> 
> I guess this is due to our little trick to deal number+password input field
> case,
> 
> e.g. from the following section
>  <div class="input-wrapper">
>   <input type="number" x-inputmode="digits" name="simpin" size="8"
> maxlength="8" />
>   <input type="text" name="simpinVis" size="8" maxlength="8" />
> </div>
> 
> the shown input is type="text" while the focused input is type="number".
> Since the focused one (with the cursor) is hidden, you could not see the
> cursor either.
> 
> Thanks.

Do you have any idea to fix this issue?
Need to show the cursor for user

Comment 5

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

Will take a look to try if we could resolve this with "x-inputmode".

Comment 6

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

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9859/files

Pointer to Github pull-request

Comment 7

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

Comment on attachment 751344 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9859/files

This is the 1st part of the patch to enable the keyboard app to support x-inputmode="digits" for type="password" and type="text", etc.

Dear David,

Could you please help take a look?
Thank you.

Comment 8

[David Flanagan [:djf]]

Rudy,

This is the first time I've looked at the code for x-inputmode=digits. I didn't know we had that...  I've made some comments on github, but am not quite ready to do a full review yet. Overall the code looks fine, but I have some questions before I can understand it well enough to review carefully:

1) What is the code in render.js for?  What problem is it solving? It seems less directly related to bug description than the changes in keyboard.js

2) I don't like that we support the non-standard input mode "digits" without supporting the "numeric" mode that is actually in the HTML5 spec.  Should we consider supporting 'numeric' in addition to 'digits'?

3) We we ever use inputmode='digits' for anything other than PINs?  Are they always type=password?  If so, then maybe we could convert inputmode=digits to inputmode=numeric, to be closer to the standard, but add a special case where type=password inputmode=numeric does digits only without the decimal point, minus sign, etc. 

I don't know what we do on Android, so I suppose we should be compatible with that. And I'm not interested in holding up this Leo+ bug for a standards discussion.  But I do want to make sure that we're not gratiuitously deviating from standards here.

Comment 9

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

Sorry for the late reply.

(In reply to David Flanagan [:djf] from comment #8)
> Rudy,
> 
> This is the first time I've looked at the code for x-inputmode=digits. I
> didn't know we had that...  I've made some comments on github, but am not
> quite ready to do a full review yet. Overall the code looks fine, but I have
> some questions before I can understand it well enough to review carefully:
> 
> 1) What is the code in render.js for?  What problem is it solving? It seems
> less directly related to bug description than the changes in keyboard.js

Originally, it depended on inputType to see we would need to apply "special-key", "big-key" class name to the keys in "pinLayout" and "numberLayout".
Right now, there are more cases, say inputmode="digits" and type="password", that we need to apply that special class names.

> 
> 2) I don't like that we support the non-standard input mode "digits" without
> supporting the "numeric" mode that is actually in the HTML5 spec.  Should we
> consider supporting 'numeric' in addition to 'digits'?
>

Yeah, I can manage to support "numeric" mode, too. Please stay tuned. 

> 3) We we ever use inputmode='digits' for anything other than PINs?  Are they
> always type=password?  If so, then maybe we could convert inputmode=digits
> to inputmode=numeric, to be closer to the standard, but add a special case
> where type=password inputmode=numeric does digits only without the decimal
> point, minus sign, etc. 
> 

So far, yes, that is for PIN code only.
I don't think we could use "numeric" here, since minus/other symbols are not allowed for PIN code, which is why "digits" mode was introduced to solve this particular requirement.

> I don't know what we do on Android, so I suppose we should be compatible
> with that. And I'm not interested in holding up this Leo+ bug for a
> standards discussion.  But I do want to make sure that we're not
> gratiuitously deviating from standards here.

I have checked Firefox on Android (Fennec) and I didn't see inputmode or x-inputmode got supported.
Since it is still "x"-inputmode here, so hopefully, we could refine what mode we need to support and the behavior after the standards are defined more clearly.
(I heard from somewhere that sometimes the implementation should go along while the standards is also evolving, hope this applies here as well.)

Thanks.

Comment 10

[David Flanagan [:djf]]

What I was suggesting is that we use inputmode=numeric, but we make a special case for type=password to show the pinLayout.  And for other types we show the numberLayout. I don't know if that is a good idea or not.

Let me know when you want me to contine the review.

Comment 12

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

Comment on attachment 751344 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9859/files

Hi David,

I have refined the patch of keyboard part as follows,

1. Enable both numeric and digit inputmode for Keyboard.
   (Note that mode="digits" has been changed to "digit", according to Bug 746142#c9)
    This part needs Yuan's feedback.

2. Also fix a regression that sometime the suggestion bar would show up incorrectly, the solution is to call renderKeyboard() after IME activation, so that the suggestion/capitalization status could be reflected correctly.

The changes are in a separate commit, hope to make it easier to review,
https://github.com/RudyLu/gaia/commit/1f9487e7c935783d723a9dbaec40e65bf2c5d354

I will squash them if r+.
Thanks.

Comment 13

[David Flanagan [:djf]]

Comment on attachment 751344 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9859/files

r=djf for github commits 8aeefcf and 1f9487e.

I'm giving r+ here, but I do have some nits on github.  In particular, I did want to say that the singular form "inputmode=digit" sounds like an error to my ear.  I do think the plural version is better.

Also, please consider modifying the number layouts in layout.js to add a bigkey:true flag to the layouts or keys that need it.  That way render.js can just check the layout data to find out if the big-key CSS class is needed.  That seems like better modularity to me.

Thanks also for fixing when renderKeyboard() is called.

Comment 14

[Yuan Xulei [:yxl] (Leave from May 18)]

(In reply to Rudy Lu [:rudyl] from comment #12)
> Comment on attachment 751344 [details]
> Pointer to Github pull request:
> https://github.com/mozilla-b2g/gaia/pull/9859/files
> 
> Hi David,
> 
> I have refined the patch of keyboard part as follows,
> 
> 1. Enable both numeric and digit inputmode for Keyboard.
>    (Note that mode="digits" has been changed to "digit", according to Bug
> 746142#c9)
>     This part needs Yuan's feedback.

It seems there is no standard specification for inputmode now and it has change a lot since Bug 746142. We may not need to change the name |digits| to |digit|.

Mounir will be a better person to provide feeback as he knowns more about the inputmode and its standard.

Comment 15

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

Hi David, Yuan,

Thanks for your feeback.

I have updated the patch with the following commit,
https://github.com/RudyLu/gaia/commit/11055a51c4c666456d15bed2523633f95f548b69

For 'digit' or digit"s" for the inputmode, I tend to use 'digit' because it synced with Gecko part of implementation.
e.g. https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/GeckoInputConnection.java#576

Comment 16

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

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9859/files

Pointer to Github pull-request

Comment 17

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

Comment on attachment 754410 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9859/files

Dear Arthur,

Please help review the simpin dialog related changes, which could be found in commit, https://github.com/RudyLu/gaia/commit/bcc9805a96f3346a4006460553b930e30bf7e014.

In this patch, we use "type=password, x-inputmode=digit" for the SIM pin, PUK code, etc., instead of using the previous hack that would do the combining of one number input and one text input.

Thanks.

Comment 18

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

I found that we also have SIM pin related code in FTU, but not sure why it does use "inputmode=digits" before.

Hi Fernando,

May I know when we could see the SIM pin dialog in FTU (not the one in system app)?
Thanks.

Comment 19

[Fernando Campo (:fcampo)]

(In reply to Rudy Lu [:rudyl] from comment #18)
> I found that we also have SIM pin related code in FTU, but not sure why it
> does use "inputmode=digits" before.
> 
> Hi Fernando,
> 
> May I know when we could see the SIM pin dialog in FTU (not the one in
> system app)?
> Thanks.

The SIM pin screen is the second screen on FTU, should appear after language screen (1st one), but only if we have a SIM inserted and PIN protected. If we don't have SIM, it will jump to wifi, if PIN is not enabled, it will only show the option for enabling mobile-data.

Comment 20

[Mounir Lamouri (:mounir)]

(In reply to Yuan Xulei [:yxl] from comment #14)
> (In reply to Rudy Lu [:rudyl] from comment #12)
> > Comment on attachment 751344 [details]
> > Pointer to Github pull request:
> > https://github.com/mozilla-b2g/gaia/pull/9859/files
> > 
> > Hi David,
> > 
> > I have refined the patch of keyboard part as follows,
> > 
> > 1. Enable both numeric and digit inputmode for Keyboard.
> >    (Note that mode="digits" has been changed to "digit", according to Bug
> > 746142#c9)
> >     This part needs Yuan's feedback.
> 
> It seems there is no standard specification for inputmode now and it has
> change a lot since Bug 746142. We may not need to change the name |digits|
> to |digit|.
> 
> Mounir will be a better person to provide feeback as he knowns more about
> the inputmode and its standard.

"digits" or "digit" doesn't change much to me. There is no standard for inputmode even if there might sound like it because Hixie wrote something. This is still not a standard. We should feel free to do whatever we want for the moment until Mozilla comes up with a real implementation we mean to support (not "x-" prefixed) and get standardized.

Comment 21

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 754410 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9859/files

Thanks for the patch. It looks good to me. r=me.

Comment 22

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

Comment on attachment 754410 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9859/files

Just updated the pull request for last part:
Use type=password, x-inputmode=digit for simpin input in FTU
 https://github.com/RudyLu/gaia/commit/db09b148263e68ac9d81101735fb4b44a503ea96

Dear Fernando,

Could you help take a look at this change?
Thanks.

Comment 23

[Fernando Campo (:fcampo)]

Comment on attachment 754410 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9859/files

FTU changes looks great, thanks for the patch!

Comment 24

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

Merged to Gaia master,
https://github.com/mozilla-b2g/gaia/commit/8eec8e331ec58e0a1b3d0b926d0aaa0d0b50f477

Hi all,

Thanks for all the reviews.

Comment 25

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

Gaia v1-train:
5919649b3ab60759ad2b55dc7e38b4cfaec7d7b9