Closed Bug 1245641 Opened 8 years ago Closed 8 years ago

Implement chrome.downloads.erase()

Categories

(WebExtensions :: Untriaged, defect)

defect
Not set
normal

Tracking

(firefox48 fixed)

RESOLVED FIXED
mozilla48
Iteration:
48.1 - Mar 21
Tracking Status
firefox48 --- fixed

People

(Reporter: aswan, Assigned: aswan)

References

(Blocks 1 open bug)

Details

(Whiteboard: [downloads])

Attachments

(1 file)

Part of supporting chrome.downloads in WebExtensions

https://developer.chrome.com/extensions/downloads#method-erase
Blocks: 1213445
Whiteboard: [downloads]
Assignee: nobody → aswan
Iteration: --- → 48.1 - Mar 21
Comment on attachment 8730726 [details]
MozReview Request: Bug 1245641 Implement chrome.downloads.erase() r?kmag

https://reviewboard.mozilla.org/r/40121/#review36661

::: toolkit/components/extensions/schemas/downloads.json:712
(Diff revision 1)
> +                "type": "number"
> +              },
> +              "exists": {
> +                "type": "boolean",
> +                "optional": true
>                }

Can we add a type for this, and use it for both `.search()` and `.erase()`?
Attachment #8730726 - Flags: review?(kmaglione+bmo) → review+
https://reviewboard.mozilla.org/r/40121/#review36661

> Can we add a type for this, and use it for both `.search()` and `.erase()`?

I thought about that, but if we generate docs from the schema, then the unified type will show up and our docs will appear to be different from Chrome's even when we're really identical (in this area anyway).  I could well be overthinking it, is that not a concern?
https://reviewboard.mozilla.org/r/40121/#review36661

> I thought about that, but if we generate docs from the schema, then the unified type will show up and our docs will appear to be different from Chrome's even when we're really identical (in this area anyway).  I could well be overthinking it, is that not a concern?

I'm not expecting to generate docs from the schema again any time soon, but even if we do, I wouldn't worry about them looking different from Chrome's. If anything, I think it should make the documentation clearer.
Comment on attachment 8730726 [details]
MozReview Request: Bug 1245641 Implement chrome.downloads.erase() r?kmag

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40121/diff/1-2/
Keywords: checkin-needed
has problems to apply:

Hunk #3 FAILED at 469
Hunk #4 FAILED at 573
2 out of 4 hunks FAILED -- saving rejects to file toolkit/components/extensions/ext-downloads.js.rej
patching file toolkit/components/extensions/test/mochitest/test_chrome_ext_downloads_misc.html
Hunk #1 FAILED at 459
1 out of 1 hunks FAILED -- saving rejects to file toolkit/components/extensions/test/mochitest/test_chrome_ext_downloads_misc.html.rej
patch failed to apply
abort: fix up the working directory and run hg transplant --continue
Flags: needinfo?(aswan)
Keywords: checkin-needed
I was able to rebase cleanly onto fx-team, should I be using something else?
Flags: needinfo?(aswan) → needinfo?(cbook)
Comment on attachment 8730726 [details]
MozReview Request: Bug 1245641 Implement chrome.downloads.erase() r?kmag

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40121/diff/2-3/
Comment on attachment 8730726 [details]
MozReview Request: Bug 1245641 Implement chrome.downloads.erase() r?kmag

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40121/diff/3-4/
Rebased onto the very latest fx-team
Flags: needinfo?(cbook)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/06ae85d888e8
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.