Closed Bug 1254359 Opened 4 years ago Closed 4 years ago

notifications.getAll() result is not Chrome-compatible

Categories

(WebExtensions :: Untriaged, defect)

defect
Not set

Tracking

(firefox48 verified, firefox52 verified)

VERIFIED FIXED
mozilla48
Iteration:
48.2 - Apr 4
Tracking Status
firefox48 --- verified
firefox52 --- verified

People

(Reporter: wbamberg, Assigned: bsilverberg)

References

Details

(Whiteboard: [notifications])

Attachments

(2 files)

Attached file notify-getall.xpi
On Firefox, notifications.getAll() returns (via its callback) an array of notification IDs, so code like this:

    function gotAll(ids) {
      console.log(ids);
    }

    function getAll() {
      chrome.notifications.getAll(gotAll);
    }

...would give you console output like:

    Array [ "ghocyrlpa", "ffadcvitq", "ctzuox", "lkgzzgsgxqiw" ]

On Chrome, the output seems to be an object, with a property for each notification, whose name is the ID, and whose value is true:

    Object {etdzysf: true, ocri: true, pwiof: true, vndcvm: true}

I've attached a demo WebExtension.
I suppose we'll have to change this, but this change doesn't seem like an improvement to me...

I'm leaning towards changing it to return an object, but make each value an object describing the notification rather than a boolean.
Flags: blocking-webextensions?
Whiteboard: [notifications]
Assignee: nobody → bob.silverberg
Status: NEW → ASSIGNED
Iteration: --- → 48.1 - Mar 21
Comment on attachment 8731181 [details]
MozReview Request: Bug 1254359 - notifications.getAll() result is not Chrome-compatible, r?kmag

https://reviewboard.mozilla.org/r/40411/#review36947

::: toolkit/components/extensions/ext-notifications.js:125
(Diff revision 1)
>        },
>  
>        getAll: function() {
> -        let result = Array.from(notificationsMap.get(extension).keys());
> +        let result = {};
> +        notificationsMap.get(extension).forEach((value, key) => {
> +          result[key] = {id: value.id, options: value.options};

I think it would be better to just return the options object, rather than an object with an `options` key.
Attachment #8731181 - Flags: review?(kmaglione+bmo) → review+
Comment on attachment 8731181 [details]
MozReview Request: Bug 1254359 - notifications.getAll() result is not Chrome-compatible, r?kmag

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40411/diff/1-2/
Iteration: 48.1 - Mar 21 → 48.2 - Apr 4
Depends on: 1190324
Please land *after* bug 1190324
Keywords: checkin-needed
Blocks: 1256635
https://hg.mozilla.org/mozilla-central/rev/1d794d077cb1
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
I can reproduce this issue on Firefox 48.0a1 (20160321030217), here is a screenshot: http://screencast.com/t/lRhyMyiv8tbj 

This issue is verified as fixed in Firefox 48.0.2 (20160823121617) and Firefox 52.0a1 (20160929030426), notifications.getAll() returns (via its callback) an object and each value has an object describing the notification.
Here is a screenshot: http://screencast.com/t/74LfyFhys
Status: RESOLVED → VERIFIED
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.