We'd like to support choosing files for a file input element for automation. Proposal: DOM.setFileInputFiles nodeId: <NodeId> files: Array<string> For the chromium port, we'd have to allow the renderer to access these files in the browser process.
Created attachment 187334 [details] Patch
Comment on attachment 187334 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=187334&action=review > Source/WebCore/inspector/Inspector.json:2106 > + "name": "setFileInputFiles", I am not sure this fits DOM domain well, but I have no better idea. > LayoutTests/inspector-protocol/dom-focus.html:-6 > -window.addEventListener("load", function() { Please move these in a separate change.
Created attachment 187613 [details] Patch
Neat. Is this intended to be protocol only (automating interaction with a page through the inspector protocol), or will there be UI for this in the frontend? Setting arbitrary file paths on an <input type="file"> through the protocol sounds like it could be dangerous. I'm thinking of remotely inspecting a mobile device where often the file system is obscured to users. Take iOS for instance, setting arbitrary paths through <input type="file"> is not possible, but if exposed through the protocol here, it would be. Maybe it should just be up to the port to protect against this?
(In reply to comment #4) > Neat. Is this intended to be protocol only (automating interaction with a page through the inspector protocol), or will there be UI for this in the frontend? > > Setting arbitrary file paths on an <input type="file"> through the protocol sounds like it could be dangerous. I'm thinking of remotely inspecting a mobile device where often the file system is obscured to users. Take iOS for instance, setting arbitrary paths through <input type="file"> is not possible, but if exposed through the protocol here, it would be. Maybe it should just be up to the port to protect against this? My intention is only for automation. I think handling the validity/security of the file upload could be done at the port level. Is this feasible for iOS with the current design?
(In reply to comment #5) > (In reply to comment #4) > > Neat. Is this intended to be protocol only (automating interaction with a page through the inspector protocol), or will there be UI for this in the frontend? > > > > Setting arbitrary file paths on an <input type="file"> through the protocol sounds like it could be dangerous. I'm thinking of remotely inspecting a mobile device where often the file system is obscured to users. Take iOS for instance, setting arbitrary paths through <input type="file"> is not possible, but if exposed through the protocol here, it would be. Maybe it should just be up to the port to protect against this? > > My intention is only for automation. I think handling the validity/security of the file upload could be done at the port level. Is this feasible for iOS with the current design? That sounds reasonable. The port should be security conscious if it needs to be. However, it would still be nice to opt out of this if needed. This could be done in the same way "canClearBrowserCache" and friends are done. An InspectorClient method, like: virtual bool canSetFileInputFiles() { return false; } And an optional protocol method to see if the feature is enabled. If not a protocol method, then an error string with your new method to let the that the front-end, or automated front-end, to know whether or not the feature is supported. This would let the front-end know immediately that things are not supported, instead of failing later attempting the upload.
> If not a protocol method, then an error string with your new method to let the that the front-end, or automated front-end, to know whether or not the feature is supported. Correction: If not a protocol method, then an error result when your new protocol method is called. This would let the front-end, or automated front-end, know whether or not the feature is supported. [...] Sorry for my poor grammar!
Created attachment 187711 [details] Patch
Comment on attachment 187711 [details] Patch Attachment 187711 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/16504359 New failing tests: inspector-protocol/dom/setFileInputFiles.html
Comment on attachment 187711 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=187711&action=review > LayoutTests/inspector-protocol/dom/setFileInputFiles.html:6 > +window.addEventListener("load", onLoad); To make it clear that onLoad is executed before runtTest I'd put runtTest() call at the end of onLoad and set onLoad as onload handler for <body> > LayoutTests/inspector-protocol/dom/setFileInputFiles.html:24 > + function abortOnError(msg) WebKit style guide discourages using abbreviations.
(In reply to comment #6) > However, it would still be nice to opt out of this if needed. This could be done in the same way "canClearBrowserCache" and friends are done. An InspectorClient method, like: > > virtual bool canSetFileInputFiles() { return false; } > Why not opt-in by default an have some ports to opt-out when needed?
(In reply to comment #11) > (In reply to comment #6) > > However, it would still be nice to opt out of this if needed. This could be done in the same way "canClearBrowserCache" and friends are done. An InspectorClient method, like: > > > > virtual bool canSetFileInputFiles() { return false; } > > > Why not opt-in by default an have some ports to opt-out when needed? I think opt-in makes more sense. This feature is only needed for automation, and that is not the focus for most ports. And as Joe mentioned this can be a security concern that would just be sitting there waiting to be exploited when it isn't needed by the port.
Created attachment 187887 [details] Patch
(In reply to comment #13) > Created an attachment (id=187887) [details] > Patch Addressed comments. Made default be to not allow setFileInputFiles. Enabled for chromium and added chromium-specific expectation.
Comment on attachment 187887 [details] Patch Clearing flags on attachment: 187887 Committed r143574: <http://trac.webkit.org/changeset/143574>
All reviewed patches have been landed. Closing bug.