Closed Bug 1112971 Opened 6 years ago Closed 6 years ago

Can't remove cell broadcast notification from utility tray

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(b2g-v2.0 wontfix, b2g-v2.0M fixed, b2g-v2.1 fixed, b2g-v2.1S fixed, b2g-v2.2 verified)

RESOLVED FIXED
2.2 S3 (9jan)
Tracking Status
b2g-v2.0 --- wontfix
b2g-v2.0M --- fixed
b2g-v2.1 --- fixed
b2g-v2.1S --- fixed
b2g-v2.2 --- verified

People

(Reporter: chens, Assigned: chens)

References

Details

(Keywords: verifyme)

Attachments

(4 files)

Attached file 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
Attachment #8538223 - Flags: review?(kgrandon)
Does this only happen in 2.0m? Is master unaffected?
Flags: needinfo?(shchen)
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?
Attachment #8538223 - Flags: review?(kgrandon) → review+
Attached file 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.
Flags: needinfo?(shchen)
Attachment #8539042 - Flags: review?(kgrandon)
Attached file Pull request to master
Attachment #8539069 - Flags: review?(kgrandon)
Attachment #8539042 - Flags: review?(kgrandon) → review+
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.
Attachment #8539069 - Flags: review?(kgrandon) → review+
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!
Attachment #8539069 - Flags: feedback?(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!
Attachment #8539069 - Flags: feedback?(kgrandon) → feedback+
master: https://github.com/mozilla-b2g/gaia/commit/1426bbf5480cb8d6ec35ada9132242ff2127453a
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Sherman, please request uplift approval. We should fix this in 2.1 as well.
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
Attachment #8539069 - Flags: approval-gaia-v2.1?
Keywords: verifyme
Brian, I'd like QA to verify this issue on 2.2 before we uplift (given the low risk here).
Flags: needinfo?(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.
Flags: needinfo?(brhuang) → needinfo?(echang)
Cannot run required automation test to verify on our end at this time due to external software issues.
QA Whiteboard: [QAnalyst-Triage?][QAnalyst-verify-]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?][QAnalyst-verify-] → [QAnalyst-Triage+][QAnalyst-verify-]
Flags: needinfo?(ktucker)
v2.2 Emulator building fail, trying to get it running.
Keep ni for reminding me.
[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.
Flags: needinfo?(echang) → needinfo?(jocheng)
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
Eric, Could you use https://github.com/mozillatw/B2G.git to config emulator for v2.0m?
Flags: needinfo?(jocheng) → needinfo?(echang)
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!
Flags: needinfo?(echang)
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 ?
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]).
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]).
This is on v2.1 emulator + patch 8539069, notification is not shown, not sure if it is expected.
http://youtu.be/vNCbQy2KnHM
Comment on attachment 8539069 [details] [review]
Pull request to master

Thanks for testing, looks good to land now
Attachment #8539069 - Flags: approval-gaia-v2.1? → approval-gaia-v2.1+
Adding keyword to exclude this from our queries (see comment 14).
QA Whiteboard: [QAnalyst-Triage+][QAnalyst-verify-] → [QAnalyst-Triage?][QAnalyst-verify-], [QAExclude]
Flags: needinfo?(jmercado)
QA Whiteboard: [QAnalyst-Triage?][QAnalyst-verify-], [QAExclude] → [QAnalyst-Triage+] [QAExclude]
Flags: needinfo?(jmercado)
You need to log in before you can comment on or make changes to this bug.