Can't remove cell broadcast notification from utility tray

RESOLVED FIXED in 2.2 S3 (9jan)

Status

RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: chens, Assigned: chens)

Tracking

({verifyme})

unspecified
2.2 S3 (9jan)
ARM
Gonk (Firefox OS)
verifyme

Firefox Tracking Flags

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

Details

Attachments

(4 attachments)

Created attachment 8538223 [details] [review]
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+
(Assignee)

Comment 3

4 years ago
Created attachment 8539042 [details] [review]
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)
(Assignee)

Updated

4 years ago
status-b2g-v2.0: --- → affected
status-b2g-v2.0M: --- → affected
status-b2g-v2.1: --- → affected
status-b2g-v2.2: --- → affected
(Assignee)

Comment 4

4 years ago
Created attachment 8539069 [details] [review]
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+
(Assignee)

Comment 7

4 years ago
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+
(Assignee)

Comment 9

4 years ago
master: https://github.com/mozilla-b2g/gaia/commit/1426bbf5480cb8d6ec35ada9132242ff2127453a
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Assignee)

Updated

4 years ago
status-b2g-v2.2: affected → fixed
Sherman, please request uplift approval. We should fix this in 2.1 as well.
(Assignee)

Comment 11

4 years ago
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)

Updated

4 years ago
Blocks: 1080481
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.
status-b2g-v2.2: fixed → verified
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?

Updated

4 years ago
Flags: needinfo?(jocheng) → needinfo?(echang)
Created attachment 8552321 [details]
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!
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 ?
(Assignee)

Comment 21

4 years ago
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+
v2.1: https://github.com/mozilla-b2g/gaia/commit/ee79bfe018bf600a0daa2870e5df32e8d7b82979
status-b2g-v2.0: affected → wontfix
status-b2g-v2.1: affected → fixed
Target Milestone: --- → 2.2 S3 (9jan)

Updated

4 years ago
status-b2g-v2.1S: --- → fixed
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.