Skip to content

Commit

Permalink
[Firefox regression] Fix disableRange=true bug in `PDFDataTransport…
Browse files Browse the repository at this point in the history
…Stream`

Currently if trying to set `disableRange=true` in the built-in PDF Viewer in Firefox, either through `about:config` or via the URL hash, the PDF document will never load. It appears that this has been broken for a couple of years, without anyone noticing.

Obviously it's not a good idea to set `disableRange=true`, however it seems that this bug affects the PDF Viewer in Firefox even with default settings:
 - In the case where `initialData` already contains the *entire* file, we're forced to dispatch a range request to re-fetch already available data just so that file loading may complete.
 - In the case where the data arrives, via streaming, before being specifically requested through `requestDataRange`, we're also forced to re-fetch data unnecessarily.

In both cases outlined above, we're having to re-fetch already available data thus potentially delaying loading/rendering of PDF files in Firefox (and wasting resources in the process).
  • Loading branch information
Snuffleupagus committed Mar 24, 2019
1 parent 9b5a937 commit 9592e3e
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 2 deletions.
19 changes: 17 additions & 2 deletions src/display/transport_stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,16 @@ var PDFDataTransportStream = (function PDFDataTransportStreamClosure() {
stream._fullRequestReader = this;

this.onProgress = null; // not used

let loaded = 0;
for (const chunk of this._queuedChunks) {
loaded += chunk.byteLength;
}
this._loaded = loaded;

if (this._loaded >= this.contentLength) {
this._done = true;
}
}
PDFDataTransportStreamReader.prototype = {
_enqueue: function PDFDataTransportStreamReader_enqueue(chunk) {
Expand All @@ -135,9 +145,14 @@ var PDFDataTransportStream = (function PDFDataTransportStreamClosure() {
if (this._requests.length > 0) {
var requestCapability = this._requests.shift();
requestCapability.resolve({ value: chunk, done: false, });
return;
} else {
this._queuedChunks.push(chunk);
}
this._loaded += chunk.byteLength;

if (this._loaded >= this.contentLength) {
this._done = true;
}
this._queuedChunks.push(chunk);
},

get headersReady() {
Expand Down
23 changes: 23 additions & 0 deletions test/unit/api_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1526,5 +1526,28 @@ describe('api', function() {
});
}).catch(done.fail);
});

it('should fetch document info and page, without range, ' +
'using complete initialData', function(done) {
let fetches = 0, loadingTask;

dataPromise.then(function(data) {
const transport = new PDFDataRangeTransport(data.length, data);
transport.requestDataRange = function(begin, end) {
fetches++;
};
loadingTask = getDocument({ disableRange: true, range: transport, });
return loadingTask.promise;
}).then(function(pdfDocument) {
expect(pdfDocument.numPages).toEqual(14);

return pdfDocument.getPage(10);
}).then(function(pdfPage) {
expect(pdfPage.rotate).toEqual(0);
expect(fetches).toEqual(0);

loadingTask.destroy().then(done);
}).catch(done.fail);
});
});
});

0 comments on commit 9592e3e

Please sign in to comment.