1489996 - Update pdf.js to version 2.0.843

Status: RESOLVED FIXED

(Firefox :: PDF Viewer, task)

Description

[Ryan VanderMeulen [:RyanVM]]

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

Changes since last update:
#10049 Fix font-string variable name typo
#9995 Refactor code in the `web/` folder to use `async`/`await`
#10053 Simplify the "spaced-comment" ESLint rule
#10034 Remove `getSinglePixelWidth` workaround
#10054 Update translations/packages and use HTTPS for links in `README.md`
#10052 Display the index of the currently active search result in the matches counter of the findbar (issue 6993, bug 1062025)
#10055 Translate the new find match count strings to Dutch
#10028 Add initial support for "Whole words" searching in the viewer

Comment 1

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c80bc4f44b7f
Update pdf.js to version 2.0.841. r=bdahl

Comment 2

[Daniel Varga [:dvarga]]

Backed out changeset c80bc4f44b7f (Bug 1489996) for bc failures in browser/extensions/pdfjs/test/browser_pdfjs_main.js

https://treeherder.mozilla.org/logviewer.html#?job_id=198423914&repo=mozilla-inbound&lineNumber=3717

[task 2018-09-10T13:35:55.857Z] 13:35:55     INFO - TEST-PASS | browser/extensions/pdfjs/test/browser_pdfjs_main.js | viewBookmark button has href - true == true - 
[task 2018-09-10T13:35:55.858Z] 13:35:55     INFO - Buffered messages finished
[task 2018-09-10T13:35:55.860Z] 13:35:55     INFO - TEST-UNEXPECTED-FAIL | browser/extensions/pdfjs/test/browser_pdfjs_main.js | Uncaught exception - TypeError: (destructured parameter) is undefined
[task 2018-09-10T13:35:55.862Z] 13:35:55     INFO - Leaving test bound test
[task 2018-09-10T13:35:55.864Z] 13:35:55     INFO - GECKO(4936) | MEMORY STAT vsizeMaxContiguous not supported in this build configuration.
[task 2018-09-10T13:35:55.864Z] 13:35:55     INFO - GECKO(4936) | MEMORY STAT | vsize 580MB | residentFast 254MB | heapAllocated 93MB
[task 2018-09-10T13:35:55.864Z] 13:35:55     INFO - TEST-OK | browser/extensions/pdfjs/test/browser_pdfjs_main.js | took 1750ms
[task 2018-09-10T13:35:55.865Z] 13:35:55     INFO - Not taking screenshot here: see the one that was previously logged
[task 2018-09-10T13:35:55.865Z] 13:35:55     INFO - TEST-UNEXPECTED-FAIL | browser/extensions/pdfjs/test/browser_pdfjs_main.js | Found an unexpected tab at the end of test run: http://example.com/browser/browser/extensions/pdfjs/test/file_pdfjs_test.pdf - 
[task 2018-09-10T13:35:55.865Z] 13:35:55     INFO - checking window state

Comment 3

[Daniel Varga [:dvarga]]

Push with failures:https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=c80bc4f44b7fca76ac23b762600cfdb3dd45a9e7&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified

Comment 4

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e6de32988b0e
Backed out changeset c80bc4f44b7f for bc failures in browser/extensions/pdfjs/test/browser_pdfjs_main.js

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Any idea what that failure is about, Brendan?

Comment 6

[Jonas Jenwald [:Snuffleupagus]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #5)
> Any idea what that failure is about, Brendan?

Sorry, this is my fault! I apparently didn't run the Mochitests locally for all the various combinations of the recent search-related patches.

Please see https://github.com/mozilla/pdf.js/pull/10056 for the fix (confirmed locally this time).

Comment 7

[Tim van der Meij]

The fix is now merged upstream, so the update can be retried.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/caa65bf34838
Update pdf.js to version 2.0.843. r=bdahl

Comment 9

[Francesco Lodolo [:flod]]

Both find_matches_count and find_matches_count_limit need proper plural forms to be correctly localized.

What kind of space do we have to make changes upstream?

Comment 10

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/caa65bf34838

Comment 11

[Ryan VanderMeulen [:RyanVM]]

(In reply to Francesco Lodolo [:flod] from comment #9)
> Both find_matches_count and find_matches_count_limit need proper plural
> forms to be correctly localized.
> 
> What kind of space do we have to make changes upstream?

Redirecting to Jonas since that change came from one of his upstream commits.

Comment 12

[Jonas Jenwald [:Snuffleupagus]]

(In reply to Francesco Lodolo [:flod] from comment #9)
> Both find_matches_count and find_matches_count_limit need proper plural
> forms to be correctly localized.
> 
> What kind of space do we have to make changes upstream?

Please keep in mind that there's two different viewers here, the general PDF.js viewer (see https://mozilla.github.io/pdf.js/web/viewer.html) and the "PDF Viewer" shipping in Firefox.
The latter one is (obviously) based on the former one, but with a couple of differences such as e.g. the fact that the built-in "PDF Viewer" uses the browser findbar.

The mentioned strings are relevant for the general PDF.js viewer, but not really for the built-in "PDF Viewer" in Firefox since it uses the browser findbar (and thus the localization data present in "findbar.properties" files).
Hence this doesn't seem like a "problem" in Firefox, but rather in the general PDF.js library (and it can be tracked upstream instead). Given that none of the existing PDF.js viewer strings uses plural forms in the localization, I don't know how easy/difficult this would be to address.

Comment 13

[Francesco Lodolo [:flod]]

(In reply to Jonas Jenwald [:Snuffleupagus] from comment #12)
> I don't know how easy/difficult this would be to address.

Me neither, likely not trivial. Mozilla has a hack on top of .properties to make that work, not sure what's the library used by PDF.js for internationalization.

At this point the other question would be how can we avoid to expose strings for localization when they're actually not used in the product.