Closed
Bug 1219063
Opened 9 years ago
Closed 9 years ago
Remove obsolete "push" permission
Categories
(Core :: DOM: Push Subscriptions, defect)
Core
DOM: Push Subscriptions
Tracking
()
VERIFIED
FIXED
mozilla45
People
(Reporter: lina, Assigned: lina)
Details
Attachments
(2 files, 1 obsolete file)
6.93 KB,
patch
|
lina
:
review+
ritu
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
3.18 KB,
patch
|
lina
:
review+
ritu
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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•9 years ago
|
||
Bug 1219063, Part 1 - Use transactions for updating Push subscription permissions. r?mt
Attachment #8680338 -
Flags: review?(martin.thomson)
Assignee | ||
Comment 2•9 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•9 years ago
|
Attachment #8680338 -
Flags: review?(martin.thomson) → review+
Comment 3•9 years ago
|
||
Comment on attachment 8680338 [details] MozReview Request: Bug 1219063, Part 2 - Remove obsolete "push" permission. r?mt https://reviewboard.mozilla.org/r/23615/#review21119
Assignee | ||
Comment 4•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=300428e3ce88
Assignee | ||
Comment 5•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/1a7a278648367a73ba842f297f2c4e042b28edfe Bug 1219063, Part 1 - Use transactions for updating Push subscription permissions. r=mt https://hg.mozilla.org/integration/mozilla-inbound/rev/9cc31dc74e391cc233fe310218f090c790a7ecc1 Bug 1219063, Part 2 - Remove obsolete "push" permission. r=mt
Comment 6•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1a7a27864836 https://hg.mozilla.org/mozilla-central/rev/9cc31dc74e39
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox45:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Comment 7•9 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/1a7a27864836 https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/9cc31dc74e39
status-b2g-v2.5:
--- → fixed
Comment 8•9 years ago
|
||
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•9 years ago
|
||
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•9 years ago
|
||
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+
Comment 16•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/d14a19cedf0c https://hg.mozilla.org/releases/mozilla-beta/rev/24692f1a1939
Comment 18•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/d14a19cedf0c https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/24692f1a1939
status-b2g-v2.5:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•