Remove obsolete "push" permission

VERIFIED FIXED in Firefox 44, Firefox OS v2.5

Status

()

VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: lina, Assigned: lina)

Tracking

Trunk
mozilla45
Points:
---

Firefox Tracking Flags

(firefox44 fixed, firefox45 fixed, b2g-v2.5 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

3 years ago
https://dxr.mozilla.org/mozilla-central/source/dom/push/PushService.jsm#1299 refers to the old "push" permission, as do the tests. Let's also add a mochitest that exercises this observer. The existing xpcshell test should take care of the other cases.
(Assignee)

Comment 1

3 years ago
Created attachment 8680338 [details]
MozReview Request: Bug 1219063, Part 2 - Remove obsolete "push" permission. r?mt

Bug 1219063, Part 1 - Use transactions for updating Push subscription permissions. r?mt
Attachment #8680338 - Flags: review?(martin.thomson)
(Assignee)

Comment 2

3 years ago
Comment on attachment 8680338 [details]
MozReview Request: Bug 1219063, Part 2 - Remove obsolete "push" permission. r?mt

Bug 1219063, Part 2 - Remove obsolete "push" permission. r?mt
Attachment #8680338 - Attachment description: MozReview Request: Bug 1219063, Part 1 - Use transactions for updating Push subscription permissions. r?mt → MozReview Request: Bug 1219063, Part 2 - Remove obsolete "push" permission. r?mt

Updated

3 years ago
Attachment #8680338 - Flags: review?(martin.thomson) → review+
Comment on attachment 8680338 [details]
MozReview Request: Bug 1219063, Part 2 - Remove obsolete "push" permission. r?mt

https://reviewboard.mozilla.org/r/23615/#review21119

Comment 6

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/1a7a27864836
https://hg.mozilla.org/mozilla-central/rev/9cc31dc74e39
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
removing the b2g 2.5 flag since this commit has been reverted due to an incorrect merge, sorry for the confusion
status-b2g-v2.5: fixed → ---
(Assignee)

Comment 9

3 years ago
Created attachment 8700918 [details] [diff] [review]
2.patch

Approval Request Comment
[Feature/regressing bug #]: Bug 1233387.
[User impact if declined]: A site may continue sending push messages even if a user revokes permission. While the messages won't be posted to the service worker, the site has no indication that the subscription is no longer valid. Additionally, the `pushsubscriptionchange` event will not fire if the user restores the permission.
[Describe test coverage new/current, TreeHerder]: Test added in bug 1223202, also requested for uplift.
[Risks and why]: Medium risk. This patch has been on Nightly for more than six weeks, and verified working in bug 1233387. However, it's a moderate, two-part refactor of the `pushsubscriptionchange` logic.
[String/UUID change made/needed]: None.
Attachment #8680338 - Attachment is obsolete: true
Attachment #8700918 - Flags: review+
Attachment #8700918 - Flags: approval-mozilla-beta?
(Assignee)

Comment 10

3 years ago
Created attachment 8700919 [details] [diff] [review]
3.patch

Same as comment 9.
Attachment #8700919 - Flags: review+
Attachment #8700919 - Flags: approval-mozilla-beta?
Comment on attachment 8700918 [details] [diff] [review]
2.patch

Given the medium risk, I would prefer significant changes to feature design (unless critical) ride the trains than be uplifted to Beta directly. Please let me know if you have any concerns.
Attachment #8700918 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Comment on attachment 8700919 [details] [diff] [review]
3.patch

Same comment as above.
Attachment #8700919 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Kit, given the scenario that we are trying to fix seems important, is there another (hacky simpler) fix that can be made in Beta44? I am still trying to understand how important it is for us to fix this. Would we block the release for it?
Flags: needinfo?(kcambridge)
Comment on attachment 8700918 [details] [diff] [review]
2.patch

On second thoughts, it does seem like a mainline scenario that we can uplift to Beta44 and get QA to sign off on.
Attachment #8700918 - Flags: approval-mozilla-beta- → approval-mozilla-beta+
Comment on attachment 8700919 [details] [diff] [review]
3.patch

Beta44+
Attachment #8700919 - Flags: approval-mozilla-beta- → approval-mozilla-beta+
(Assignee)

Comment 17

3 years ago
Thanks for uplifting this!
Flags: needinfo?(kcambridge)
Verified Beta 44.0b4
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.