Fix the exposure of Push interfaces

RESOLVED FIXED in Firefox 42

Status

()

defect
RESOLVED FIXED
4 years ago
2 months ago

People

(Reporter: Ehsan, Assigned: Ehsan)

Tracking

unspecified
mozilla42
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox42 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Currently we don't check the dom.push.enabled pref in some cases for
some of these interfaces.  This patch unifies how all of these
interfaces are exposed to Window, Worker, and ServiceWorker.
Over to Doug for the push specific review, and to Boris for the test_interfaces changes.
Attachment #8639484 - Flags: review?(dougt)
Attachment #8639484 - Flags: review?(bzbarsky)
Josh, can you please see if this patch fixes the issues you were seeing?
Flags: needinfo?(josh)
Comment on attachment 8639484 [details] [diff] [review]
Fix the exposure of Push interfaces

r=me on the IDL and test_*interfaces changes.
Attachment #8639484 - Flags: review?(bzbarsky) → review+
Comment on attachment 8639484 [details] [diff] [review]
Fix the exposure of Push interfaces

Review of attachment 8639484 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm
Attachment #8639484 - Flags: review?(dougt) → review+
Things are way more broken than I realized.  :(  We don't even build dom/push on b2g, so the Enabled method cannot live there.
I will fold this into the previous patch before landing.
Attachment #8639637 - Flags: review?(bzbarsky)
Comment on attachment 8639637 [details] [diff] [review]
Part 2: Move PushManager::Enabled() to nsContentUtils to make things build on b2g

r=me
Attachment #8639637 - Flags: review?(bzbarsky) → review+
Doug, why is push enabled/disabled based on MOZ_WIDGET_GONK and MOZ_WIDGET_ANDROID and not MOZ_B2G and ANDROID?

http://mxr.mozilla.org/mozilla-central/source/modules/libpref/init/all.js#4411
Flags: needinfo?(dougt)
If I'm guessing the intention behind hiding this API correctly, this should be the correct way to expose it only to desktop.  This passes the Mulet tests, and makes the API unavailable there, similar to b2g.
Attachment #8639484 - Attachment is obsolete: true
Attachment #8639637 - Attachment is obsolete: true
Attachment #8639981 - Flags: review?(dougt)
(Note that this patch bounced on inbound earlier today because of test failures on Mulet: <https://treeherder.mozilla.org/logviewer.html#?job_id=12203809&repo=mozilla-inbound>)
(In reply to Josh Matthews [:jdm] from comment #14)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=823038be6a1c

That doesn't build.
Comment on attachment 8639981 [details] [diff] [review]
Fix the exposure of Push interfaces

Nikhil, can you please help review the all.js changes?
Attachment #8639981 - Flags: review?(nsm.nikhil)
Part 1 and part 2 gave me a green try build, so I'm happy.
Flags: needinfo?(josh)
Blocks: 1125961
https://hg.mozilla.org/mozilla-central/rev/ffd18bccd1ce
Assignee: nobody → ehsan
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
eshan: copy and paste.
Flags: needinfo?(dougt)
Attachment #8639981 - Flags: review?(dougt) → review+
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.