Sakurai/pdf.js
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Ensure that onProgress is always called when the entire PDF file has been loaded, regardless of how it was fetched (issue 10160)

Browse Source 
*Please note:* I'm totally fine with this patch being rejected, and the issue closed as WONTFIX; however these changes should address the issue if that's desired.

From a conceptual point of view, reporting loading progress doesn't really make a lot of sense for PDF files opened by passing raw binary data directly to `getDocument` (since obviously *all* data was loaded).
This is compared to PDF files loaded via e.g. `XMLHttpRequest` or the Fetch API, where the entire PDF file isn't available from the start and knowing the loading progress makes total sense.

However I can certainly see why the current API could be considered inconsistent, which isn't great, since a registered `onProgress` callback will never be called for certain `getDocument` calls.
The simplest solution to this inconsistency thus seem to be to ensure that `onProgress` is always called when handling the `DataLoaded` message, since that will *always* be dispatched[1] from the worker-thread.

---
[1] Note that this isn't guaranteed to happen, since setting `disableAutoFetch = true` often prevents the *entire* file from ever loading. However, this isn't relevant for the issue at hand, and is a well-known consequence of using `disableAutoFetch = true`; note how the default viewer even has a specialized code-path for hiding the loadingBar.
This commit is contained in:
Jonas Jenwald 2018-10-16 13:24:02 +02:00
parent ecbdc508f7
commit 327f2eb588
2 changed files with 25 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
src/display/api.js
View File

@ -1763,12 +1763,9 @@ class WorkerTransport {
fullReader.headersReady.then(() => { fullReader.headersReady.then(() => {
// If stream or range are disabled, it's our only way to report // If stream or range are disabled, it's our only way to report
// loading progress. // loading progress.
if (!fullReader.isStreamingSupported || if (!fullReader.isStreamingSupported || !fullReader.isRangeSupported) {
!fullReader.isRangeSupported) { if (this._lastProgress && loadingTask.onProgress) {
if (this._lastProgress) { loadingTask.onProgress(this._lastProgress);
if (loadingTask.onProgress) {
loadingTask.onProgress(this._lastProgress);
}
} }
fullReader.onProgress = (evt) => { fullReader.onProgress = (evt) => {
if (loadingTask.onProgress) { if (loadingTask.onProgress) {
@ -1866,6 +1863,14 @@ class WorkerTransport {
}, this); }, this);
messageHandler.on('DataLoaded', function(data) { messageHandler.on('DataLoaded', function(data) {
// For consistency: Ensure that progress is always reported when the
// entire PDF file has been loaded, regardless of how it was fetched.
if (loadingTask.onProgress) {
loadingTask.onProgress({
loaded: data.length,
total: data.length,
});
}
this.downloadInfoCapability.resolve(data); this.downloadInfoCapability.resolve(data);
}, this); }, this);

17
test/unit/api_spec.js
View File

@ -142,9 +142,20 @@ describe('api', function() {
// Sanity check to make sure that we fetched the entire PDF file. // Sanity check to make sure that we fetched the entire PDF file.
expect(typedArrayPdf.length).toEqual(basicApiFileLength); expect(typedArrayPdf.length).toEqual(basicApiFileLength);
var loadingTask = getDocument(typedArrayPdf); const loadingTask = getDocument(typedArrayPdf);
loadingTask.promise.then(function(data) {
expect(data instanceof PDFDocumentProxy).toEqual(true); const progressReportedCapability = createPromiseCapability();
loadingTask.onProgress = function(data) {
progressReportedCapability.resolve(data);
};
Promise.all([
loadingTask.promise,
progressReportedCapability.promise,
]).then(function(data) {
expect(data[0] instanceof PDFDocumentProxy).toEqual(true);
expect(data[1].loaded / data[1].total).toEqual(1);
loadingTask.destroy().then(done); loadingTask.destroy().then(done);
}).catch(done.fail); }).catch(done.fail);
}); });