|Submitter||Diff||Changes||Open Issues||Last Updated|
|Error loading review requests:|
58 bytes, text/x-review-board-request
|Details | Review|
Technically, they're related to `nsIPushNotifier`...but that would require consumers to import two services, and `nsIPushNotifier` is an internal interface that only supports `PushService.jsm`.
Created attachment 8726451 [details] MozReview Request: Bug 1253438 - Expose Push observer notification topics. r?markh Review commit: https://reviewboard.mozilla.org/r/38005/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/38005/
Attachment #8726451 - Flags: review?(markh)
Comment on attachment 8726451 [details] MozReview Request: Bug 1253438 - Expose Push observer notification topics. r?markh https://reviewboard.mozilla.org/r/38005/#review34563 Loogs good to me. Note I'm not a DOM peer, so I'm probably not technically able to r+ this, but given the changes are all in dom/push and you are requesting review, it's probably fine. ::: dom/push/PushComponents.js:19 (Diff revision 1) > +const OBSERVER_TOPIC_PUSH = "push-message"; I think it's probably worth a comment indicating that these constants are duplicated in PushNotifier.cpp (avoiding the duplication is probably possible if PushNotifier got a handle to the push service, but I don't think we need to bother with that). Ditto in the .cpp (ie, a comment noting the constants are duplicated in the .js)
Attachment #8726451 - Flags: review?(markh) → review+
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.