For #11838: trigger fallback bar after user clicks in pdf

[emalysz]

No description provided.

[timvandermeij]

Moreover, it looks like this happens for every type of error. Shouldn't we limit this to just form errors for now? I think there are also types of errors that for example prevent pages from being shown or being rendered correctly, and I think that's something you want to be notified about regardless of whether the user clicks or not.

[emalysz]

@timvandermeij @Snuffleupagus Thanks for the review!

So looking at https://www.irs.gov/pub/irs-pdf/f1040.pdf as an example, it would now display the fallback bar with the new error message "The PDF document may not be displayed correctly" due to a font error. It would no longer display the "This PDF contains forms..." message, but the fallback bar is still present.

Is this desired behavior?

[Snuffleupagus]

[...]
Is this desired behavior?

Good point, probably not. Taking #11953 (comment) into account while also fixing the other issues (with the fallback bar showing multiple times) seems a bit trickier than you'd expect.

Hopefully I'm not overstepping here, but would something like the following work?

diff --git a/web/app.js b/web/app.js
index 1d442f0a..550cce69 100644
--- a/web/app.js
+++ b/web/app.js
@@ -189,6 +189,7 @@ const PDFViewerApplication = {
   externalServices: DefaultExternalServices,
   _boundEvents: {},
   contentDispositionFilename: null,
+  _delayedFallbackFeatureId: null,
 
   // Called once when the document is loaded.
   async initialize(appConfig) {
@@ -867,6 +868,11 @@ const PDFViewerApplication = {
       typeof PDFJSDev === "undefined" ||
       PDFJSDev.test("MOZCENTRAL || GENERIC")
     ) {
+      // Comment about forms here...
+      if (this._delayedFallbackFeatureId) {
+        featureId = this._delayedFallbackFeatureId;
+        this._delayedFallbackFeatureId = null; <-- Reset this to prevent (possibly) every single click event from trying to show the fallback bar.
+      }
       // Only trigger the fallback once so we don't spam the user with messages
       // for one PDF.
       if (this.fellback) {
@@ -1317,7 +1323,7 @@ const PDFViewerApplication = {
 
     if (info.IsAcroFormPresent) {
       console.warn("Warning: AcroForm/XFA is not supported");
-      this.fallback(UNSUPPORTED_FEATURES.forms);
+      this._delayedFallbackFeatureId = UNSUPPORTED_FEATURES.forms;
     }
 
     if (
@@ -2473,6 +2479,13 @@ function webViewerWheel(evt) {
 }
 
 function webViewerClick(evt) {
+  if (
+    PDFViewerApplication._delayedFallbackFeatureId &&
+    PDFViewerApplication.pdfViewer.containsElement(evt.target) <-- Avoid triggering the fallback bar when the user clicks on e.g. the toolbar or sidebar.
+  ) {
+    PDFViewerApplication.fallback();
+  }
+
   if (!PDFViewerApplication.secondaryToolbar.isOpen) {
     return;
   }

[emalysz]

@Snuffleupagus I combined our approaches to make sure that we indicate when the click has happened with _hasInteracted. Without that, I don't believe it would have returned early. After a discussion with the pdf team, it was decided thatif it's a form and script error message, then we delay the fallback bar until after the click. For other errors, we should immediately display.

[Snuffleupagus]

it was decided thatif it's a form and script error message, then we delay the fallback bar until after the click.

But doesn't this mean that unless both forms/javascript entries exists (which isn't generally guaranteed), the fallback bar won't actually display with this patch; is that actually intentional?

For some PDF documents, this thus means that we no longer show the fallback bar at all, right?

---

Basically, my question is if it shouldn't have been forms OR javascript, and the conditions thus >= 1 instead!?

Finally, note that with this patch the fallback bar message will also change, in the way described in #11953 (comment), given the order of which the PDFViewerApplication.fallback method is called (since all call-sites are in asynchronous code).

[emalysz]

@Snuffleupagus Thanks!

Yeah unfortunately we'd still be displaying a fallback bar for the font error. But in the event that a font error drastically changes the appearance of a form pdf, it seems reasonable to show this bar regardless of a click.

[emalysz]

#11838 (comment) mentions accessibility risks. I consulted with the accessibility team, and they thought that we should trigger the fallback bar if the user clicks or tabs in. I have added a new event handler for Tab key presses.

[timvandermeij]

/botio-linux preview

[pdfjsbot]

From: Bot.io (Linux m4)

---

Received

Command cmd_preview from @timvandermeij received. Current queue size: 0

Live output at: http://54.67.70.0:8877/b44b01f623e3c71/output.txt

[pdfjsbot]

From: Bot.io (Linux m4)

---

Success

Full output at http://54.67.70.0:8877/b44b01f623e3c71/output.txt

Total script time: 3.44 mins

Published

• Viewer: http://54.67.70.0:8877/b44b01f623e3c71/web/viewer.html
• Viewer (ES5): http://54.67.70.0:8877/b44b01f623e3c71/es5/web/viewer.html

[brendandahl]

@Snuffleupagus is this good to go now?

[Snuffleupagus]

@Snuffleupagus is this good to go now?

Yes, I think so. (There's probably some potential for simplification, especially once #11969 has landed as well, but that can probably be handled in a follow-up though.)

[timvandermeij]

Thank you for improving this!