JS - Avoid a popup to ask for specific version of Acrobat #14207

janekotovich · 2021-10-29T00:50:23Z

Because

Popup keep asking regarding specific version of Acrobat (Spanish and version 5.0 or greater).
That happens because:

JS in pdf is enabled;
And the pdf contains some unsupported features (e.g. XFA);

By searching through pdf source code I have found that there is an embedded JS in pdf throws error & alert, if

If ( app.formVersion = undefined || app.formVersion < 5.0 ) {
Alert(“These documents are only functional in Spanish versions of Acrobat equal to or greater than 5.0”);
}

Regarding documents in https://helpx.adobe.com/livecycle/help/mobile-forms/scripting-support.html for Scripting Support,

Issue that this pull request solves

Closes: #14206

janekotovich · 2021-10-29T00:54:23Z

I have tested that via:

gulp dist install
gulp server
gulp lint

Hope its all good 😊

Snuffleupagus · 2021-10-29T07:54:54Z

As a general rule: Please make sure to provide more context in the commit message as well, such that the information also becomes available on the Git command line.
You've included a bunch of context in #14207 (comment), e.g. a description of the underlying problem and a specification link, all of which would be great to have in the commit message as well.

I have tested that via:
* gulp dist install

[...]

In order to save your time: Please note that gulp dist-install is really only necessary when working with the examples in https://github.com/mozilla/pdf.js/tree/master/examples/, since given its run-time it'll otherwise just waste your time.

Snuffleupagus · 2021-10-29T08:12:06Z

src/scripting_api/app.js

@@ -22,7 +22,7 @@ import { Thermometer } from "./thermometer.js";
 const VIEWER_TYPE = "PDF.js";
 const VIEWER_VARIATION = "Full";
 const VIEWER_VERSION = "10.0";
-const FORMS_VERSION = undefined;
+const FORMS_VERSION = "11.001";


According to the scripting-spec, see https://opensource.adobe.com/dc-acrobat-sdk-docs/acrobatsdk/pdfs/acrobatsdk_jsapiref.pdf#G4.1992920, the value returned by the formsVersion getter should be a number. Hence shouldn't this constant thus be a number as well?

@calixteman What's correct here, since according to this the existing VIEWER_VERSION constant is also the wrong type?

Yep that's correct: they're both numbers.
I checked the values in Acrobat DC and they're both equal to 21.00720099.
I looked in pdfium source code and they're using 7 for the formsVersion, when they're using 8 or 11 for viewerVersion depending of the context (xfa or not).
@Snuffleupagus, what do you think we should choose for those values ?

What's the practical effect of choosing "too large" values here; that we sort-of claim to support various scripting features that we perhaps haven't actually implemented? (I'm guessing this might already be the case though...)

Anyway, going with the larger values seem nice since that would mean that we don't unnecessarily bother users with warnings upon loading of documents.

Yeah, sorry, I figure out its a number but was relating to VIEWER_VISION and thats why decided to go with the string.

Yes I think it's better to have a large number for the reasons you mentioned (this is my opinion and it's conform with the Mozilla policy).
Is the value 21 ok for you ?

Is the value 21 ok for you ?

That seems fine by me!

janekotovich · 2021-10-29T11:24:01Z

@Snuffleupagus no problem, I though it should be one-line commit messages to keep it simple and not clogged.
What would be good to include? Better description or link as well? Thanks

Snuffleupagus · 2021-10-29T11:43:55Z

I though it should be one-line commit messages to keep it simple and not clogged.

The first line of the commit message should indeed be a short summary of the changes in the patch. However, you're then free to add additional lines with more information (as much as you think is helpful/useful) to the commit message.

You might want to look at some older commit messages, e.g. in https://github.com/mozilla/pdf.js/commits/master, to see how this is often done.

What would be good to include? Better description or link as well?

Both seem like a good idea to me.

Something I always find helpful, when writing a commit message, is to imagine that you're reading it yourself in e.g. a months time (on the Git command line): Does the commit message clearly explain the problem that's being solved and why the patch is implemented like it is?

Embedded JS in PDF keep throwing alert reagdring specific version of Acrobat (Spanish and version 5.0 or greater). This happens because: - JS in pdf is enabled - PDF contains some unsupported features (e.g. XFA) Alert come when app.formVersion = undefined || app.formVersion < 5.0 In pdf.js we were using FORM_VERSION = undefined. After researching based on https://opensource.adobe.com/dc-acrobat-sdk-docs/acrobatsdk/pdfs/acrobatsdk_jsapiref.pdf\#G4.1993509 and Acrobat DC we decided to go with the larger number to avoid unnecessary popups. Through investigation we realise that VIEWER_VERSION should have same value - a number. Due to all that, we implemented 21.00720099 as a value for both FORMS_VERSION and VIEWER_VERSION

janekotovich · 2021-10-29T13:13:41Z

@Snuffleupagus thanks heaps, your advice of future time was really helpful. Changed the commit msg, its more descriptive now :)

Snuffleupagus · 2021-10-30T11:40:35Z

@calixteman Do we have a good/simple way of testing that an alert box isn't displayed on document loading, or can we land this as-is?

/botio integrationtest

pdfjsbot · 2021-10-30T11:40:36Z

From: Bot.io (Linux m4)

Received

Command cmd_integrationtest from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/99f88ff6afe20a1/output.txt

pdfjsbot · 2021-10-30T11:40:37Z

From: Bot.io (Windows)

Received

Command cmd_integrationtest from @Snuffleupagus received. Current queue size: 1

Live output at: http://54.193.163.58:8877/76011312febb158/output.txt

pdfjsbot · 2021-10-30T11:44:28Z

From: Bot.io (Linux m4)

Success

Full output at http://54.241.84.105:8877/99f88ff6afe20a1/output.txt

Total script time: 3.84 mins

Integration Tests: Passed

pdfjsbot · 2021-10-30T12:08:08Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/76011312febb158/output.txt

Total script time: 6.35 mins

Integration Tests: FAILED

calixteman · 2021-10-30T12:32:42Z

@calixteman Do we have a good/simple way of testing that an alert box isn't displayed on document loading, or can we land this as-is?

Some actions are ran when the document is open:

pdf.js/src/scripting_api/doc.js

Lines 100 to 114 in ec1633c

    
           if (name === "Open") { 
        
             const dontRun = new Set([ 
        
               "WillClose", 
        
               "WillSave", 
        
               "DidSave", 
        
               "WillPrint", 
        
               "DidPrint", 
        
               "OpenAction", 
        
             ]); 
        
             for (const actionName of this._actions.keys()) { 
        
               if (!dontRun.has(actionName)) { 
        
                 this._runActions(actionName); 
        
               } 
        
             } 
        
             this._runActions("OpenAction");

and alert is defined here:

pdf.js/src/scripting_api/app.js

Line 429 in ec1633c

alert(

and the app object has access to the doc.
So it's easy to add a flag in the doc in order to know if we are in the "open" process and disable alert when this flag is true.

We can disable the popups at loading time but if one contains an information of interest (e.g. "Don't forget to fill whatever before sending the form"), the user will miss it.
I think that a well-designed form shouldn't have any intrusive popup at loading but it should warn before printing, saving, ... that something is missing.

So I think we could land that stuff as it is and I can ask to Product/UX/UI people what they think about disabling all popups at loading. @Snuffleupagus does that work for you ?

Snuffleupagus · 2021-10-30T12:34:56Z

So I think we could land that stuff as it is and I can ask to Product/UX/UI people what they think about disabling all popups at loading. @Snuffleupagus does that work for you ?

Sure, that sounds good to me!

calixteman

LGTM, thank you for doing this.

Snuffleupagus added the form-js label Oct 29, 2021

Snuffleupagus reviewed Oct 29, 2021

View reviewed changes

janekotovich force-pushed the forms_version_popup branch from 0e6e2f7 to 12f89d2 Compare October 29, 2021 13:10

calixteman approved these changes Oct 30, 2021

View reviewed changes

calixteman merged commit 2d8b6fd into mozilla:master Oct 30, 2021

janekotovich deleted the forms_version_popup branch November 2, 2021 05:30

Snuffleupagus mentioned this pull request Sep 26, 2022

[JS] Override the Doc.external-getter to avoid alert-modals on load (issue 15509) #15512

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JS - Avoid a popup to ask for specific version of Acrobat #14207

JS - Avoid a popup to ask for specific version of Acrobat #14207

janekotovich commented Oct 29, 2021 •

edited

Loading

janekotovich commented Oct 29, 2021

Snuffleupagus commented Oct 29, 2021

Snuffleupagus Oct 29, 2021

calixteman Oct 29, 2021

Snuffleupagus Oct 29, 2021

janekotovich Oct 29, 2021

calixteman Oct 29, 2021

Snuffleupagus Oct 29, 2021

janekotovich commented Oct 29, 2021

Snuffleupagus commented Oct 29, 2021

janekotovich commented Oct 29, 2021

Snuffleupagus commented Oct 30, 2021

pdfjsbot commented Oct 30, 2021

pdfjsbot commented Oct 30, 2021

pdfjsbot commented Oct 30, 2021

pdfjsbot commented Oct 30, 2021

calixteman commented Oct 30, 2021

Snuffleupagus commented Oct 30, 2021

calixteman left a comment

JS - Avoid a popup to ask for specific version of Acrobat #14207

JS - Avoid a popup to ask for specific version of Acrobat #14207

Conversation

janekotovich commented Oct 29, 2021 • edited Loading

Because

Issue that this pull request solves

janekotovich commented Oct 29, 2021

Snuffleupagus commented Oct 29, 2021

Snuffleupagus Oct 29, 2021

Choose a reason for hiding this comment

calixteman Oct 29, 2021

Choose a reason for hiding this comment

Snuffleupagus Oct 29, 2021

Choose a reason for hiding this comment

janekotovich Oct 29, 2021

Choose a reason for hiding this comment

calixteman Oct 29, 2021

Choose a reason for hiding this comment

Snuffleupagus Oct 29, 2021

Choose a reason for hiding this comment

janekotovich commented Oct 29, 2021

Snuffleupagus commented Oct 29, 2021

janekotovich commented Oct 29, 2021

Snuffleupagus commented Oct 30, 2021

pdfjsbot commented Oct 30, 2021

From: Bot.io (Linux m4)

Received

pdfjsbot commented Oct 30, 2021

From: Bot.io (Windows)

Received

pdfjsbot commented Oct 30, 2021

From: Bot.io (Linux m4)

Success

pdfjsbot commented Oct 30, 2021

From: Bot.io (Windows)

Failed

calixteman commented Oct 30, 2021

Snuffleupagus commented Oct 30, 2021

calixteman left a comment

Choose a reason for hiding this comment

janekotovich commented Oct 29, 2021 •

edited

Loading