Closed Bug 1316526 Opened 8 years ago Closed 7 years ago

[jsplugins][UI] Implement download feature

Categories

(Firefox :: PDF Viewer, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 53
Tracking Status
firefox53 --- fixed

People

(Reporter: lchang, Assigned: lchang)

References

Details

(Whiteboard: [pdfium-viewer-mvpscope])

Attachments

(1 file, 1 obsolete file)

Let's implement download feature.
Attached file Pull Request #82 (obsolete) —
The code in "ppapipdf.js:save" handler is basically copied from PDF.js [1]. 

However, the following code fragment is simpler and also works. Paste it here in case someday we need it.

>  let xhr = new containerWindow.XMLHttpRequest({ mozSystem: true });
>  xhr.responseType = "blob";
>  xhr.onreadystatechange = () => {
>    if (xhr.readyState === containerWindow.XMLHttpRequest.DONE &&
>        xhr.status === 200) {
>      let a = containerWindow.document.createElement("a");
>      a.href = containerWindow.URL.createObjectURL(xhr.response);
>      a.download = filename;
>      containerWindow.document.body.appendChild(a);
>      a.click();
>      a.remove();
>    }
>  };;
>  xhr.open("GET", url.href, true);
>  xhr.send();

[1] https://dxr.mozilla.org/mozilla-central/source/browser/extensions/pdfjs/content/PdfStreamConverter.jsm#241
Comment on attachment 8809290 [details] [review]
Pull Request #82

Hi Evelyn,

Would you mind taking a look in your spare time? Thanks.
Attachment #8809290 - Flags: review?(ehung)
Comment on attachment 8809290 [details] [review]
Pull Request #82

LGTM, please rebase against m-c and send patch via MozReview. Thanks.
Attachment #8809290 - Flags: review?(ehung)
Group: mozilla-employee-confidential
Attachment #8809290 - Attachment is obsolete: true
Comment on attachment 8822381 [details]
Bug 1316526 - [jsplugins][UI] Implement download feature;

https://reviewboard.mozilla.org/r/101320/#review103760

LGTM. Thanks!
Attachment #8822381 - Flags: review?(ehung) → review+
Thanks.
Keywords: checkin-needed
evelyn: seems you need to set review +/ landable in mozreview so that we can use autoland. Can you fix this, thanks!
Flags: needinfo?(ehung)
Comment on attachment 8822381 [details]
Bug 1316526 - [jsplugins][UI] Implement download feature;

https://reviewboard.mozilla.org/r/101320/#review104040
I gave my r+ in mozreview but it seems not working. I saw this message on the commit: A suitable reviewer has not given a "Ship It!", and I'm guessing it's because we don't have a proper module owner set on this part of code as it's a new module. Could you provide more instruction of how to fix this? Thanks!
Flags: needinfo?(ehung) → needinfo?(cbook)
(In reply to Evelyn Hung [:evelyn] from comment #10)
> I gave my r+ in mozreview but it seems not working. I saw this message on
> the commit: A suitable reviewer has not given a "Ship It!", and I'm guessing
> it's because we don't have a proper module owner set on this part of code as
> it's a new module. Could you provide more instruction of how to fix this?
> Thanks!

Hi Evelyn, redirecting this to mark since he knows mozreview more and this seems a more general issue.
Flags: needinfo?(cbook) → needinfo?(mcote)
(In reply to Carsten Book [:Tomcat] from comment #11)
> (In reply to Evelyn Hung [:evelyn] from comment #10)
> > I gave my r+ in mozreview but it seems not working. I saw this message on
> > the commit: A suitable reviewer has not given a "Ship It!", and I'm guessing
> > it's because we don't have a proper module owner set on this part of code as
> > it's a new module. Could you provide more instruction of how to fix this?

evelyn,

mozreview doesn't follow the module owner system (unfortunately there's no machine-readable way to consume it).
currently you need scm_level 3 for your r+ to result in an landing approval in mozreview, and as far as i can tell you don't have that level of access.

see https://www.mozilla.org/en-US/about/governance/policies/commit/access-policy/ for how to gain access.
Flags: needinfo?(mcote)
Yeah, I don't have L3. I have requested for access. Thanks for the explanation, Byron.
Hmm... just got my L3 access and it seems my given r+ has been applied on the commit. Ship it!
Pushed by ehung@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/75cc309e8443
[jsplugins][UI] Implement download feature; r=evelyn
https://hg.mozilla.org/mozilla-central/rev/75cc309e8443
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: