Closed
Bug 1188091
Opened 9 years ago
Closed 9 years ago
Fix the exposure of Push interfaces
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla42
Tracking | Status | |
---|---|---|
firefox42 | --- | fixed |
People
(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)
References
Details
Attachments
(1 file, 2 obsolete files)
13.57 KB,
patch
|
dougt
:
review+
nsm
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•9 years ago
|
||
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)
Assignee | ||
Comment 2•9 years ago
|
||
Josh, can you please see if this patch fixes the issues you were seeing?
Flags: needinfo?(josh)
Comment 3•9 years ago
|
||
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 5•9 years ago
|
||
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+
Assignee | ||
Comment 6•9 years ago
|
||
Things are way more broken than I realized. :( We don't even build dom/push on b2g, so the Enabled method cannot live there.
Assignee | ||
Comment 7•9 years ago
|
||
I will fold this into the previous patch before landing.
Attachment #8639637 -
Flags: review?(bzbarsky)
Comment 8•9 years ago
|
||
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+
Comment 10•9 years ago
|
||
Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/da44ec87bf15
Assignee | ||
Comment 11•9 years ago
|
||
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)
Assignee | ||
Comment 12•9 years ago
|
||
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)
Assignee | ||
Comment 13•9 years ago
|
||
(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>)
Assignee | ||
Comment 15•9 years ago
|
||
(In reply to Josh Matthews [:jdm] from comment #14) > https://treeherder.mozilla.org/#/jobs?repo=try&revision=823038be6a1c That doesn't build.
Assignee | ||
Comment 17•9 years ago
|
||
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)
Comment 18•9 years ago
|
||
Part 1 and part 2 gave me a green try build, so I'm happy.
Flags: needinfo?(josh)
Attachment #8639981 -
Flags: review?(nsm.nikhil) → review+
Comment 20•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ffd18bccd1ce
Assignee: nobody → ehsan
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox42:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Updated•9 years ago
|
Attachment #8639981 -
Flags: review?(dougt) → review+
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•