1196178 - [Settings]The "Ok" icon doesn't respond correctly, changing on the APN can't be saved.

Status: VERIFIED FIXED

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

Description

[Shally Li（leave from Mozilla）]

[1.Description]:
[Flame KK v2.2&v2.5][Aries KK v2.5][Settings]Modifying a APN (ex:  Identifier, Password...), then tapping the "OK" icon, the "Ok" icon will flash to gray (unavailable) and then immediately restore to highlighted(available), but the change can't be saved anyway.

Found at: 12:57
Attach: APN_FlameKK_v2.5.3gp and logcat_1257.txt.

[2.Testing Steps]: 
1. Power on device with SIM2 or SIM1 card.
2. Go to Settings -> Cellular & Data -> Choose SIM1 -> Select APN settings -> Data Settings page 
3. Choose a new added APN,  or default APN. 
4. Modify the APN, 
ex: APN, Identifier, Password, HTTP proxy host, HTTP proxy port, Authentication, Protocol, and Roaming Protocol. 
5. Click "OK" icon. 

[3.Expected Result]: 
5. After modifying the APN, the change can be saved successfully. 

[4.Actual Result]: 
5. "Ok" icon will flash to gray (unavailable) and then immediately restore to highlighted (available), but the change in step 4 can't be saved.

[5.Reproduction build]: 
Device: Flame KK 2.2 (Affected) 
Build ID               20150818032504
Gaia Revision          335cd8e79c20f8d8e93a6efc9b97cc0ec17b5a46
Gaia Date              2015-08-14 19:06:41
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/c29970d16372
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150818.065127
Firmware Date          Tue Aug 18 06:51:38 EDT 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0

Device: Flame KK 2.5 (Affected)
Build ID               20150818150207
Gaia Revision          507ba38fb64b27f87d11f4104dfcc58448e12b1a
Gaia Date              2015-08-18 10:50:12
Gecko Revision         https://hg.mozilla.org/mozilla-

central/rev/2c272af993c23e803f6ea7798a812b0c8abfad4d
Gecko Version          43.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150818.184156
Firmware Date          Tue Aug 18 18:42:07 EDT 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0

Device: Aries KK 2.5(Affected)
Build ID               20150818233027
Gaia Revision          1e1197e0e8e64307aa382ffba4711d1c661de7ca
Gaia Date              2015-08-18 16:54:35
Gecko Revision         https://hg.mozilla.org/mozilla-

central/rev/88e1d293ffa9faf065b2a944c94a2705aabf1fb9
Gecko Version          43.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20150818.225517
Firmware Date          Tue Aug 18 22:55:24 UTC 2015
Bootloader             s1


[6.Reproduction Frequency]: 
Always Recurrence,5/5

[7.TCID]: 
15053

[8.Note]:
After step 3, if you do nothing and directly tap "Ok" icon, the behaviour of "Ok" icon is the same as actual result mentioned above.

Comment 1

[Shally Li（leave from Mozilla）]

Attachment: logcat_1257.txt

Comment 2

[Shally Li（leave from Mozilla）]

Attachment: APN_FlameKK_v2.5.3gp

Comment 3

[William Hsu [:whsu]]

[Blocking Requested - why for this release]:

Basic functionality is broken.
"OK" button cannot work as usual on APN configuration page.

Comment 4

[William Hsu [:whsu]]

[Blocking Requested - why for this release]:

Comment 5

[Mahendra Potharaju [:mahe]]

Broken functionality. Blocking 2.5

Comment 6

[Evan Tseng [:evanxd]]

Hi Shally,

I cannot reproduce this bug.

The environment is
Build ID               20150826150203
Gaia Revision          c1ae9f02f2a9cfb89bf67aeea97e467c41c3362c
Gaia Date              2015-08-25 22:03:05
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/fea87cbeaa6b64510dff835549ed906fe405d558
Gecko Version          43.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150622.193834
Firmware Date          Mon Jun 22 19:38:45 EDT 2015
Bootloader             L1TC00011880

Comment 7

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

I was able to reproduce this issue on Flame and Aries builds.  Of note is that step 3 of the description is referring to pressing the name of the APN note the radial button to bring up the APN editor page.

Actual Results: Pressing the OK button on an APN Editor does not save the information.

Environmental Variables:
Device: Aries 2.5
BuildID: 20150827170205
Gaia: d784c81961d82cbe9e111405468c590a8345856c
Gecko: ca086f9ef8bca2d6cdfa79bfc4c854f56a59859e
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 43.0a1 (2.5) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0

Environmental Variables:
Device: Flame 2.5
BuildID: 20150827083151
Gaia: d784c81961d82cbe9e111405468c590a8345856c
Gecko: b33eae31bd7188024b54228e0c0345800a65e595
Gonk: c4779d6da0f85894b1f78f0351b43f2949e8decd
Version: 43.0a1 (2.5) 
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0

Comment 8

[Shally Li（leave from Mozilla）]

Attachment: (2)Flamekk_v2.5.3gp

Hi Evan,

    Please don't enable data roaming for reproduction. Because the 'OK' button will be available after enabling data roaming and tapping "Yes" button on the roaming confirmation dialog, the modification APN can be saved successfully. 
    Please refer the attachments "(2)Flamekk_v2.5.3gp" and "(2)logcat_1130.txt".

Thank you.

Comment 9

[Shally Li（leave from Mozilla）]

Attachment: (2)logcat_1130.txt

Device: Flame KK 2.5 (Affected)
Build ID               20150827150209

Device: Aries KK 2.5(Affected)
Build ID               20150827170125

Comment 10

[Evan Tseng [:evanxd]]

Shally, thanks.

I can reproduce with disabling data roaming.

Comment 11

[GitHub Autolander Bot]

Attachment: [gaia] evanxd:bug-1196178 > mozilla-b2g:master

Comment 12

[Evan Tseng [:evanxd]]

Comment on attachment 8653956 [details] [review]
[gaia] evanxd:bug-1196178 > mozilla-b2g:master

Fred, how do you think the patch?

We always need to show the change apn warning when data roaming is on or off.

Comment 13

[Evan Tseng [:evanxd]]

Comment on attachment 8653956 [details] [review]
[gaia] evanxd:bug-1196178 > mozilla-b2g:master

Evelyn, how do you think the patch?

We always need to show the change apn warning when data roaming is on or off.

Comment 14

[Evan Tseng [:evanxd]]

It follows UI design from Harly.

Comment 15

[[e-mail]]

Comment on attachment 8653956 [details] [review]
[gaia] evanxd:bug-1196178 > mozilla-b2g:master

Thank for take a look on this issue. But the PR only solve parts of the issue, It still fail when editing non-active item.

1. create an extra apn
2. tap one of apn that not selected (activeApnId !== _apnItem.id)
3. edit and press ok

the ok button does not work correctly.

Comment 16

[[e-mail]]

For always showing the `change apn warning` when data roaming is on or off, I've noticed that there's a clear statement in code to describe the show warning condition:

1. when Data roaming is turned on
2. when the current APN in use that’s being edited

https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/js/panels/apn_editor/panel.js#L84

So I think the proper way is to keep current assertion (of the show warning condition) and made other cases still editable (can save the change without warning dialog). 

@harly how do you think?

Comment 17

[Harly Hsu[:harly]]

After an offline discussion with Fred, I think that user should be able to save the APN settings no matter if the APN is in used or if Data roaming is turned on.

Comment 18

[Evan Tseng [:evanxd]]

Thanks, Harly and Fred.

I'll update the patch for your comments.

Comment 19

[Evan Tseng [:evanxd]]

Updated patch[1] for Fred's comments.

Continue to add tests.

[1]: https://github.com/evanxd/gaia/commit/9f7abfa762c3379e32b4cd3297e9b52bfea4901c

Comment 20

[Evan Tseng [:evanxd]]

Added tests[1].

But I have questions about writing tests with good practice. Is that good if I write tests like [2]?

[1]: https://github.com/evanxd/gaia/commit/66ee21cc4d729677126c5373b4e33a8f2b0d4e26
[2]: https://github.com/evanxd/gaia/commit/66ee21cc4d729677126c5373b4e33a8f2b0d4e26#diff-978bbcc8a03cb9d2040869a726b6166cR63

Comment 21

[Evelyn Hung]

Comment on attachment 8653956 [details] [review]
[gaia] evanxd:bug-1196178 > mozilla-b2g:master

Okay to me although the condition looks like a bit weird.

Comment 22

[Evan Tseng [:evanxd]]

Thanks Fred and Evelyn's feedbacks.

I was trying to write unit tests for the patch[1]. I did some workaround there because the module is not testable. I'll file a follow up bug to fix that.

[1]: https://github.com/evanxd/gaia/commit/66ee21cc4d729677126c5373b4e33a8f2b0d4e26

Comment 23

[Evan Tseng [:evanxd]]

The follow up bug: Bug 1204460.

Comment 24

[Evan Tseng [:evanxd]]

Comment on attachment 8653956 [details] [review]
[gaia] evanxd:bug-1196178 > mozilla-b2g:master

Hi Evelyn,

Could you review the patch?

We don't have tests because the module is not testable. But we will fix it in the follow up bug(Bug Bug 1204460).

Thanks.

Comment 25

[Evan Tseng [:evanxd]]

Comment on attachment 8653956 [details] [review]
[gaia] evanxd:bug-1196178 > mozilla-b2g:master

Hi Fred,

Could you review the patch?

We don't have tests because the module is not testable. But we will fix it in the follow up bug(Bug Bug 1204460).

Thanks.

Comment 26

[[e-mail]]

Comment on attachment 8653956 [details] [review]
[gaia] evanxd:bug-1196178 > mozilla-b2g:master

looks fine! 

During the test I found another 2 issues in apn

1. when dialog is shown, no matter I press any button, the radio will set to current tapped item. 
2. the apn item is only select-able on string, but not the whole field

Please also fire bugs for them.

Comment 27

[Evan Tseng [:evanxd]]

Fred,

Sure, let me test the two issues.

Comment 28

[Evan Tseng [:evanxd]]

If the treeherder[1] is good, let's land the code.

[1]: https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=d6de94bf26a04d6c929123f7b626fb4737089c75

Comment 29

[Evan Tseng [:evanxd]]

GijTV is broken. We'll land after that is fixed.

Comment 30

[Evan Tseng [:evanxd]]

Land the code. The failures of GijTV is not about the patch.

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

Comment 31

[Evan Tseng [:evanxd]]

File bugs for Comment 26.

Comment 32

[Shally Li（leave from Mozilla）]

This bug has been verified as "pass" on the latest build of Flame KK 2.5 and Aires KK 2.5 by the STR in comment 0.

Actual results: When changing the existing APN or added APN, the "Ok" icon works normally, changing on the APN is saved successfully.
See attachment: verified_Aries_v2.5.3gp
Reproduce rate: 0/10


Device: Flame KK 2.5 (Pass)
Build ID               20150917150223
Gaia Revision          aede8622d780ec71f766a3ecccbff74c04aaba4e
Gaia Date              2015-09-17 03:40:46
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/3929b8fc6c33bf9cc80299743063f0981f545452
Gecko Version          43.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150917.183330
Firmware Date          Thu Sep 17 18:33:40 EDT 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0

Device: Aries KK 2.5 (Pass)
Build ID               20150917232610
Gaia Revision          2082894c8e974b0c371e4dec298e0ad0f3ac56b1
Gaia Date              2015-09-17 14:56:47
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/01a75ffad1024a3f75d494fb77a022c96a497eb2
Gecko Version          43.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20150917.224608
Firmware Date          Thu Sep 17 22:46:16 UTC 2015
Bootloader             s1

Comment 33

[Shally Li（leave from Mozilla）]

Attachment: verified_Aries_v2.5.3gp