fix: don't crash the tab when upload_file targets a chooser-opening element#2311
fix: don't crash the tab when upload_file targets a chooser-opening element#2311serhiizghama wants to merge 3 commits into
Conversation
Calling uploadFile on a proxy element (e.g. a button that opens the chooser via input.click()) sends DOM.setFileInputFiles to a non-input node, which kills the renderer (RESULT_CODE_KILLED_BAD_MESSAGE). The previous try/catch could not recover because the tab was already dead. Check whether the element is an <input type=file> first and drive the file chooser explicitly for everything else.
|
Thanks for the PR but I think the test passes without the fix (please see #2313) |
The previous test passed on unmodified source: the in-process browser throws when uploadFile hits a non-file element, so the old code recovered via the chooser and the assertion held either way. Stub uploadFile to a silent no-op so a chooser-opening button behaves like it does in headful Chrome (no throw, file never set), then assert uploadFile is never called on it and the chooser path delivers the file. Reverting the fix now fails the test.
|
You're right — the old test passed without the fix. The in-process test browser throws when Reworked the test to reproduce that: it stubs |
2 participants
Fixes #2310.
upload_filecallshandle.uploadFile()on whatever element you pass it first, and only falls back to the file chooser if that throws. But when the element is a proxy that opens the chooser (a button callinginput.click(), which is how most real upload UIs work),uploadFiledoesn't throw — puppeteer readselement.multiple(undefined on a button, no error) and then sendsDOM.setFileInputFilesto a non-input node, which kills the renderer withRESULT_CODE_KILLED_BAD_MESSAGE. The try/catch can't help because the tab is already gone, and the tool then reports a misleading "element no longer exists" error.The tool's own schema documents the chooser-opening element as supported, so the fix is to stop probing with
uploadFile: check whether the element is actually an<input type=file>and only use the direct path for those. Everything else goes straight to the click +waitForFileChooserpath it was always meant to use.All three existing upload_file tests still pass. Added one for a non-file
<input>to lock in that the check istype === 'file', not justinstanceof HTMLInputElement— otherwise a text input would still be routed into the crashing path.