832093 - STK display text and input timeout needs to be increased

Status: VERIFIED FIXED

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

Description

[Carol Yang [:cyang]]

The original issue of the GET_INPUT display timeout was reported in: https://bugzilla.mozilla.org/show_bug.cgi?id=827655

In comment 19 of that bug, there was a request to make the timeout to be reduced to 5 seconds in order to pass the TC referenced in that bug. With the reduction to 5 seconds, the TC can be passed however, the change creates an usability issue in other TCs where the user does not have enough time to do anything before the input times out.

This bug is used to track changes that would need to be made to increase the timeout value for both display text and input. The value can be anywhere between 30 to 60 seconds, whatever makes the most sense. Note that the values for display text and input should be the same though.

Comment 1

[psiddh]

Attachment: Configurable property for STK UI timeout

Comment 2

[psiddh]

Testers can set the system property this way at the time of boot up (early bootup)

adb shell setprop ril.icc.getinput_timeout-ms 15000 // 15 s

This will need a fix in gaia - icc.js as well

Just uploaded initial fix .

Comment 3

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

CCing frsela and ochameau

Hi, Siddartha

I think the timeout value should be related to UI, so I think this should be done in gaia side.

Comment 4

[psiddh]

OK.. but how do you make it configurable at the gaia side. One option that I can think of is to use - prefs.js way.
So the question is , how do we make the same piece of code in gaia pass test cases that require ( 5 - 10 s) timeout for a special GetInkey/Input use case and also make it pass in default time out scenarios ?

Comment 5

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Hi, I agree with Yoshi, this is now in Gaia:

https://github.com/mozilla-b2g/gaia/blob/master/build/settings.py#L135

So it's accesible through the mozSettings API.

We can recover as in:

https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/js/icc.js#L98

or modify as in:

https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/icc_cache.js#L64

So we can modify it from Gaia side, the only missing piece is a settings page to change it or leave it for the operator variant perso.

@dcoloma: WDYT? It's better to allow users to perso it's STK default settings or only allow operator variant to change it.

Comment 6

[Daniel Coloma:dcoloma]

Carol, I must admit I am totally lost here. Are the pointers provided by Fernando enough to configure the device to pass the tests?

Comment 7

[psiddh]

Fernando, Could you please tell me  how I can configure 'icc.displayTextTimeout' & 'icc.inputTextTimeout'  at the Gaia side at runtime ?

Comment 8

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

(In reply to Siddartha P from comment #7)
> Fernando, Could you please tell me  how I can configure
> 'icc.displayTextTimeout' & 'icc.inputTextTimeout'  at the Gaia side at
> runtime ?

Executing this code:

                var reqIccData = window.navigator.mozSettings.createLock().set({
                  'icc.displayTextTimeout': <new value>
                });
                reqIccData.onsuccess = function icc_setIccData() {
                  // Callback after update
                }


                var reqIccData = window.navigator.mozSettings.createLock().set({
                  'icc.inputTextTimeout': <new value>
                });
                reqIccData.onsuccess = function icc_setIccData() {
                  // Callback after update
                }

Comment 9

[psiddh]

Hi Fernando, Please correct me here unless I am missing something here ?
We need a way to dynamically configure timeouts during run time (at the time of executing the test case) . Isn't? Would testers be able to update these gals ' icc.displayTextTimeout' & 'icc.inputTextTimeout' during runtime ?  If not following was my suggestion:-
There is one default time out  in icc.js in gaia. ( maybe  default value could be 45 s. Current value is 5 s).  We had to put 5 s because we were expected to pass a certain GCF T.C. But this will greatly impact user experience , as 5 s is too short a period. Therefore the suggestion to have a default value set to 45 s (instead of current 5 s). 
Now GCF testers at the time of running this special test case can simply set the configurable prop to 5s : adb shell setprop ril.icc.getinput_timeout-ms 5000 - This will ensure that GCF test case is passed. However for all practical purposes we would have 45s as default timeout which is reasonable time in such scenarios.
What do you think ? Correct me as well if I was off mark!

Comment 10

[Daniel Coloma:dcoloma]

Nominating as it blocks bug 808607

Comment 11

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

(In reply to Siddartha P from comment #9)
> Hi Fernando, Please correct me here unless I am missing something here ?
> We need a way to dynamically configure timeouts during run time (at the time
> of executing the test case) . Isn't? Would testers be able to update these
> gals ' icc.displayTextTimeout' & 'icc.inputTextTimeout' during runtime ?  If
> not following was my suggestion:-
> There is one default time out  in icc.js in gaia. ( maybe  default value
> could be 45 s. Current value is 5 s).  We had to put 5 s because we were
> expected to pass a certain GCF T.C. But this will greatly impact user
> experience , as 5 s is too short a period. Therefore the suggestion to have
> a default value set to 45 s (instead of current 5 s). 
> Now GCF testers at the time of running this special test case can simply set
> the configurable prop to 5s : adb shell setprop ril.icc.getinput_timeout-ms
> 5000 - This will ensure that GCF test case is passed. However for all
> practical purposes we would have 45s as default timeout which is reasonable
> time in such scenarios.
> What do you think ? Correct me as well if I was off mark!

Now it can be changed from the code, but the question is if we want to create a UI to change this settins. Really I don't see any user changing STK setting.

Another alternative is move this to applications.js (as we do with defaultURL) so it's configurable at building time.

Finally, the fastest one is to set a static value of 45000 if you want.

Please, let me know what's better to you ;)

Comment 12

[Daniel Coloma:dcoloma]

I am confused, there should be only one value for this setting and that one should allow to pass all the test cases with good usability. Increasing this value to 30/40 secs would not make bug  827655 occur again?

Comment 13

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Attachment: Increased timeout to 40 secs

Increased timeout

Comment 14

[psiddh]

@frsela, Yes  I agree that the default timeout should be 40s and it makes perfect sense. Infact we can have only one timer called UI_TIMEOUT and use the same value for both DISPLAY TEXT & GETINPUT / INKEY cases. However that said, I am OK with having two different settings for two proactive commands as it is the case now.

Could you please clarify if 'Attachment #704444 [details]' would break 'bug  827655' ? I would think so. What are your thoughts ? In the earlier comment you were mentioning to use a UI setting for configuring 'time-out' values. So is this UI setting going to be in some hidden menu ? I think. having a build time option may not be a viable solution.

Comment 15

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

(In reply to Siddartha P from comment #14)
> @frsela, Yes  I agree that the default timeout should be 40s and it makes
> perfect sense. Infact we can have only one timer called UI_TIMEOUT and use
> the same value for both DISPLAY TEXT & GETINPUT / INKEY cases. However that
> said, I am OK with having two different settings for two proactive commands
> as it is the case now.
> 

Yeap, I agree, but this is not blocker so we can change it in the future.

> Could you please clarify if 'Attachment #704444 [details]' would break 'bug 
> 827655' ? I would think so.

I tested on my Android handset and waited near 60 secs.

> What are your thoughts ? In the earlier comment
> you were mentioning to use a UI setting for configuring 'time-out' values.
> So is this UI setting going to be in some hidden menu ? I think. having a
> build time option may not be a viable solution.

This timeout is in mozsettings, so it can be changed by an app like settings. We can do that, but the question is if it's really needed.

Another alternative is to move to personalization settings at compile time (like we do with defaultURL)  but as the previous point, this is not blocker now so meanwhile we can live with mozSettings ;)

Comment 16

[Daniel Coloma:dcoloma]

based in comment 12, do you think the patch frsela suggests is enough?

Comment 17

[Anshul]

Daniel/Fernando/Siddartha, the patch fresla suggested should be good enough. We have asked the test team to configure the timer in the GCF setup to a longer value for bug 827655 to work with the default timer of 40s being proposed in the patch by fersla.

Comment 18

[psiddh]

Thanks Anshul. The fix from Fernando should suffice then

Comment 19

[Anshul]

Fernando, could you please land your change?

Comment 20

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 704444 [details]
Increased timeout to 40 secs

Please fix your commit message before landing in order to include the bug number.

Comment 21

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #20)
> Comment on attachment 704444 [details]
> Increased timeout to 40 secs
> 
> Please fix your commit message before landing in order to include the bug
> number.

Thanks, commit desc. changed.

Landed: https://github.com/mozilla-b2g/gaia/commit/19954b0ee2c3e745582c2cbcfab01a4b58e27b04

Comment 22

[Lukas Blakk [:lsblakk] use ?needinfo]

Landed on mozilla-b2g18/gaia master prior to the 1/25 branching to mozilla-b2g18_v1_0_0/v1.0.0, updating status-b2g-v1.0.0 to fixed.

Comment 23

[amiller]

Unagi Build ID: 20130313070202
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/e74dafa6b2d9
Gaia: b34e726147f8e671ad8c538b50900ccfbffcb084
Kernel: Dec 5th

The Increased timeout to 40 secs is verified. Issue does not reproduce.