Expose observer notification names on `nsIPushService`

RESOLVED FIXED in Firefox 47

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: lina, Assigned: lina)

Tracking

unspecified
mozilla47
Points:
---

Firefox Tracking Flags

(firefox47 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
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`.
(Assignee)

Comment 1

3 years ago
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+

Comment 4

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/65955cf5a546
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.