-
Notifications
You must be signed in to change notification settings - Fork 10k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[JS] Use beforeinput event to trigger a keystroke event in the sandbox #14430
Conversation
/botio integrationtest |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @calixteman received. Current queue size: 0 Live output at: http://54.241.84.105:8877/1fcbc73bff57fe0/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @calixteman received. Current queue size: 0 Live output at: http://54.193.163.58:8877/fb1d6a2ea91bd76/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/1fcbc73bff57fe0/output.txt Total script time: 4.94 mins
|
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/fb1d6a2ea91bd76/output.txt Total script time: 6.98 mins
|
/botio integrationtest |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/da1f3d0d2f08a99/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/1952160057b40c5/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/da1f3d0d2f08a99/output.txt Total script time: 4.83 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.193.163.58:8877/1952160057b40c5/output.txt Total script time: 6.01 mins
|
There is definitely something wrong on linux. I wrote the tests under windows... |
And of course I can't reproduce the issue locally on debian... |
c5c539d
to
b3ad48c
Compare
/botio integrationtest |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @calixteman received. Current queue size: 0 Live output at: http://54.241.84.105:8877/68d25f9889a1b82/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @calixteman received. Current queue size: 0 Live output at: http://54.193.163.58:8877/6a641ac014484ae/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/68d25f9889a1b82/output.txt Total script time: 3.95 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.193.163.58:8877/6a641ac014484ae/output.txt Total script time: 6.21 mins
|
/botio integrationtest |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @calixteman received. Current queue size: 0 Live output at: http://54.193.163.58:8877/2d21e467c69e720/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @calixteman received. Current queue size: 0 Live output at: http://54.241.84.105:8877/da1d48865993413/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/da1d48865993413/output.txt Total script time: 3.94 mins
|
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/2d21e467c69e720/output.txt Total script time: 4.27 mins
|
/botio integrationtest |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @calixteman received. Current queue size: 0 Live output at: http://54.241.84.105:8877/94f044bd3bc0128/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @calixteman received. Current queue size: 0 Live output at: http://54.193.163.58:8877/f7e81367f97af77/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/94f044bd3bc0128/output.txt Total script time: 4.03 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.193.163.58:8877/f1bad878264b381/output.txt Total script time: 6.61 mins
|
/botio integrationtest |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @calixteman received. Current queue size: 0 Live output at: http://54.241.84.105:8877/f836e3a1eb57785/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @calixteman received. Current queue size: 0 Live output at: http://54.193.163.58:8877/34d3a4f209a3eb1/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/f836e3a1eb57785/output.txt Total script time: 3.89 mins
|
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/34d3a4f209a3eb1/output.txt Total script time: 6.73 mins
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r=me, with one final comment/question addressed; thank you!
@@ -56,7 +56,7 @@ class GenericScripting { | |||
|
|||
async dispatchEventInSandbox(event) { | |||
const sandbox = await this._ready; | |||
sandbox.dispatchEvent(event); | |||
setTimeout(() => sandbox.dispatchEvent(event), 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, that was just a bit surprising since naively I'd have assumed that the previous line (where we await a Promise) already introduces asynchronicity in the event dispatching.
Anyway, if we're adding this additional "delay" to the event dispatching there's potentially a slightly larger risk than before that we're now attempting to dispatch an event after destruction have started.
Given all the various dispatchEventInSandbox
call-sites it's difficult to easily tell what an error could lead to, so how about also changing
Line 106 in 23b6fde
this.support.callSandboxFunction("dispatchEvent", event); |
into the following to prevent any future errors elsewhere:
this.support?.callSandboxFunction("dispatchEvent", event);
When entering
and in Firefox:
When adding the |
94b4715
to
51a64b3
Compare
/botio integrationtest |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @calixteman received. Current queue size: 0 Live output at: http://54.241.84.105:8877/1048d0bbcb832f1/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @calixteman received. Current queue size: 0 Live output at: http://54.193.163.58:8877/6e5f1496d99f3b8/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/1048d0bbcb832f1/output.txt Total script time: 4.86 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.193.163.58:8877/6e5f1496d99f3b8/output.txt Total script time: 5.68 mins
|
- it aims to fix issue mozilla#14307; - this event has been added recently in Firefox and we can now use it; - fix few bugs in aform.js or in annotation_layer.js; - add some integration tests to test keystroke events (see `AFSpecial_Keystroke`); - make dispatchEvent in the quickjs sandbox async.
51a64b3
to
6ac296e
Compare
/botio integrationtest |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @calixteman received. Current queue size: 0 Live output at: http://54.241.84.105:8877/3c60402e66e9e4f/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @calixteman received. Current queue size: 0 Live output at: http://54.193.163.58:8877/46ae0a192453b60/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/3c60402e66e9e4f/output.txt Total script time: 3.92 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.193.163.58:8877/46ae0a192453b60/output.txt Total script time: 6.66 mins
|
/botio integrationtest |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @calixteman received. Current queue size: 0 Live output at: http://54.241.84.105:8877/8f38bba25094c54/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @calixteman received. Current queue size: 0 Live output at: http://54.193.163.58:8877/000ddaae2924129/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/8f38bba25094c54/output.txt Total script time: 3.99 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.193.163.58:8877/000ddaae2924129/output.txt Total script time: 6.62 mins
|
AFSpecial_Keystroke
);