1073032 - New notification indicator stays on even if you cleared all notifications

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)

Description

[Anthony Ricaud (:rik)]

Steps to reproduce:
1. Make sure you have no notifications in the notification tray
2. Receive a call and don't answer it
3. Open the phone app
4. Go to call log

Expected:
The blue new notification indicator should be gone

Actual:
Notification indicator is still there.

Note:
If you open the notification tray after that, you can see that there are no notifications in the notification tray.

Comment 1

[Julien Wajsberg [:julienw]]

I'd say the new notification tray is only in v2.2. Right Michael?

Comment 2

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

This probably has something to do with the way the attention screen adds notifications. We shouldn't show the ambient notification indicator for the attention screen, so that is the bug.


(In reply to Julien Wajsberg [:julienw] from comment #1)
> I'd say the new notification tray is only in v2.2. Right Michael?

The ambient indicator is in 2.1, but I don't think the attention window made it in for that. In any case, QA wanted for branch checks on 2.1 and 2.2 only.

Comment 3

[Pi Wei Cheng [:piwei] (inactive)]

(In reply to Michael Henretty [:mhenretty] from comment #2)
> In any case, QA wanted for branch checks on 2.1 and 2.2 only.

This issue is reproducible on Flame 2.2. After accessing Call Log at step 4, the blue notification bar on top of screen remains until user pulls down the utility tray (and see there is 0 notification).

Device: Flame
BuildID: 20140925134737
Gaia: a06714c555ca7068545f10b4437a16c14cd8e7f5
Gecko: 9e3d649b80a2
Version: 35.0a1 (2.2 Master)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

----------

This issue is NOT reproducible on Flame 2.1. After accessing Call Log at step 4, the blue notification bar on top of screen disappears.

Device: Flame
BuildID: 20140925140043
Gaia: 86905e14c3ff06a0e6952ba635b6066ad2eea6b4
Gecko: bc8aad860662
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/35.0 Firefox/34.0

Comment 4

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

Alberto, can you take a look here?

Comment 5

[Alberto Pastor [:albertopq]]

Sure thing.

Comment 6

[Alberto Pastor [:albertopq]]

Attachment: Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/24464

Comment 7

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

[Blocking Requested - why for this release]:
Blocks a blocker.

Comment 8

[Alberto Pastor [:albertopq]]

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

Comment 9

[Gregor Wagner [:gwagner]]

see comment 7

Comment 10

[Julien Wajsberg [:julienw]]

We need an approval anyway, Alberto.

Comment 11

[Alberto Pastor [:albertopq]]

2.1 is still not affected, until Bug 1065519 lands. That's the reason I was waiting for requesting approval. Should I request approval anyways? Thanks Julien!

Comment 12

[Julien Wajsberg [:julienw]]

Yeah, please do, both will be uplifted in the same time :)

Comment 13

[Alberto Pastor [:albertopq]]

Comment on attachment 8495789 [details] [review]
Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/24464

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): 1065519 
[User impact] if declined: Notifications not cleared correctly
[Testing completed]: Added unit tests for the affected use cases
[Risk to taking this patch] (and alternatives if risky): Simple fix. Low risk
[String changes made]:-

Comment 14

[Ryan VanderMeulen [:RyanVM]]

v2.1: https://github.com/mozilla-b2g/gaia/commit/b0fe1073010b0589d884c28673072b5493f3bc49

Comment 15

[Yeojin Chung [:YeojinC]]

This issue is verified on Flame 2.2:

Flame 2.2 KitKat Base (319mb)(Full Flash)

Environmental Variables:
Device: Flame 2.2 Master
BuildID: 20141001040205
Gaia: 0e280591881d44b80f456bc27e12d9114c218868
Gecko: 14665b1de5ee
Gonk: 2c909e821d107d414f851e267dedcd7aae2cebf
Version: 35.0a1 (2.2 Master)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

The blue new notification indicator disappears after accessing call long
===============================================
This issue still reproduces on Today's Flame 2.1(Nithglty):

Flame 2.1 KitKat Base (319mb)(Full Flash)

Environmental Variables:
Device: Flame 2.1
BuildID: 20141002000202
Gaia: 94dcc25f2e34a4900ea58310c26be52bcb089161
Gecko: baaa0c3ab8fd
Gonk: 2c909e821d107d414f851e267dedcd7aae2cebf
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

2.1 was uplifted yesterday. Leaving veryfime keyword to verify tomorrow.

Comment 16

[Yeojin Chung [:YeojinC]]

This issue is verified on Flame 2.1 as well:

Flame 2.1 KitKat Base (319mb)(Full Flash)

Environmental Variables:
Device: Flame 2.1
BuildID: 20141003000203
Gaia: 9861c61ec302fb0316c753a2e1c0f592180515fd
Gecko: da68900d1c66
Gonk: 2c909e821d107d414f851e267dedcd7aae2cebf
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

The blue new notification indicator disappears after accessing call long

Comment 17

[Alberto Pastor [:albertopq]]

Attachment: TESTS: Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/25215

Comment 18

[Etienne Segonzac (:etienne)]

Comment on attachment 8506058 [details] [review]
TESTS: Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/25215

lots of small comments so worth an extra review round but this is a very welcome clean-up :)

Comment 19

[Alberto Pastor [:albertopq]]

Comment on attachment 8506058 [details] [review]
TESTS: Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/25215

Totally agree with the comments. Just sent an update :)
Thanks for the review!

Comment 20

[Etienne Segonzac (:etienne)]

Comment on attachment 8506058 [details] [review]
TESTS: Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/25215

r=me with the last comments addressed... and a green try build (had issues locally with the last test)

Comment 21

[Alberto Pastor [:albertopq]]

It seems that the last test is failing as you mentioned, but I cannot repro it locally. Could you please send me the log? Thanks!

Comment 22

[Etienne Segonzac (:etienne)]

(In reply to Alberto Pastor [:albertopq] from comment #21)
> It seems that the last test is failing as you mentioned, but I cannot repro
> it locally. Could you please send me the log? Thanks!

Well, this drove me a little crazy, especially since it works with VERBOSE=1.
I think it's a race, but it might be a gecko race.

The best and safest fix I have is to add this in the setup()
```
  var sys = new System(client);
  sys.waitForStartup();
```

(with the correct require)

We'll watch out for intermittent but looks like this completely fixes the issue.

Comment 23

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

Alberto, how are we doing here? Did Etienne's suggestion fix the tests?

Comment 24

[Alberto Pastor [:albertopq]]

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

Finally... green!