1112971 - Can't remove cell broadcast notification from utility tray

Status: RESOLVED FIXED

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

Description

[Sherman Chen [:chens] (inactive)]

Attachment: Pull request for 2.0m

Precondition: Turn lockscreen on and received cell broadcast message

Steps to reproduce:
1. Unlock device and pull down utility tray
2. Click on cell broadcast message notification
3. Cell broadcast message shows up

Actual:
4. Notification for cell broadcast message still in utility tray

Expected
4. Notification for cell broadcast message in utility tray is removed

Comment 1

[Kevin Grandon :kgrandon]

Does this only happen in 2.0m? Is master unaffected?

Comment 2

[Kevin Grandon :kgrandon]

Comment on attachment 8538223 [details] [review]
Pull request for 2.0m

I don't have a 2.0m build around, but looking at the code only seems fine to me. I left a comment on github, please let me know why, and comment in the code if needed.

Also please be sure this is fixed in master, I assume it is since we're only doing the fix for 2.0m?

Comment 3

[Sherman Chen [:chens] (inactive)]

Attachment: Pull request to 2.0m

PR updated and should make this more clear.
And yes this is for 2.0m only, will provide patch to master later.

Comment 4

[Sherman Chen [:chens] (inactive)]

Attachment: Pull request to master

Comment 5

[Kevin Grandon :kgrandon]

Comment on attachment 8539069 [details] [review]
Pull request to master

You should probably write a MarionetteJS test for this. If you can't, please open a follow-up but to write a test and I could help.

Comment 6

[Sherman Chen [:chens] (inactive)]

2.0m: https://github.com/mozilla-b2g/gaia/commit/1f11c58d7ad7b1eb2e08db98154ff4f20aa760ed

Comment 7

[Sherman Chen [:chens] (inactive)]

Comment on attachment 8539069 [details] [review]
Pull request to master

Hi Kevin, I've add MarionetteJS test for it, would you take a look and give some feedback? thanks!

Comment 8

[Kevin Grandon :kgrandon]

Comment on attachment 8539069 [details] [review]
Pull request to master

Assuming that the test fails if the patch is reverted, this looks good to me. Thanks for the test!

Comment 9

[Sherman Chen [:chens] (inactive)]

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

Comment 10

[Gregor Wagner [:gwagner]]

Sherman, please request uplift approval. We should fix this in 2.1 as well.

Comment 11

[Sherman Chen [:chens] (inactive)]

Comment on attachment 8539069 [details] [review]
Pull request to master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Pre-existed situation, we didn't remove notification from utility tray when click on cell broadcast notification.
[User impact] if declined: Cell broadcast notification not able to remove and notification count not updated.
[Testing completed]: Manual and marionette test.
[Risk to taking this patch] (and alternatives if risky): Trivial patch, should be low risk.
[String changes made]: None

Comment 12

[bhavana bajaj [:bajaj]]

Brian, I'd like QA to verify this issue on 2.2 before we uplift (given the low risk here).

Comment 13

[Brian Huang [:brianhuang][:brhuang]]

(In reply to bhavana bajaj [:bajaj] from comment #12)
> Brian, I'd like QA to verify this issue on 2.2 before we uplift (given the
> low risk here).

No problem. 

Eric, please help to have someone to verify this. Thank you.

Comment 14

[Brogan Zumwalt [Inactive]]

Cannot run required automation test to verify on our end at this time due to external software issues.

Comment 15

[Eric Chang [:ericcc] [:echang]]

v2.2 Emulator building fail, trying to get it running.
Keep ni for reminding me.

Comment 16

[Eric Chang [:ericcc] [:echang]]

[Verified Okay] V2.2 + emulator-kk http://youtu.be/6VMnZHqCehA

Hi Josh,
What's woodduck config? V2.0 + JB or KK? I can try tomorrow.

Comment 17

[Eric Chang [:ericcc] [:echang]]

Cannot find V2.0m on emulator-jb, emulator-kk or emulator-x86... etc, not likely to test.

ericcc@BP6375:~/repo/mozilla-b2g/B2G-1112971$ BRANCH=v2.0m ./config.sh emulator-jb
Get git://github.com/mozilla-b2g/b2g-manifest
remote: Counting objects: 1999, done.
remote: Compressing objects: 100% (803/803), done.
remote: Total 1999 (delta 1209), reused 1956 (delta 1173)
Receiving objects: 100% (1999/1999), 915.48 KiB | 393.00 KiB/s, done.
Resolving deltas: 100% (1209/1209), done.
From git://github.com/mozilla-b2g/b2g-manifest
 * [new branch]      master     -> origin/master
 * [new branch]      revert-203-bug1025788-v2 -> origin/revert-203-bug1025788-v2
 * [new branch]      revert-226-v1.3t -> origin/revert-226-v1.3t
 * [new branch]      v1-train   -> origin/v1-train
 * [new branch]      v1.0.0     -> origin/v1.0.0
 * [new branch]      v1.0.1     -> origin/v1.0.1
 * [new branch]      v1.1.0hd   -> origin/v1.1.0hd
 * [new branch]      v1.2       -> origin/v1.2
 * [new branch]      v1.2f      -> origin/v1.2f
 * [new branch]      v1.3       -> origin/v1.3
 * [new branch]      v1.3t      -> origin/v1.3t
 * [new branch]      v1.4       -> origin/v1.4
 * [new branch]      v2.0       -> origin/v2.0
 * [new branch]      v2.1       -> origin/v2.1
 * [new branch]      v2.1s      -> origin/v2.1s
 * [new branch]      v2.2       -> origin/v2.2
 * [new tag]         B2G_1_0_1_20130213094222 -> B2G_1_0_1_20130213094222
 * [new tag]         B2G_1_1_0_hd_20130530182315 -> B2G_1_1_0_hd_20130530182315
 * [new tag]         B2G_1_1_0_hd_20130530182315_BASE -> B2G_1_1_0_hd_20130530182315_BASE
 * [new tag]         closing-nightly -> closing-nightly
error: in `init -u git://github.com/mozilla-b2g/b2g-manifest -b v2.0m -m emulator-jb.xml`: revision v2.0m in manifests not found
Repo sync failed

Comment 18

[Kai-Zhen Li [:seinlin][:kli]]

Eric, Could you use https://github.com/mozillatw/B2G.git to config emulator for v2.0m?

Comment 19

[Eric Chang [:ericcc] [:echang]]

Attachment: Screenshot from 2015-01-21 15:12:25.png

Not able to run this on emulator, settings has 9 sim cards in it, but signal bars are missing, error from adb logs.. with either emulator-kk or emulator-jb.
Still trying to get it work.

01-21 02:12:12.605    45   343 E         : GetPrevFBAcquireFd: FB acquire fence is invalid!
01-21 02:12:12.615   524   524 E GeckoConsole: [JavaScript Error: "TypeError: desc is undefined" {file: "app://settings.gaiamobile.org/js/carrier_iccs.js" line: 147}]
01-21 02:12:12.615    45   343 E         : GetPrevFBAcquireFd: FB acquire fence is invalid!
01-21 02:12:12.615    45   343 E         : GetPrevFBAcquireFd: FB acquire fence is invalid!

Comment 20

[bhavana bajaj [:bajaj]]

Sherman, do you have any other ideas for QA to test this ? Given the low risk patch with tests I am fine to take this but also do not want to deal with any fallouts here given the lack of testing. So what's your opinion here on living with this for 2.1 or can you in some way get me more confidence on the patch ?

Comment 21

[Sherman Chen [:chens] (inactive)]

I think there are two possible options for testing this patch. Given we are trying to land this on 2.1, we could ask QA to verify 2.1 with this patch (attachment 8539069 [details] [review]), that would give us the most clear view on it. Or if we want to verify this on 2.0m, we can verify on 2.0 with this patch (attachment 8539042 [details] [review]).

Comment 22

[Eric Chang [:ericcc] [:echang]]

Let me try again, thanks for the info.
(In reply to Sherman Chen [:chens] from comment #21)
> I think there are two possible options for testing this patch. Given we are
> trying to land this on 2.1, we could ask QA to verify 2.1 with this patch
> (attachment 8539069 [details] [review]), that would give us the most clear view on
> it. Or if we want to verify this on 2.0m, we can verify on 2.0 with this
> patch (attachment 8539042 [details] [review]).

Comment 23

[Eric Chang [:ericcc] [:echang]]

This is on v2.1 emulator + patch 8539069, notification is not shown, not sure if it is expected.
http://youtu.be/vNCbQy2KnHM

Comment 24

[bhavana bajaj [:bajaj]]

Comment on attachment 8539069 [details] [review]
Pull request to master

Thanks for testing, looks good to land now

Comment 25

[Ryan VanderMeulen [:RyanVM]]

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

Comment 26

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

Adding keyword to exclude this from our queries (see comment 14).