1112401 - [Notifications] Opening the Power Saving Mode notification takes user to homescreen

Status: VERIFIED FIXED

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

Description

[KTucker [:KTucker][Inactive 3/4/2016]]

When the user taps "Open" on the "Power Saving Mode" notification on the lockscreen, they will be taken to the homescreen.

Prerequisite: Power saving mode and lockscreen are enabled.

Repro Steps:
1)  Updated Flame to Build ID: 20141215040201
2)  Restart the phone so that the "Power Saving Mode" notification appears on the lockscreen. 
3) Swipe right to left on the notification and tap on "Open".

Actual:
The user will be redirected to the homescreen.

Expected:
The user should be taken to the battery section in settings. 

Environmental Variables
Device: Flame 2.2 Master (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141216040205
Gaia: af3d2f89f391c92667e04676fc0ac971e6021bb7
Gecko: a3030140d5df
Gonk: e5c6b275d77ca95fb0f2051c3d2242e6e0d0e442
Version: 37.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Notes:
Repro frequency: 5/5 100%
See attached: video

Comment 1

[KTucker [:KTucker][Inactive 3/4/2016]]

Adding qawanted for branch check.

Comment 2

[Chris Kreinbring [:CKreinbring]]

I've found that the key to reproing the bug is to set the "Turn on automatically" setting to a threshold higher than the device's current battery level.  This will cause the notification to appear when the device is restarted.

Able to repro on Flame 2.2 engineering with shallow flash.
Actual result: After tapping the Open button on the power save notification, the homescreen appears instead of the Battery page in the Settings.

BuildID: 20141217035735
Gaia: d22dfece04fc00457e8369c660c11f945b088d2f
Gecko: cb8ad2251c09
Platform Version: 37.0a1
Firmware Version: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

--------------------------------------------------------------------------------------------------------

For 2.1, the user must also unplug their device from a charging source while on the lockscreen in order for the notification to appear.
Unable to repro on Flame 2.1 engineering with shallow flash.
Actual result: After tapping the Open button on the power save notification, the Battery page will appear.

BuildID: 20141216123009
Gaia: b868736aec314c5e69c1329249f170c7b07802ea
Gecko: 7b161e9fe25e
Platform Version: 34.0
Firmware Version: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 3

[Joshua Mitchell (Inactive)]

Blocking on 2.2; regression, broken functionality (of error message)

Comment 4

[howie [:howie]]

Triage: blocking.

Comment 5

[(inactive after 6/18) Alive Kuo [:alive]]

https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/power_save.js#L105
Could you take this issue from here? Thx.

Comment 6

[Chris Kreinbring [:CKreinbring]]

Regression window
Last working
BuildID: 20141204080450
Gaia: 0462090a99093049add9268d14cbc7e44c1d1ccb
Gecko: 29d086b32a26
Platform Version: 37.0a1
Firmware Version: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

First broken
BuildID: 20141205035850
Gaia: 529c5fcd234ffd108b57629673ca97c2ef73376d
Gecko: 9eda28f821a2
Platform Version: 37.0a1
Firmware Version: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Working Gaia / Broken Gecko = No repro
Gaia: 0462090a99093049add9268d14cbc7e44c1d1ccb
Gecko: 9eda28f821a2
Broken Gaia / Working Gecko = Repro
Gaia: 529c5fcd234ffd108b57629673ca97c2ef73376d
Gecko: 29d086b32a26
Gaia pushlog: https://github.com/mozilla-b2g/gaia/compare/0462090a99093049add9268d14cbc7e44c1d1ccb...529c5fcd234ffd108b57629673ca97c2ef73376d


B2G Inbound
Last working
BuildID: 20141204082246
Gaia: a208afd47dd0f9633068ada61bc9829b5a2bed7a
Gecko: 643a2477719e
Platform Version: 37.0a1
Firmware Version: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

First broken
BuildID: 20141204090247
Gaia: 256ffaa7ae85bc95cae269482fae7314fdbf2cc6
Gecko: 07a5a56ccf69
Platform Version: 37.0a1
Firmware Version: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Working Gaia / Broken Gecko = No repro
Gaia: a208afd47dd0f9633068ada61bc9829b5a2bed7a
Gecko: 07a5a56ccf69
Broken Gaia / Working Gecko = Repro
Gaia: 256ffaa7ae85bc95cae269482fae7314fdbf2cc6
Gecko: 643a2477719e
Gaia pushlog: https://github.com/mozilla-b2g/gaia/compare/a208afd47dd0f9633068ada61bc9829b5a2bed7a...256ffaa7ae85bc95cae269482fae7314fdbf2cc6

Comment 7

[Joshua Mitchell (Inactive)]

Bug caused by patch to Bug 1095109 - 

Since this bug already has someone 'assigned' - no further action at this time.

Comment 8

[Evan Tseng [:evanxd]]

Looks like this bug is caused by the Notification[1] api.

We used the Notification api in bug 1095109. But the Notification's `click` listener in [2] could not be triggered if we create the notification object too early. That's why we could not go to Battery page in settings app after click the notification.

If we used this patch[3] to delay 5 seconds(maybe less) to create the notification object, we could not reproduce this bug with following the STR in this bug[4].

Hi Gregor,

Do you know that is the Notification issue(too early create notification object and the `click` listener just could not work) a known issue? And how do you think we fix this bug with fixing the Notification bug and no gaia change?

Thanks.

[1]: https://developer.mozilla.org/en-US/docs/Web/API/notification
[2]: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/power_save.js#L119
[3]: https://github.com/evanxd/gaia/commit/d870fb26186cc0344ede9c800012874877566b32
[4]: Comment 0

Comment 9

[Gregor Wagner [:gwagner]]

NI Michael for comment 8

Comment 10

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

Still looking at this, so not clearing the ni?.

(In reply to Evan Tseng [:evanxd][:愛聞插低] from comment #8)
> Do you know that is the Notification issue(too early create notification
> object and the `click` listener just could not work) a known issue? And how
> do you think we fix this bug with fixing the Notification bug and no gaia
> change?

This is not a known issue, but I can verify this is what seems to be happening. If this is true, we should fix this in the notification API rather than a workaround in gaia. I'll look at it more tomorrow.

Comment 11

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

Taking for now.

Comment 12

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

The problem is that power_save.js sends it's notification on boot-up even before mozChromeNotifications has a chance to resend all existing notifications. So the notification gets resent almost immediately, and the notification click observer gets overwritten with null [1].

We can fix this in Gaia for the power save notifications by specifying showOnlyOnce for the mozbehavior. I think we should do this anyway because the power save notification will always get recreated on boot-up by power_save.js, making resending the existing one redundant and confusing.

We also need to fix this in Gecko to prevent notifications that are created too early from getting their observers (ie. event handlers) from getting overwritten by the resending logic. I'll look into that after the Gaia fix, probably in a followup.

1.) http://hg.mozilla.org/mozilla-central/file/9155e0c35233/dom/notification/ChromeNotifications.js#l66

Comment 13

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

Attachment: [gaia PR] don't resend power save notifications

Comment 14

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

Attachment: [Gecko patch] Don't resend live notifications

Here's the gecko path. Let's get some feedback from Alexandre before continuing with tests.

Note, these two patches [gecko and gaia] are completely independent and while they both fix the problem in this bug, they each are for difference reasons and can be landed separately.

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=b81a6c8c0b70

Comment 15

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8546921 [details] [review]
[gaia PR] don't resend power save notifications

Excellent.

Comment 16

[:gerard-majax]

(In reply to Michael Henretty [:mhenretty] from comment #14)
> Created attachment 8547118 [details] [diff] [review]
> [Gecko patch] Don't resend live notifications
> 
> Here's the gecko path. Let's get some feedback from Alexandre before
> continuing with tests.
> 
> Note, these two patches [gecko and gaia] are completely independent and
> while they both fix the problem in this bug, they each are for difference
> reasons and can be landed separately.
> 
> https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=b81a6c8c0b70

Isn't it the solution we considered a workaround in bug 1088224 ?

Comment 17

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

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

this will need uplift. note, if we want to fix this for 2.0m, we will need a different solution.

Comment 18

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

After speaking to Alexandre on IRC, it seems the workaround we had in bug 1088224 addressed this exact issue. It was for 2.1 only, so we should just backport that. I filed bug 1120598 to track that.

Comment 19

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

Comment on attachment 8547118 [details] [diff] [review]
[Gecko patch] Don't resend live notifications

[Bug caused by] (feature/regressing bug #):
bug 1095109.

[User impact] if declined:
In certain scenarios, clicking on the power save notification will do nothing.

[Testing completed]:
Manual testing. Added a unit test.

[Risk to taking this patch] (and alternatives if risky):
Very low risk since this just changes the mozbehavior field of the power save notification.

[String changes made]: none.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/8b868120ac55de6e0e199acb7b62e89cf4c2cf04

Comment 21

[Sherman Chen [:chens] (inactive)]

(In reply to Michael Henretty [:mhenretty] from comment #17)
> master:
> https://github.com/mozilla-b2g/gaia/commit/
> 7c5b27cad370db377b18a742d3f3fdb0070e899f
> 
> this will need uplift. note, if we want to fix this for 2.0m, we will need a
> different solution.

Hi Michael, I found 'mozbehavior' doesn't do the trick on 2.0m, do you have any suggestion on how to fix this on 2.0m?

Comment 22

[Shine(Leave from Mozilla)]

Attachment: Flame2.2 video

This problem is verified pass on latest build of Flame2.2
See attachments: Flame2.2_video.MP4
Rate: 0/5

Flame 2.2 build:
Gaia-Rev        f5b3d1b6cfa3e702033f613915ae637cb735cbfb
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/8067c111ddff
Build-ID        20150118002501
Version         37.0a2
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150118.035516
FW-Date         Sun Jan 18 03:55:27 EST 2015
Bootloader      L1TC000118D0

Comment 23

[Sue]

Attachment: verify_v3.0.MP4

This issue has been verified successfully on Flame v3.0
See attachment:verify_v3.0.MP4
Rate:0/5

Flame 3.0 build:
Gaia-Rev        b02ec9713e6de8d96c6954d2c0dfd0442b0656ac
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/38e4719e71af
Build-ID        20150126160233
Version         38.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150126.192457
FW-Date         Mon Jan 26 19:25:08 EST 2015
Bootloader      L1TC000118D0