Update the events, used with scripting, to use lower-case names and avoid using DOM events internally in the viewer + misc scripting-related tweaks #12748
Conversation
Snuffleupagus
force-pushed
the
scripting-misc-fixes
branch
from
df97fda to
7427ce3
Compare
|
/botio unittest |
From: Bot.io (Linux m4)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/7a320c38782de47/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://3.101.106.178:8877/4f9a461b774cda8/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/7a320c38782de47/output.txt Total script time: 3.61 mins
|
From: Bot.io (Windows)SuccessFull output at http://3.101.106.178:8877/4f9a461b774cda8/output.txt Total script time: 4.71 mins
|
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/3000d2854969c94/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://3.101.106.178:8877/a6f96d195131a99/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/3000d2854969c94/output.txt Total script time: 2.52 mins
|
From: Bot.io (Windows)SuccessFull output at http://3.101.106.178:8877/a6f96d195131a99/output.txt Total script time: 3.53 mins
|
Snuffleupagus
force-pushed
the
scripting-misc-fixes
branch
2 times, most recently
from
bf7011a to
6b049bc
Compare
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/75b1f3669b95016/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://3.101.106.178:8877/b84b850ad0bad7d/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/75b1f3669b95016/output.txt Total script time: 2.52 mins
|
From: Bot.io (Windows)SuccessFull output at http://3.101.106.178:8877/b84b850ad0bad7d/output.txt Total script time: 3.55 mins
|
|
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/1e5106cbe665d19/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://3.101.106.178:8877/4213c1b992b6750/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/1e5106cbe665d19/output.txt Total script time: 25.79 mins
Image differences available at: http://54.67.70.0:8877/1e5106cbe665d19/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://3.101.106.178:8877/4213c1b992b6750/output.txt Total script time: 26.91 mins
Image differences available at: http://3.101.106.178:8877/4213c1b992b6750/reftest-analyzer.html#web=eq.log |
…mponents and the `AnnotationLayer` - Actually remove the `isDown` property when destroying the scripting-instance. - Mark all `mouseState` usage as "private" in the various classes. - Ensure that the `AnnotationLayer` actually treats the parameter as properly *optional*, the same way that the viewer components do. - For now remove the `mouseState` parameter from the `PDFPageView` class, and keep it only on the `BaseViewer`, since it's questionable if all of the scripting-functionality will work all that well without e.g. a full `BaseViewer`. - Append the `mouseState` to the JSDoc for the `AnnotationElement` class, and just move its definition into the base-`AnnotationElement` class.
…Element.{_setEventListener, _setEventListeners}` methods
- Update the `LinkAnnotationElement._bindJSAction` call-site to actually agree with the JSDocs, by passing in the `data`.
- Prevent the links created by `LinkAnnotationElement._bindJSAction` from being displayed with empty hashes; compare with e.g. `LinkAnnotationElement. _bindNamedAction`.
- The overall indentation-level in `WidgetAnnotationElement._setEventListener` can be reduced slightly by using early returns, which improves the overall readability of this method a bit. (We're also able to avoid unnecessary `in` usage here.)
- The code can also be made *slightly* more efficient overall, by moving the `this.data.actions` check into `WidgetAnnotationElement._setEventListeners` instead. This way we can avoid useless `this._setEventListener`-calls when there are no actions present.
…void using DOM events internally in the viewer For DOM events all event names are lower-case, and the newly added PDF.js scripting-events thus "stick out" quite a bit. Even more so, considering that our internal `eventBus`-events follow the same naming convention. Hence this patch, which changes the "updateFromSandbox"/"dispatchEventInSandbox" events to be lower-case instead. Furthermore, using DOM events for communication *within* the PDF.js code itself (i.e. between code in `web/app.js` and `src/display/annotation_layer.js/`) feels *really* out of place. That's exactly the reason that we have the `EventBus` abstraction, since it allowed us to remove prior use of DOM events, and this patch thus re-factors the code to make use of the `EventBus` instead for scripting-related events. Obviously for events targeting a *specific element* using DOM events is still fine, but the "updatefromsandbox"/"dispatcheventinsandbox" ones should be using the `EventBus` internally. *Drive-by change:* Use the `BaseViewer.currentScaleValue` setter unconditionally in `PDFViewerApplication._initializeJavaScript`, since it accepts either a string or a number.
There's no really compelling reason, as far as I can tell, to introduce the `ENABLE_SCRIPTING` build-target, instead of simply re-using the existing `TESTING` build-target for the new `gulp integrationtest` task. In general there should be no problem with just always enable scripting in TESTING-builds, and if I were to *guess* the reason that this didn't seem to work was most likely because the Preferences ended up over-writing the `AppOptions`. As it turns out the GENERIC-viewer has already has built-in support for disabling of Preferences, via the `AppOptions`, and this can be utilized in TESTING-builds as well to ensure that whatever `AppOptions` are set they're always respected.
Snuffleupagus
force-pushed
the
scripting-misc-fixes
branch
from
6b049bc to
66e9d4a
Compare
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/716594cf98701ef/output.txt Total script time: 2.53 mins
|
From: Bot.io (Windows)SuccessFull output at http://3.101.106.178:8877/8b70b88f57ce8b0/output.txt Total script time: 3.51 mins
|
|
/botio integrationtest |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/bc93a8d57e5f495/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://3.101.106.178:8877/8ee30bfc79a49a9/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/bc93a8d57e5f495/output.txt Total script time: 2.58 mins
|
From: Bot.io (Windows)SuccessFull output at http://3.101.106.178:8877/8ee30bfc79a49a9/output.txt Total script time: 3.51 mins
|
|
/botio integrationtest |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/16581d9d5b3677e/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://3.101.106.178:8877/09570ce0d8fb031/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/16581d9d5b3677e/output.txt Total script time: 2.49 mins
|
From: Bot.io (Windows)SuccessFull output at http://3.101.106.178:8877/09570ce0d8fb031/output.txt Total script time: 3.55 mins
|
|
/botio integrationtest |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/55581f0ac24891d/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://3.101.106.178:8877/5e9e271d23dd2b2/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/55581f0ac24891d/output.txt Total script time: 2.59 mins
|
From: Bot.io (Windows)SuccessFull output at http://3.101.106.178:8877/5e9e271d23dd2b2/output.txt Total script time: 3.54 mins
|
|
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/b8e2adfe6e8546b/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://3.101.106.178:8877/b3a1cea8af29e93/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/b8e2adfe6e8546b/output.txt Total script time: 25.81 mins
Image differences available at: http://54.67.70.0:8877/b8e2adfe6e8546b/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://3.101.106.178:8877/b3a1cea8af29e93/output.txt Total script time: 27.11 mins
Image differences available at: http://3.101.106.178:8877/b3a1cea8af29e93/reftest-analyzer.html#web=eq.log |
|
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/82904dcec180ef9/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/82904dcec180ef9/output.txt Total script time: 4.15 mins Published |
|
Thank you for improving the overall consistency here! |
Please refer to the individual commit messages for additional information.
Also, ignoring white-space changes when viewing the larger patches should be helpful.