1106956 - [FFOS2.0][Woodduck][STK]The idle text can't be removed after power off and on.

Status: RESOLVED FIXED

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

Description

[sync-1]

email:pengfei.huang.hz.com
 tel:[phone]
 DEFECT DESCRIPTION:
 The idle text can't be removed after power off and on.
 
  REPRODUCING PROCEDURES:
 1.power on with SIM card which can send "set up idle text" command.
 2.Enter settings->STK menu->excute STM12006--Remove “Idle Text” by OFF/ON from handset
 3.power off and on
 4.check the text still in the state bar.--KO
 
  EXPECTED BEHAVIOUR:
 The idle text should be removed after power off and on.
 
 reporter's office number:0752-2639219(61219)
 
  ASSOCIATE SPECIFICATION:
 
  TEST PLAN REFERENCE:
 
  TOOLS AND PLATFORMS USED:
 
  USER IMPACT:
 
  REPRODUCING RATE:
 
  For FT PR, Please list reference mobile's behavior:

Comment 1

[sync-1]

Attachment: not ok mtklog

Comment 2

[sync-1]

Attachment: O7G18080AZ00.db

Comment 3

[sync-1]

Dear mozilla,
    This bug is related to bug 1106449[FFOS2.0][Woodduck][STK]The idle text was removed after excuting REFRESH-FCN.
    How could we listen the device power off or power on at gaia? If could ,we can remove the idle text.
    it's very nice of you to give us a better suggestion.
    Thanks.

Comment 4

[Josh Cheng [:josh]]

Hi Sean,
Could you please help to check the problem? Thanks!

Comment 5

[Sean Lee [:seanlee][:weilonge]]

Notes:
[<<SIM] commandNumber = 1 typeOfCommand = 28 commandQualifier = 0
[<<SIM] Received message from worker: {"commandNumber":1,"typeOfCommand":40,"commandQualifier":0,"rilMessageType":"stkcommand","options":{"text":"IdleText #1"},"rilMessageClientId":0}

[TR]    Received chrome message {"resultCode":0,"command":{"commandNumber":1,"typeOfCommand":40,"commandQualifier":0,"rilMessageType":"stkcommand","options":{"text":"IdleText #1"},"rilMessageClientId":0},"rilMessageClientId":0,"rilMessageToken":19,"rilMessageType":"sendStkTerminalResponse"}

[>>SIM] Received message from worker: {"rilMessageType":"stksessionend","rilMessageClientId":0}

Comment 6

[Josh Cheng [:josh]]

Hi Sean,
This is urgent bug. Please help to fix asap. Thanks!

Comment 7

[Sean Lee [:seanlee][:weilonge]]

Attachment: STK_PATCH_20141211_v1.tar.gz

Hi,

Changelist:
1. Remove Notifications from IDLE TEXT.

Please try it. Thank you.

Comment 8

[pengfei.huang]

Attachment: 12_12_mtklog_idle_power_off_on.zip

Hi Sean Lee,
   The idle text still stays on notification bar after power off/on.please check the log.

Comment 9

[Sean Lee [:seanlee][:weilonge]]

Attachment: STK_PATCH_20141212_v2.tar.gz

Hi Pengfei,

The solution is tested on my Woodduck v2.0m (black) .
In this attachment, I add some logs for debugging.
Please help to test it again. Thank you.

Comment 10

[Sean Lee [:seanlee][:weilonge]]

Attachment: STK_PATCH_20141212_v3.tar.gz

Please use this one for test. (ignore the previous one - STK_PATCH_20141212_v2.tar.gz)

Comment 11

[pengfei.huang]

Attachment: 12_12_mtklog_menu_select_exeception.zip

Hi Sean Lee,
   After merging patch STK_PATCH_20141212_v3.tar.gz ,I couldn't select STK menu item to do further testing, because the menu was stopping.And the same version code for bug1106454, I have to test them individually tomorrow. Thanks.

Comment 12

[Sean Lee [:seanlee][:weilonge]]

Hi Pengfei,

The crash is caused by that gsmLocationAreaCode can not be retrieved properly.
    DUMP('TESTCONN 4.1: ' + JSON.stringify(conn.voice.cell.gsmLocationAreaCode) );
    DUMP('TESTCONN 4.2: ' + JSON.stringify(conn.voice.cell.gsmCellId) );

Could you help to check you inserted the SIM card to Slot 1?
If not, please insert to slot 1 and test again.
IMHO, we could focus on the test case first, then multi-sim issue is the next item that we should resolve.

Thanks.

Comment 13

[Sean Lee [:seanlee][:weilonge]]

Attachment: STK_PATCH_20141212_v4.tar.gz

Fix the select item issue.
This can be for bug 1106450 and bug 1106454 as well.

Comment 14

[pengfei.huang]

Attachment: 12-13-mtklog-idle-power-off-on.zip

The idle text was removed after power off/on.Check the log.

Comment 15

[Sean Lee [:seanlee][:weilonge]]

Hi Pengfei,

There are couple things that need your comments:
1. Could you help to provide the specification for this test case?
2. Per the test result in your comment 14, is this test case PASS?

Thank you.

Comment 16

[pengfei.huang]

Hi Sean Lee,
   Sorry reply in a hurry.The test result is expected and test pass.
   I need to ask VAL Project Manager for the permission of the specification of this test case.
   Many thanks your support.

Comment 17

[Josh Cheng [:josh]]

Dear Sean,
Good job! Please process patch review. Thanks!

Comment 18

[Sean Lee [:seanlee][:weilonge]]

Attachment: PR for master

Hello Fernando,

Here is my PR for this bug. Please help to review it. Thank you.

Comment 19

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

Comment on attachment 8536407 [details] [review]
PR for master

LGTM - f+
Cancelling review. I leave you a question in Github :)

Comment 20

[Sean Lee [:seanlee][:weilonge]]

Attachment: STK_PATCH_20141216_v1.tar.gz

Hello Pengfei,

I need your help again for this bug. :P
I found a better way to wait for loading notifications.
Please help to test the trail patch and see whether there is any regression.

Thank you very much.

Comment 21

[pengfei.huang]

Ok,got it.

Comment 22

[pengfei.huang]

Hi Sean Lee, 
  I try your new patch and it works well. Test Pass.
How could I know what the event 'desktop-notification-resend' means? I can't find the event at developer.mozilla.org website.

Comment 23

[Sean Lee [:seanlee][:weilonge]]

Hi Pengfei,

Thanks a lot!

You can see the source code apps/system/js/notifications.js .
The event 'desktop-notification-resend' will be sent after mozResendAllNotifications .

Comment 24

[Sean Lee [:seanlee][:weilonge]]

Hello Fernando,

Thanks for your comments.
I've modified a better solution to wait for all notifications loaded.
Please help to review the PR again. Thank you.

Comment 25

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

Comment on attachment 8536407 [details] [review]
PR for master

Much better :)
Thanks for taking care of it.

Comment 26

[Sean Lee [:seanlee][:weilonge]]

gaia-try PASS: https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=72f4215678de

landed on https://github.com/mozilla-b2g/gaia/commit/efeabc1b3435eeea619734051f144a521cf483f8

Comment 27

[:gerard-majax]

(In reply to Sean Lee [:seanlee] from comment #23)
> Hi Pengfei,
> 
> Thanks a lot!
> 
> You can see the source code apps/system/js/notifications.js .
> The event 'desktop-notification-resend' will be sent after
> mozResendAllNotifications .

And it's not meant for this kind of use. This is not the proper way to fix this on master: please use showOnlyOnce field of mozbehavior. This patch would do the trick on 2.0, because we don't have a better way.

Comment 28

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

(In reply to Alexandre LISSY :gerard-majax from comment #27)

> And it's not meant for this kind of use. This is not the proper way to fix
> this on master: please use showOnlyOnce field of mozbehavior. This patch
> would do the trick on 2.0, because we don't have a better way.

Sounds nice, but where is it documented? I didn't see it as a Notification option [1]

[1] https://developer.mozilla.org/en-US/docs/Web/API/notification

Comment 29

[:gerard-majax]

(In reply to Fernando R. Sela (no CC, needinfo please) [:frsela] from comment #28)
> (In reply to Alexandre LISSY :gerard-majax from comment #27)
> 
> > And it's not meant for this kind of use. This is not the proper way to fix
> > this on master: please use showOnlyOnce field of mozbehavior. This patch
> > would do the trick on 2.0, because we don't have a better way.
> 
> Sounds nice, but where is it documented? I didn't see it as a Notification
> option [1]
> 
> [1] https://developer.mozilla.org/en-US/docs/Web/API/notification

I'm not sure this should be exposed on MDN, but it's documented in bug 1100876.

Maybe Chris can help us here.

Comment 30

[Josh Cheng [:josh]]

Hi Sean,
Can you raise PR for 2.0M since the schedule is tight for partner device launch? 
The discussion here might leads rework of master patch if necessary.
Thanks!

Comment 31

[Michael Henretty [:mikehenrty][:mhenretty]]

Agreed we should rework the master patch to use showOnlyOnce since scenarios like this are exactly what that was designed for. 

Also, I don't think we can document showOnlyOnce on the main web notification MDN page, since it's not an official part of the web notification spec (ie. desktop web doesn't have a need for reboot/resending notifications right now). Maybe we should document this moz only feature here: https://developer.mozilla.org/en-US/docs/Web/API/Notification/Using_Web_Notifications

Comment 32

[Sean Lee [:seanlee][:weilonge]]

Hi Alexandre,

Thanks for your suggestions.
I will create a new bug for reworking the patch with showOnlyOnce for master as soon. ( I will take it. )
As Josh mentioned, this patch will apply to v2.0m because it needs for partner device .

Please ni me if you still have other concern.

Comment 33

[Sean Lee [:seanlee][:weilonge]]

v2.0m: https://github.com/mozilla-b2g/gaia/commit/b81fea3fc351a91f8aead499d514584f67b06b5a

Comment 34

[Sean Lee [:seanlee][:weilonge]]

Create a new bug 1112978 for reworking showOnlyOnce for IDLE MODE TEXT notifications.

Comment 35

[Chris Mills [:cmills]]

(In reply to Alexandre LISSY :gerard-majax from comment #29)
> (In reply to Fernando R. Sela (no CC, needinfo please) [:frsela] from
> comment #28)
> > (In reply to Alexandre LISSY :gerard-majax from comment #27)
> > 
> > > And it's not meant for this kind of use. This is not the proper way to fix
> > > this on master: please use showOnlyOnce field of mozbehavior. This patch
> > > would do the trick on 2.0, because we don't have a better way.
> > 
> > Sounds nice, but where is it documented? I didn't see it as a Notification
> > option [1]
> > 
> > [1] https://developer.mozilla.org/en-US/docs/Web/API/notification
> 
> I'm not sure this should be exposed on MDN, but it's documented in bug
> 1100876.
> 
> Maybe Chris can help us here.

Can the NotificationBehaviour dictionary options be used as notification options when creating a new notification, as controllable by 3rd party app developers, or is it more of a system level Gaia thing? I don't really fully understand what they are, but once I understand better, I can make more of a decision.

Comment 36

[sync-1]

Dear DengWei,
   we can close this bug. see bug851802.
   Many thanks.