Notifications not re-displayed after reboot

VERIFIED FIXED in 2.2 S14 (12june)

Status

VERIFIED FIXED
4 years ago
3 years ago

People

(Reporter: gerard-majax, Assigned: gerard-majax)

Tracking

({regression})

unspecified
2.2 S14 (12june)
ARM
Gonk (Firefox OS)
regression
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.5+, b2g-master verified)

Details

(Whiteboard: [systemsfe])

Attachments

(1 attachment)

pr
46 bytes, text/x-github-pull-request
alive
: review+
alive
: feedback+
Details | Review | Splinter Review
(Assignee)

Description

4 years ago
I have just spotted on current master that notifications are not retriggered after reboot :(
(Assignee)

Comment 1

4 years ago
Checked on my device, notifications are correctly stored.
(Assignee)

Comment 2

4 years ago
Calling |navigator.mozChromeNotifications.mozResendAllNotifications(function(e) { console.debug(e); });| by hand from WebIDE in the scope of system app makes it working. So it means this code is probably not called during the boot time anymore?

Maybe a fallout from bug 1094759 ?
Flags: needinfo?(alive)
(Assignee)

Comment 3

4 years ago
Ok, so the code from notification_screen.js used to wait for the "load" event to trigger the resend mechanisms. This seems not to work anymore now.
Summary: Notifications lost after reboot → Notifications not re-displayed after reboot
(Assignee)

Comment 4

4 years ago
Removing the event listener on the load event fixes ...
Cool, thanks for filing bug. So does it mean window.onload is not triggered anymore?
Flags: needinfo?(alive)
I will check what happened tomorrow morning; but you are welcome to submit patch.
(Assignee)

Comment 7

4 years ago
Alive, I suspect the logic was good when it used the 'load' event because the js was loaded from a <script> statement. Now, it's being loaded by the module logic.
(Assignee)

Updated

4 years ago
Keywords: qaurgent, regressionwindow-wanted
(Assignee)

Comment 8

4 years ago
Created attachment 8616483 [details] [review]
pr

Alive, this approach should be enough, at least it works properly locally. Do you have any opinion, or suggestion to share?
Attachment #8616483 - Flags: feedback?(alive)
Comment on attachment 8616483 [details] [review]
pr

No, it's the same and as good as I think. Thank you!
Attachment #8616483 - Flags: feedback?(alive) → feedback+
blocking-b2g: spark? → spark+
(Assignee)

Updated

4 years ago
Depends on: 1172426
(Assignee)

Comment 10

4 years ago
Comment on attachment 8616483 [details] [review]
pr

Here we go, fix with tests. I'm also fixing the tests because:
 - we were lacking some this.sinon.tick() to properly deal with mozSettings
 - the test 'desktop-notification-resend not sent' was just wrong since we early return when the setting is set to false
Attachment #8616483 - Flags: review?(alive)
(Assignee)

Updated

4 years ago
Whiteboard: [systemsfe]
Target Milestone: --- → 2.2 S14 (12june)
(Assignee)

Updated

4 years ago
Depends on: 1172548
(Assignee)

Comment 11

4 years ago
So it looks like Gij12 is failing, but I'm not able to investigate for now because running those tests locally do not work anymore ...
(Assignee)

Comment 12

4 years ago
Finally reproduced locally, then rebasing on master and it's not reproducing locally anymore?
(Assignee)

Comment 13

4 years ago
Ok, it's reproducing and only with my patch. Alberto, since you wrote the test, do you have any idea what might be going wrong here ?

So far I have been checking and I don't see anything obvious :(
Flags: needinfo?(apastor)
May be we need to wait for 'system.waitForFullyLoaded()' in the setup, now that the new bootstrap has landed?
Flags: needinfo?(apastor)
(Assignee)

Comment 15

4 years ago
(In reply to Alberto Pastor [:albertopq] from comment #14)
> May be we need to wait for 'system.waitForFullyLoaded()' in the setup, now
> that the new bootstrap has landed?

Nope that's already present. Forcing the setting |notifications.resend| to false gets me green. Given my observation, what happens is that with this patch the call to resend all stored notifications will be delayed compared to before the landing of new bootstrap. In this test, this makes it triggered after the call to |new Notification|, thus messing up.
Comment on attachment 8616483 [details] [review]
pr

Thank you Alex.
Attachment #8616483 - Flags: review?(alive) → review+
(Assignee)

Comment 17

4 years ago
And it's all green.
(Assignee)

Comment 18

4 years ago
https://github.com/mozilla-b2g/gaia/commit/eeaacf041d025f53849b80b85de3736ba399782c
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
blocking-b2g: spark+ → 2.5+
status-b2g-v2.5: --- → fixed
status-b2g-master: --- → fixed
status-b2g-v2.5: fixed → ---
This issue is verified fixed on Aries and Flame. However I noticed that notifications on lockscreen upon reboot are NOT immediately shown. It takes several seconds to show which is a regression from Flame 2.2. NI myself to bug this tomorrow.

Device: Aries (dogfood debug build)
BuildID: 20150721153949
Gaia: 805cf546729ba742bf23febda52970fcb35c0e8f
Gecko: 512c7e8f0030
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 42.0a1 (2.5 Master) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0

Device: Flame (KK 319MB full flashed)
BuildID: 20150721010202
Gaia: 4fe0507781f3ed56c8ae5e66dd9489165d1ff68e
Gecko: 3a4bfa5d2d02
Gonk: 41d3e221039d1c4486fc13ff26793a7a39226423
Version: 42.0a1 (2.5 Master) 
Firmware Version: v18D nightly v4
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
status-b2g-master: fixed → verified
Flags: needinfo?(pcheng)
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Filed bug 1186568 for issue observed on comment 19
Flags: needinfo?(pcheng)
You need to log in before you can comment on or make changes to this bug.