Closed Bug 871530 Opened 6 years ago Closed 6 years ago

Update pdf.js to version 0.8.169

Categories

(Firefox :: PDF Viewer, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 24

People

(Reporter: RyanVM, Assigned: RyanVM)

References

(Depends on 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #861399 +++

Changes since the last update:
#3089 grammar fix for some comments
#3123 Do not cache content stream
#3126 Clamp end of range request to be the length of the file
#3132 Fix issue #3130 by changing a wrong reference of scope
#3131 Only perform range requests for identity Content-Encoding
#3103 Fixed typo in Czech viewer.properties
#3135 Workaround for issue 3068
#3142 Use same obj/font id counter for all partial evaluators on page
#3128 Use decodeURIComponent instead of unescape in parseQueryString() in viewer.js
#3143 Disable range request if content-length is unknown
#3023 Remove DOM window URI check
#3082 Use at least 1x1 pixel canvas for groups.
#3151 Remove redundant log in network.js
#3086 Lower two common warnings to info.
#3107 Vietnamese language
#3154 Add Korean language localization (ko)
#3053 Improve TT font program parser
#3088 Update Spanish l10n, issue #2979
#3056 Remove prefixed gradients usage
#3102 Normalize CFF CID sub matrices to work on windows.
#3176 Fixes the unprefixed gradient declaration
#3177 Make spacebar work on document load - fixes bug 864619
#3152 Fix node make extension for building chrome
#3075 Fix priority of which font encoding is used.
#3178 Cherry-pick of #3157
#3179 Cont of #3171, Reusing pattern canvas fixes
#3171 Use only one temp canvas for patterns
#3007 Fix encoding of type1 private dictionary arrays.
#3080 Fix disabling of zoom buttons and add camel case button names
#3104 Increase minimum font size to 16px to avoid conflict with browser's mini...
#3182 Remove background when printing.
#3183 Ignore malformed dictionary entries.
#3199 Rounding floats in CFF
#3197 Fix type3 font loading regression.
#3187 Fix glyph selection for CID fonts that don't actually have CID font files.
#3124 Update loading bar during onprogress of range requests
#3210 Fixes bug 863591
#3211 Translated "document_colors_disabled" to PL
#3216 Update translation
#3193 Fixes issue 3076 (viewer off by one page)
#3060 Ask for password on failed decryption
#3228 Updated Japanese locale
#3227 Update translation (invalid password)
#3225 Update the Swedish l10n
#3201 Adds fill('evenodd') as alternative for mozFillRule
#2988 Refactor annotation code
#3229 Propagate promise rejections so we show the fallback.
#3231 Rejects loading when data is not available for checkHeader
#3209 Forces to measure "standard" fonts
#3233 Fixes password for range request loading
#3239 Bind chunk promises to avoid scope problems.
Attached patch Update pdf.js to version 0.8.160 (obsolete) — Splinter Review
Assignee: nobody → ryanvm
Status: NEW → ASSIGNED
Attachment #748799 - Flags: review?(bdahl)
Comment on attachment 748799 [details] [diff] [review]
Update pdf.js to version 0.8.160

Dave,
In this update we've added support for ranged loading so we can start rendering the PDF before we download the whole thing.  Could you review the applicable chrome privileged code (mainly components/PdfStreamConverter.js and content/network.js).
Attachment #748799 - Flags: review?(bdahl) → review?(dtownsend+bugmail)
Blocks: 862679
Might as well pick up a few more updates while we wait.

#3203 Fix for issue #2881
#3259 Fixes a broken pdf link for kdchart.pdf
#3245 Localized "invalid_password" to PL
#3241 Dutch localization (nl_NL) update
Summary: Update pdf.js to version 0.8.160 → Update pdf.js to version 0.8.169
Attachment #748799 - Attachment is obsolete: true
Attachment #748799 - Flags: review?(dtownsend+bugmail)
Attachment #750657 - Flags: review?(dtownsend+bugmail)
Comment on attachment 750657 [details] [diff] [review]
Update pdf.js to version 0.8.169

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

I'd rather remove the closures unless there is a reason I'm missing that makes them necessary.

::: browser/extensions/pdfjs/components/PdfStreamConverter.js
@@ +402,5 @@
>                                     .updateControlState(result, findPrevious);
>    }
>  };
>  
> +var RangedChromeActions = (function RangedChromeActionsClosure() {

Why are we doing closures here?

@@ +411,5 @@
> +              domWindow, contentDispositionFilename, originalRequest) {
> +
> +    ChromeActions.call(this, domWindow, contentDispositionFilename);
> +
> +    this.pdfUrl = originalRequest.URI.resolve('');

Why not just originalRequest.URI.spec?

::: browser/extensions/pdfjs/content/network.js
@@ +30,5 @@
> +    var msg = 'network.js: ' + (aMsg.join ? aMsg.join('') : aMsg);
> +    Services.console.logStringMessage(msg);
> +    // TODO(mack): dump() doesn't seem to work here...
> +    dump(msg + '\n');
> +  }

Some strange indenting going on here

@@ +32,5 @@
> +    // TODO(mack): dump() doesn't seem to work here...
> +    dump(msg + '\n');
> +  }
> +
> +var NetworkManager = (function NetworkManagerClosure() {

Why the closure here?
Attachment #750657 - Flags: review?(dtownsend+bugmail) → review+
Well, that was dumb. Forgot I had this in my queue when I pushed another patch to m-c. Mossop said over IRC it's ok to push a follow-up to address his comments rather than backing out.

https://hg.mozilla.org/mozilla-central/rev/aaf5d70ba693
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 24
(In reply to Dave Townsend (:Mossop) from comment #5)
> ::: browser/extensions/pdfjs/components/PdfStreamConverter.js
> @@ +402,5 @@
> >                                     .updateControlState(result, findPrevious);
> >    }
> >  };
> >  
> > +var RangedChromeActions = (function RangedChromeActionsClosure() {
> 
> Why are we doing closures here?

They're not really needed here, but this is a standard way we do our "class like" objects in pdf.js.  We can remove them from this file though since it doesn't fit in with the rest of the file.
 
> Why not just originalRequest.URI.spec?
Will fix.

> Some strange indenting going on here
Will fix.

> > +var NetworkManager = (function NetworkManagerClosure() {
> 
> Why the closure here?
This file is shared with our regular library so I'd like to keep it in the same style as the rest of pdf.js.
Blocks: 863159
Depends on: 878406
Blocks: 878897
Blocks: 854420
Blocks: 888777
Blocks: 895472
Depends on: 901288
Blocks: 904941
Depends on: 921409
Blocks: 857995
Depends on: 955851
Blocks: 831463
You need to log in before you can comment on or make changes to this bug.