880356 - [STK] "STK_CMD_GET_INPUT" code refactoring

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Noemí Freire (:noemi)]

STK code refactoring moving "STK_CMD_GET_INPUT" code from settings to system

Comment 1

[Alex Keybl [:akeybl]]

Milestoning for QE3 (due by 6/24), since this blocks bug 875679 which blocks 865985 (QE3 milestoned).

Comment 2

[Noemí Freire (:noemi)]

Notice that Bug 881672 needs to be uplifted to v1-train in order to enable STK/ICC testing

Comment 3

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

Attachment: STK_CMD_GET_INPUT moved to system

Comment 4

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

Attachment: STK_CMD_GET_INPUT moved to system

Comment 6

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

Comment on attachment 767201 [details]
STK_CMD_GET_INPUT moved to system

Please try not to mess up with the class names ('locked') set by other modules.

Comment 7

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

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #6)
> Comment on attachment 767201 [details]
> STK_CMD_GET_INPUT moved to system
> 
> Please try not to mess up with the class names ('locked') set by other
> modules.

Hi Tim,

The case here is if we receive a STK INPUT command while screen is locked ... should we attend this inmediatenly?

In order to allow this, I check if screen is locked so unlock it to allow keyboard to work, after that I'll lock screen again.

Any other better idea to do this?

Comment 8

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

Move the STK features to an app and use the attention screen maybe?

In any case, breaking module boundary with the code here is _not_ acceptable.

Comment 9

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

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #8)
> Move the STK features to an app and use the attention screen maybe?
> 
> In any case, breaking module boundary with the code here is _not_ acceptable.

I'll avoid showing input box over lock screen so it should be attended when the handset is unlocked.

In a half an hour or so I'll change this.

Comment 10

[(inactive after 6/18) Alive Kuo [:alive]]

It sounds bad to me to call lock and unlock arbitrary in other module other than lockscreen.
And where is the discussion about why we need to move all ICC stuff into system? Are you notifying any system app owner/peer about this?

Anyway, you should put z-index of any new element under screen element to |zindex.css|
Put it anywhere in the module makes it unmanageable.

Comment 11

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

Comment on attachment 767201 [details]
STK_CMD_GET_INPUT moved to system

We need to land this today so I rebased and now I maintain STK dialogs behind LOCK screen so no "auto-magick" unlocks by now ;)

I'm fixing the remove of the event in https://github.com/mozilla-b2g/gaia/pull/10607/files#L3R226 which is not working well, but if you can start the review we can move faster.

Comment 12

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

I won't be able to review this today under your time frame. Please find someone in the Europe timezone for that.

Comment 13

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

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #12)
> I won't be able to review this today under your time frame. Please find
> someone in the Europe timezone for that.

Do you suggest somebody?

BTW, event remove fixed.

Comment 14

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

Comment on attachment 767201 [details]
STK_CMD_GET_INPUT moved to system

Julien, can you review this patch?
Is urgent since it's needed in leo ...

Comment 15

[(inactive after 6/18) Alive Kuo [:alive]]

Unfortunately Julien is right now in Taipei work week...
Maybe you should find :etienne.

Comment 16

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

(In reply to Alive Kuo [:alive] from comment #15)
> Unfortunately Julien is right now in Taipei work week...
> Maybe you should find :etienne.

Thanks

Comment 17

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

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

Comment 18

[Noemí Freire (:noemi)]

Hi John,

Could you please help us on uplifting this patch to V1-train?, we need to unblock STK/ICC testing on V1-train. Thanks!

Comment 19

[John Ford [:jhford] CET/CEST Berlin Time]

I was not able to uplift this bug to v1-train.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1-train, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1-train
  git cherry-pick -x -m1 b2da43408f4f53429db6b0a7f22c80aa73b87580
  <RESOLVE MERGE CONFLICTS>
  git commit

Comment 20

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

Fixed: https://github.com/mozilla-b2g/gaia/commit/bd02ba45ff3fba6701da4940e63f60a15e551b38

Comment 21

[Gareth Aye [:gaye] (back from PTO)]

v1.1.0hd: bd02ba45ff3fba6701da4940e63f60a15e551b38

Comment 22

[dkumar]

Can you please provide steps to verify this fix?

Comment 23

[Axel Hecht [:Pike]]

Adding late-l10n keyword as this introduced new strings, even though actually getting them exposed to l10n is bug 901449