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

Refactor/simplify the "delayedFallback" handling in the default viewer

Browse Source 
There's a few things that could be improved in the current implementation, such as:
 - It's currently necessary to *both* manually track the `featureId`s which should trigger delayed fallback, as well as manually report telemetry in affected cases.
Obviously there's only two call-sites as of now (forms and javaScript), but it still feels somewhat error-prone especially if more cases were to be added in the future. To address this, this patch adds a new (private) method which abstracts away these details from the call-sites.

 - Generally, it also seems nice to reduce *and* simplify the amount of state we need to store on `PDFViewerApplication` in order to support the "delayedFallback" functionality.
Also, having to *manually* work with the "delayedFallback"-array in multiple places feels cumbersome and makes e.g. the `PDFViewerApplication.fallback` method less clear as to its behaviour.

 - Having code *outside* of `PDFViewerApplication`, i.e. in the event handlers, directly access properties which are marked as "private" via a leading underscore doesn't seem that great in general.
Furthermore, having the event handlers directly deal with that should be internal state also seem unfortunate. To address this, the patch will instead make use of a new `PDFViewerApplication.triggerDelayedFallback` callback.

 - There's at least one code-path in the viewer, see `PDFViewerApplication.error`, where `fallback` can be called without an argument.
It's currently possible (although maybe somewhat unlikely) that such a call *could* be overridden by the `featureId` of a pending "delayedFallback" call, thus not reporting the *correct* fallback reason.

 - The "delayedFallback"-state weren't being reset on document close (which shouldn't affect Firefox, but nonetheless it ought to be fixed).
This commit is contained in:
Jonas Jenwald 2020-07-04 11:29:46 +02:00
parent 780faf6b26
commit 625f8a6f51
1 changed files with 33 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

63
web/app.js
View File

@ -189,8 +189,7 @@ const PDFViewerApplication = {
externalServices: DefaultExternalServices,
_boundEvents: {},
contentDispositionFilename: null,
_hasInteracted: false,
_delayedFallbackFeatureIds: [],
triggerDelayedFallback: null,
// Called once when the document is loaded.
async initialize(appConfig) {
@ -690,6 +689,7 @@ const PDFViewerApplication = {
this.url = "";
this.baseUrl = "";
this.contentDispositionFilename = null;
this.triggerDelayedFallback = null;
this.pdfSidebar.reset();
this.pdfOutlineViewer.reset();
@ -864,12 +864,29 @@ const PDFViewerApplication = {
.catch(downloadByUrl); // Error occurred, try downloading with the URL.
},
_recordFallbackErrorTelemetry(featureId) {
if (typeof PDFJSDev === "undefined" || PDFJSDev.test("MOZCENTRAL")) {
/**
* For PDF documents that contain e.g. forms and javaScript, we should only
* trigger the fallback bar once the user has interacted with the page.
* @private
*/
_delayedFallback(featureId) {
if (
typeof PDFJSDev === "undefined" ||
PDFJSDev.test("MOZCENTRAL || GENERIC")
) {
// Ensure that telemetry is always reported, since it's not guaranteed
// that the fallback bar will be shown (depends on user interaction).
this.externalServices.reportTelemetry({
type: "unsupportedFeature",
featureId,
});
if (!this.triggerDelayedFallback) {
this.triggerDelayedFallback = () => {
this.fallback(featureId);
this.triggerDelayedFallback = null;
};
}
}
},
@ -878,24 +895,16 @@ const PDFViewerApplication = {
typeof PDFJSDev === "undefined" ||
PDFJSDev.test("MOZCENTRAL || GENERIC")
) {
if (featureId) {
this._recordFallbackErrorTelemetry(featureId);
}
// For PDFs that contain script and form errors, we should only trigger
// the fallback once the user has interacted with the page.
if (this._delayedFallbackFeatureIds.length >= 1 && this._hasInteracted) {
featureId = this._delayedFallbackFeatureIds[0];
// Reset to prevent all click events from showing fallback bar.
this._delayedFallbackFeatureIds = [];
}
this.externalServices.reportTelemetry({
type: "unsupportedFeature",
featureId,
});
// Only trigger the fallback once so we don't spam the user with messages
// for one PDF.
if (this.fellback) {
return;
}
this.fellback = true;
this.externalServices.fallback(
{
@ -1259,8 +1268,7 @@ const PDFViewerApplication = {
return false;
}
console.warn("Warning: JavaScript is not supported");
this._delayedFallbackFeatureIds.push(UNSUPPORTED_FEATURES.javaScript);
this._recordFallbackErrorTelemetry(UNSUPPORTED_FEATURES.javaScript);
this._delayedFallback(UNSUPPORTED_FEATURES.javaScript);
return true;
});
@ -1342,8 +1350,7 @@ const PDFViewerApplication = {
if (info.IsAcroFormPresent) {
console.warn("Warning: AcroForm/XFA is not supported");
this._delayedFallbackFeatureIds.push(UNSUPPORTED_FEATURES.forms);
this._recordFallbackErrorTelemetry(UNSUPPORTED_FEATURES.forms);
this._delayedFallback(UNSUPPORTED_FEATURES.forms);
}
if (
@ -2501,15 +2508,13 @@ function webViewerWheel(evt) {
}
function webViewerClick(evt) {
PDFViewerApplication._hasInteracted = true;
// Avoid triggering the fallback bar when the user clicks on the
// toolbar or sidebar.
if (
PDFViewerApplication._delayedFallbackFeatureIds.length >= 1 &&
PDFViewerApplication.triggerDelayedFallback &&
PDFViewerApplication.pdfViewer.containsElement(evt.target)
) {
PDFViewerApplication.fallback();
PDFViewerApplication.triggerDelayedFallback();
}
if (!PDFViewerApplication.secondaryToolbar.isOpen) {
@ -2527,12 +2532,10 @@ function webViewerClick(evt) {
function webViewerKeyUp(evt) {
if (evt.keyCode === 9) {
// The user is tabbing into the pdf. Display the error message
// if it has not already been displayed.
PDFViewerApplication._hasInteracted = true;
if (PDFViewerApplication._delayedFallbackFeatureIds.length >= 1) {
PDFViewerApplication.fallback();
// The user is tabbing into the viewer. Trigger the fallback bar if it has
// not already been displayed.
if (PDFViewerApplication.triggerDelayedFallback) {
PDFViewerApplication.triggerDelayedFallback();
}
}
}