Closed Bug 773422 Opened 9 years ago Closed 9 years ago

Update pdf.js for FF16

Categories

(Firefox :: PDF Viewer, task)

x86
macOS
task
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 16

People

(Reporter: bdahl, Assigned: bdahl)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

#1743 Add viewport metatag for mobile
#1793 Uses ellipsis instead of dots
#1794 Fix fallback after new download.
#1763 Type3 smoothing: pre-scale image in the paintImageMaskXObject
#1807 Dismiss native browser zoom, and use PDF.JS zoom instead
#1811 Adding explicit masking
#1816 Added missing css vendor prefixes
#1823 PDF.js should use a consistent font for its user interface
#1693 Use a reusable style sheet instead of creating a new tag for each rule.
#1808 UI update from shorlanders comments.
#1837 JBIG2 implementation
#1831 Change to priority/pausible rendering.
#1849 Fix for web worker test in Safari
#1856 Warn instead of fail for CID fonts
#1881 Adds lossless segment types; fixes generic region prediction flag
#1882 Segmentation symbol check for JPX
#1890 Skip properties inherited from array.prototype
#1884 Moz print callback
We've mainly been trying to improve printing over the last six weeks, so there aren't many changes.  For the review the main thing I'd focus are the changes to the PdfStreamConverter(reviewed by Yury already as well) to support downloading the pdf file from a blob instead of re-requesting the file.  See https://github.com/mozilla/pdf.js/pull/1786 for those specific changes.  Most of the other changes are to code that run in content permission code.
Attachment #641588 - Flags: review?(dtownsend+bugmail)
Comment on attachment 641588 [details] [diff] [review]
pdf.js 0.3.452

Review of attachment 641588 [details] [diff] [review]:
-----------------------------------------------------------------

A couple of tweaks but r+ once those are done

::: browser/extensions/pdfjs/components/PdfStreamConverter.js
@@ +133,5 @@
> +    // The data may not be downloaded so we need just retry getting the pdf with
> +    // the original url.
> +    var blobUrl = data.blobUrl || originalUrl;
> +    var originalUri = NetUtil.newURI(originalUrl);
> +    var blobUri = NetUtil.newURI(blobUrl);

You may be parsing the same uri twice here and the near-identical variable names are a little confusing. How about something like this:

var originalUri = NetUtil.newURI(data.originalUrl);
var blobUri = data.blobUrl ? NetUtil.newURI(data.blobUrl) : originalUri;

@@ +143,2 @@
>      var ioService = Services.io;
> +    var channel = ioService.newChannel(originalUrl, null, null);

This channel seems to go unused
Attachment #641588 - Flags: review?(dtownsend+bugmail) → review+
Attached patch pdf.js 0.3.452Splinter Review
Attachment #641588 - Attachment is obsolete: true
Updated patch. Check-in needed see previous patch for r+.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/f9723f26c8a9
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 16
Blocks: 778398
Type: defect → task
You need to log in before you can comment on or make changes to this bug.