1008061 - [OPEN C_1.3] There are two input text in "Enter NCK code" windows

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System, defect, P2)

Description

[chencong]

Attachment: NCK windows.bmp

User Agent: Mozilla/5.0 (Windows NT 6.1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/34.0.1847.131 Safari/537.36

Steps to reproduce:

1、 Lock the Open C network Operator by MCC and MNC code;
2、 Insert one invalid sim;
3、 Power on the phone, it will display one Enter NCK code windows;


Actual results:

There are two input text controller in "Enter NCK code" windows


Expected results:

There is one input text controller in "Enter NCK code" windows

Comment 1

[chencong]

Attachment: NCK Enter Code.zip

Comment 2

[isabelrios]

Hi Chen Cong,

Have you been able to reproduce this bug also in an Open 2 device, just wondering if this issue could be related to the change of the screen size and resolution.

Also, once you see that screen, is it possible to insert the NCK code? And, in that case, is it working?

Thanks!

Comment 3

[chencong]

Yes, it can reproduce in Open 2 or Open C device.  It works when input the NCK code in first input number controller.  I think it is OK if delete the second input text controller and change the input number controller to the size of the input text controller.

Comment 4

[isabelrios]

Hi EJ,

I have been told that you worked a lot on dsds locks issues. Maybe you could help us with this bug.  
We are wondering if this could be a regression on 1.3 branch after the work done related to that. Could you please have a look?

Thank you in advance

Comment 5

[Jason Smith [:jsmith]]

Vance - Can you find out if this is a cert blocker?

Comment 6

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

(In reply to Isabel Rios [:isabel_rios] from comment #4)
> Hi EJ,
> 
> I have been told that you worked a lot on dsds locks issues. Maybe you could
> help us with this bug.  
> We are wondering if this could be a regression on 1.3 branch after the work
> done related to that. Could you please have a look?
> 
> Thank you in advance

Hi @Isabel, 

I checked codebase from v1.0 ~ master, we all have the second input field from then on so this may not be a regression from v1.3. 

@José, can you help to put some comments here because I checked back the whole history and found you are the author of this part of code. thanks :)

Comment 7

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Hi @Isabel, I am back again ! After digging more about this history, the root cause is bug 869879.

Because at that time, we would use a fake input field to make a fake UI with **** on it. After the patch, all related styles got removed but there is one input field left without changing which makes this bug.

So this is 1.3+ bug ?

Comment 8

[Juan Perez-Bedmar [:juanpbf]]

(In reply to Jason Smith [:jsmith] from comment #5)
> Vance - Can you find out if this is a cert blocker?

Yes, this is a cert. blocker that must be solved within 1.3 scope, as it is preventing our devices to get cert. approvals in several countries

Comment 9

[isabelrios]

(In reply to EJ Chen [:eragonj][:小龍哥] from comment #7)
> Hi @Isabel, I am back again ! After digging more about this history, the
> root cause is bug 869879.
Hi EJ, thanks for your investigation on this bug

> Because at that time, we would use a fake input field to make a fake UI with
> **** on it. After the patch, all related styles got removed but there is one
> input field left without changing which makes this bug.
> 
> So this is 1.3+ bug ?
Yes, per previous comment #8

Comment 10

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

ok, I can help on this bug. Just took it.

Comment 11

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Attachment: patch on master

Comment 12

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Attachment: patch on v1.3

Comment 13

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Attachment: patch on v1.4

Comment 14

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

@Isabel,

can you help me check if these patches help because I have no device on my side right now and these patches are just one-line fix to remove the input field.

If not, I would try my best get one if possible.

Thanks :)

Comment 15

[isabelrios]

Thanks for the patch EJ,

Chen Cong would you please be so kind to check if the patch works?
We do not have a device sim locked.
Thanks!

Comment 16

[chencong]

It only displays number input controller. But the number input controller is too short.

Comment 17

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

(In reply to chencong from comment #16)
> It only displays number input controller. But the number input controller is
> too short.

Yes, I didn't change the width of the first number input controller. 

If you want to change the width of the input, what's the needed width !?

Comment 18

[chencong]

Would you please modify the width as the text input controller? The width of text input controller seems ok.

Comment 19

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

(In reply to chencong from comment #18)
> Would you please modify the width as the text input controller? The width of
> text input controller seems ok.

I just updated the CSS and I think it would be better to make it 100% width of the parent container and it looks good on master ! Hope this is to you !

Comment 20

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

If they all look good, I would start the review process. Thanks Chencong !

Comment 21

[chencong]

I think it is better to make it 100% width of the parent container. Would you please release the patch?

Comment 22

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

(In reply to chencong from comment #21)
> I think it is better to make it 100% width of the parent container. Would
> you please release the patch?

Ok cool, I would ask for reviewing process. THanks :)

Comment 23

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8420832 [details] [review]
patch on master

Hi Alive, 

can you help me review this small patch that focusing on removing out-dated input field ?

From the commit history, it says that we already removed all the other hinting input fields when keyboard supports `number keyboard`, but there is one input field left and forgot to remove.

Thanks :)

Comment 24

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8420833 [details] [review]
patch on v1.3

Same as above.

Comment 25

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8420835 [details] [review]
patch on v1.4

Same as above.

Comment 26

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8420835 [details] [review]
patch on v1.4

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined: users would see one more input field when typing NCK code
[Testing completed]: no, just remove one redundant input field and update CSS. (After checking back from the history, this one was forgot to be removed when keyboard app supports number keyboard)
[Risk to taking this patch] (and alternatives if risky): low
[String changes made]: no

Comment 27

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Merged patch into 

master: 95349002a417494b3edc2af1309d25ce4055a425
v1.3: 0ce948e378cab7ed3db20231281dd7ca2eb99779

And let's wait for v1.4 approval :)

Comment 28

[Jason Smith [:jsmith]]

Loli - Can you look into adding test coverage for this?

Comment 29

[isabelrios]

Hi Jason,

In order to test this it is necessary to have a device with a build simlocked. Usually the builds used for the test runs are not locked so this could not be checked unless using a build locked to an specific operator, which is done by the vendor.

Just to let you know that although the test case is added to the test run, it will remain as blocked due to that reason.

Comment 30

[Jason Smith [:jsmith]]

Okay - marking in-moztrap- then for that reason.

Comment 31

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Thanks Preeti, 

just merged into Gaia/v1.4 : 961b79a924dd4389bdf9e9ca482e3305cdea2436

All patches got landed into Gaia, so I would just mark this bug as resolved ! 

Thanks all :)

Comment 32

[Ryan VanderMeulen [:RyanVM]]

Per https://wiki.mozilla.org/Release_Management/B2G_Landing, this should have had approval to land on v1.3. Funny enough, it *didn't* need 1.4 approval. Can you please familiarize yourself with the B2G Landing page and fix up the approval flags here?

Comment 33

[bhavana bajaj [:bajaj]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #32)
> Per https://wiki.mozilla.org/Release_Management/B2G_Landing, this should
> have had approval to land on v1.3. Funny enough, it *didn't* need 1.4
> approval. Can you please familiarize yourself with the B2G Landing page and
> fix up the approval flags here?

switching the NI on :preeti as she approved the patches here. Preeti, since this is blocking 1.3, this should have had the 1.3 approval request and would get auto-approved for 1.4 if we a+Ed it. I think the shreiff could have easily missed the 1.3 uplift in this case, as the patch is requested for 1.4. Thanks Ryan for catching it.

Comment 34

[Preeti Raghunath(:Preeti)]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #32)
> Per https://wiki.mozilla.org/Release_Management/B2G_Landing, this should
> have had approval to land on v1.3. Funny enough, it *didn't* need 1.4
> approval. Can you please familiarize yourself with the B2G Landing page and
> fix up the approval flags here?

Yes Ryan,

There should have been a request for 1.3. Since there wasn't one approved it for 1.4 (which was the ask).

Comment 35

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Thanks Ryan, 

I would fix the flag now.

Comment 36

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Thanks Ryan, 

I would fix the flag now.

Comment 37

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8420833 [details] [review]
patch on v1.3

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined: users would see one more input field when typing NCK code
[Testing completed]: no, just remove one redundant input field and update CSS. (After checking back from the history, this one was forgot to be removed when keyboard app supports number keyboard)
[Risk to taking this patch] (and alternatives if risky): low
[String changes made]: no

Comment 38

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8420835 [details] [review]
patch on v1.4

Take off this approval tag because 1.3+ blocking bugs have auto-approval to land on 1.4 if affected and do not need additional approval.

Comment 39

[bhavana bajaj [:bajaj]]

Comment on attachment 8420833 [details] [review]
patch on v1.3

just rubberstamping at this point, its already landed on the right branches.

Comment 40

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Thanks :bajaj !