Skip to content
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

Special-casing file upload controls in keyboard-interactability check #1230

Closed
andreastt opened this issue Feb 21, 2018 · 29 comments
Closed

Comments

@andreastt
Copy link
Member

andreastt commented Feb 21, 2018

It is apparently the case that a range of “modern” JS frameworks hides <input type=file> and provides indirection for selecting files via alternate UX elements. When using Element Send Keys to upload files to the file upload element WebDriver will typically complain that the element is not interactable, because the JS framework will have hidden the element from view (display: none).

I have not yet received a reproducible test case for this, so it’s unclear to me if it is possible to write automation code to interact witht he UX element to indirectly set the file on the file upload element or not. If it is true that this is impossible, we should consider how we can make it possible to use Element Send Keys to upload files when the file upload element is hidden from view.

At the moment I can’t think of any better solutions that to special case <input type=file> in the keyboard-interactability check so that we always treat it as interactable. This would enable users to upload files to non-displayed elements.

Can someone who is more JS-savvy confirm how file upload control indirection works in practice?

See also:

@shs96c
Copy link
Contributor

shs96c commented Mar 13, 2018

I think it's wise to get a reproducible test case for this, and then see if we can interact with the alternative elements. The two cases listed there show people attempting to interact with the file input element, but not (I think) with whatever the user actually interacts with to upload elements.

I wonder what chromedriver does, since some folks are reporting that works for them.

@twalpole
Copy link

twalpole commented Mar 13, 2018

A common approach is to make the file input transparent and overlay it over the frameworks alternative elements. That way a click "on" the alternative elements actually clicks the file input and opens the file chooser dialog. If the file chooser dialog was treated similar to a JS prompt dialog and allowed to set the files being selected when it displays then it would solve all the issues (would require WebDriver spec update though). The issue with just treating a hidden file input as always accessible (which chromedriver appears to do) is that you're not then actually testing the page as a user would be able to use the page since it may be able to add files when a user really should not be able to.

@shs96c
Copy link
Contributor

shs96c commented Mar 13, 2018

I like the idea of being able to interact with the file upload dialog as if it were a modal prompt. I do worry about how easy it is to implement.

@kerrykimbrough
Copy link

kerrykimbrough commented Mar 14, 2018

As the one who initiated this issue at mozilla/geckodriver, I agree that the problem would be better solved by a standard WebDriver API for recognizing a file chooser dialog and setting the file it returns, as if entered by a user.

I don't think a "reproducible case" will reveal any better solutions. As twalpole suggests, whenever the <input type=file> is hidden, whatever users actually interact with must ultimately act directly on the DOM to activate that element and present the file chooser dialog. But at that point, a WebDriver client program is helpless.

The sendKeys hack is a way avoid the file chooser dialog altogether. But it would be better if a WebDriver client could control it.

@twalpole
Copy link

twalpole commented Mar 14, 2018

@kerrykimbrough Just a quick fix - "setting the FILES it returns" -- anything around the file input needs to support the selection of multiple files to be useful for modern testing.

@AlexanderArendar
Copy link

AlexanderArendar commented Mar 23, 2018

@shs96c I am not sure what exactly chromedriver does technically, but for me as a developer of automated tests, it gives ability to automate. My concrete example right now is a project where users can create events, part of events data is event logo, event favicon and event background image. So there are just 3 file inputs with all the Agular JS/CSS stuff wrapped around them which makes the inputs themselves not intractable. With chromedriver I just can use sendKeys and have my basic test case of event creation/editing automated. With geckodriver I just check if(driver.instanceOf[FirefoxDriver]) and fail major part of test with the message "can not be automated in Firefox".
So you must understand that such cases are typical and you don't need to philosophize if it would be better to have a reproducible case or if we automation engineers are really trying to automate exactly what users are doing or not. With the current approach geckodriver JUST DON'T LET YOU AUTOMATE TYPICAL CASES FOR FILE UPLOAD. We are not living in 1990-s where just pure HTML file inputs are common thing.
Don't get me wrong, I am not complaining. If you look I was actively commenting the source tickets referenced in this one. But already a month passed and the ticket is still in a state where nothing helpful was suggested. So if you guys take path to discuss for months I believe me and others will just consider geckodriver not suitable and that would be a real PITA because no alternatives exist for Firefox. After all geckodriver should be focused on providing way to automate, not to restrict basic use cases of automation.

@twalpole
Copy link

twalpole commented Mar 23, 2018

@AlexanderArendar Yes - chromedriver makes it work, but you need to realize that you're not actually testing the page then. If all the Angular JS/CSS stuff wrapped around the inputs was broken, hidden, etc your tests would still pass even though a user of the page would be unable to use it - not exactly the best position to be in.

@AlexanderArendar
Copy link

AlexanderArendar commented Mar 23, 2018

@twalpole, I agree with this. But again, in 90% or more while things are not broken I would be able to test if the user can create/edit event, if user can create event request, etc. At the moment I just can't do this in Firefox at all. Even when nothing ever breaks in input file UI wrappers I am blocked and can't cover really big part of my app functionality.
In my other project there was similar situation with CSV upload for price management module. I've been working on this project for a while and again, nothing ever broken in particular UI controls for CSV upload, but I am not able to cover tests for prices import/update in Firefox.
So what good is that for to defend such approach?

@twalpole
Copy link

twalpole commented Mar 23, 2018

@AlexanderArendar "what good is that for to defend such approach?" - the flip side to that is what good is it to argue for an approach that means tests don't let you know when your app is broken? The reality is there needs to be a better approach than just special casing file inputs to ignore visibility.

Additionally this repo is for the W3C WebDriver spec (not geckodriver). Expecting any W3C spec - which relies on consensus of multiple people - to be changed in a month is wishful thinking at best. If you need to test your pages using geckodriver today then in your tests you'll need to modify the CSS of the file inputs using JS so they become visible.

@AlexanderArendar
Copy link

AlexanderArendar commented Mar 23, 2018

@twalpole, in real world, it is very often that you cannot just "modify the CSS of the file inputs" because you work with already existing app and how UI looks is not something QA can and should impact.
Also I am not against the "better approach". But question is when will it materialize, right? I am just a man from the field who tries to use existing toolkit and provides some feedback.
And finally, recommending me to "modify the CSS of the file inputs using JS" would not it then make me test not what real users have in their browsers? Looks like your argument is then biased.
Probably I have nothing more to add, so have a good day and I hope W3C team will eventually come up to some good solution.

@twalpole
Copy link

twalpole commented Mar 23, 2018

@AlexanderArendar Of course you can ALWAYS modify the CSS of the file input via JS, as I mentioned, from inside your test - It's even mentioned in the first geckodriver issue linked in this issue description. Yes this would make it test what real users can't do, but that's exactly what the current chromedriver approach you seem to want put into the spec does too, but in this case you would have to proactively do something thereby indicating you know you're not actually testing what a user can do.

@andreastt
Copy link
Member Author

andreastt commented Mar 28, 2018

This discussion appears to have derailed quite substantially. Let me try to bring it back on track.

@shs96c said he liked the idea of being able to interact with the file upload dialogue “as if it were a modal prompt”. I can say straight away that this is not viable because browsers use native widget toolkits to present the dialogue over which the browser has no control. Control is only returned to the browser event loop after the dialogue closes.

I think if we were designing this API from scratch a new endpoint such as /session/{session id}/element/{element id}/upload would make sense. This would bypass the keyboard interactability checks associated with the Element Send Keys command, and also avoid the cognitive overload that interacting with <input type=text> has a different meaning and effect than interacting with <input type=file>. I think many users are also confused by how multiple repeated calls to <input type=file multiple> appends to the FileList.

However, we need to deal with realities. It is expected behaviour that a conforming WebDriver implementation must be able to upload a file by using Element Send Keys to an <input type=file> element. It is incorrect, as some of the comments in this discussion imply, that geckodriver does not support this. The current situation is that chromedriver does not conform to the specification by honouring the keyboard-interactability promise of Element Send Keys, but the discussion is about whether we should change the specification to match chromedriver’s behaviour.

In order to move forward on this, we require two things:

  1. Clarity into what chromedriver does when Element Send Keys is called on <input type=file> and the form control is not keyboard-interactable, e.g. display: none.
  2. A reproducible test case showcasing the Angular/React problem.

Perhaps @kereliuk can help explain how chromedriver works, and it would also be useful with input from the other spec editor, @AutomatedTester.

@QAdamek
Copy link

QAdamek commented Mar 29, 2018

@andreastt Good day.

My name is Adam Domzalski and currently I work as a QA / automation guy.
Since yesterday I have EXACTLY the same problem:

  • Frontend written in React.JS
  • <input type="file" multiple="" style="display: none;">
  • Chrome can upload file using sendKeys()
  • Firefox can't

If you want me to get some data I would love to help.

@twalpole
Copy link

twalpole commented Mar 29, 2018

@andreastt The quote about "as if it were a modal prompt" doesn't mean the browser has to actually open the native widget. The driver could intercept that and create an object that behaves the same way modal dialogs do in the API - i.e. when the file picker would normally be opened, provide an object that can be switched to and have the files to upload be set.

As for "I think many users are also confused by how multiple repeated calls to appends to the FileList" that's because appending makes no sense when the user can set multiple files in the call (as defined in the spec) - geckodriver/marionette is non-compliant with the spec on this currently (multiple files can't currently be set in one call).

For the other points

  1. Chromedriver just ignores keyboard interact-ability for file inputs

file inputs are special cased here - https://chromium.googlesource.com/chromium/src/+/master/chrome/test/chromedriver/element_commands.cc#320 - while non file input elements are checked here - https://chromium.googlesource.com/chromium/src/+/master/chrome/test/chromedriver/element_commands.cc#45

  1. It's not specifically an Angular/React problem - it's any framework/widget/page that makes the file input non-interactable but provides other elements which trigger the non-interactable file input to open the file picker

@andreastt
Copy link
Member Author

andreastt commented Mar 29, 2018

The quote about "as if it were a modal prompt" doesn't mean the browser has to actually open the native widget.

Right, this is a distinct possibility. We appear to be discussing two things here though: one is what a new, better API for interacting with file form controls would look like, and the other what the behaviour of the existing Element Send Keys should be.

We need to reach consistency with the latter first, but I’m sympathetic to the idea of treating file uploads as a quasi-modal dialogue.

As for "I think many users are also confused by how multiple repeated calls to appends to the FileList" that's because according to the spec it shouldn't be appending to the FileList - geckodriver/marionette is non-compliant with the spec on this.

I think you have misread the specification then. The remote end steps of Element Send Keys step 10 substep 6 says:

Complete implementation specific steps equivalent to setting the selected files on the input. If multiple is true files are be appended to element’s selected files.

If Chrome does not implement this behaviour either, there are in fact two conformance problems we have to resolve here…

Thanks for your link to the Chromium source code.

@twalpole
Copy link

twalpole commented Mar 29, 2018

@andreastt Yes I had missed that in the spec - I had updated my post, while you were replying I believe, since appending doesn't make sense when the specs already defines the ability to send multiple files at once. Appending also doesn't replicate any behavior a user could actually do in any current browser (from an events triggered perspective), selecting files just replaces those that were previously selected, so is relatively useless for testing purposes. Chrome does implement the ability to upload multiple files in one send keys command (geckodriver/ff does not).

@florentbr
Copy link

florentbr commented Mar 30, 2018

An <input type="file"> doesn't need to be interractable to be updated by a real user via the file picker from the OS. So expecting the input to be interractable via keyboard is simply wrong.

This example reproduces the issue where the <input type="file"> is not interactable :

<body>
    <form>
        <input type="button" value="Upload" onclick="this.nextElementSibling.click()">
        <input type="file" name="upload" style="display:none;">
    </form>
</body>

With chromedriver, the driver simply assigns the file to the input and then emits the change event.

With marionette, it's not possible to assign the file since the input is not visible. It's technically possible by changing the style of the input to make it visible, but it will alter the content of the page which is not desired.

Note that it's possible to simulate a file upload just like a real user by clicking on the button presented to the user event though it's not a file input. The idea is to intercept the click once it reaches the file input, to assign the files and to cancel the event to avoid the filepicker from the browser.

This example with Firefox injects some code in the context of the browser to automatically assign the file input:

from selenium import webdriver
from selenium.webdriver.remote.webelement import WebElement
from urllib.parse import quote
import os.path


def upload_moz(element, *paths):
    driver = element.parent
    
    # ensure the files are present, and upload to the remote server if it's a remote session
    
    for i in range(len(paths)):
        if not os.path.isfile(paths[i]) :
            raise FileNotFoundError(paths[i])
        if driver._is_remote:
            paths[i] = element._upload(paths[i])
    
     # switch to browser context
    
    driver.set_context("chrome")
    
    # listen to the click event and assign the files to the file input when it receives the click
    
    driver.execute_script("""
        let paths = arguments[0].join(';');

        gBrowser.selectedBrowser.messageManager.loadFrameScript(encodeURI(`data:,(function(){
            content.document.addEventListener('click', function listener(evt) {
                if (evt.target.type === 'file') {
                    content.document.removeEventListener('click', listener);
                    evt.preventDefault();

                    Components.utils.importGlobalProperties(['File']);
                    let paths = '${paths}'.split(';');
                    Promise.all(paths.map(path => File.createFromFileName(path))).then(files => {
                        evt.target.mozSetFileArray(files);
                        evt.target.dispatchEvent(new content.Event('change'));
                    });
                }
            });
        })()`), false);
        """, paths)
    
    # get back to content context
    
    driver.set_context("content")
    
    # trigger a click which should propagate to the file input and be intercepted by the listener
    
    element.click()

WebElement.upload = upload_moz

Usage :

driver = webdriver.Firefox()

driver.get('data:text/html;charset=utf-8,' + quote("""
  <!DOCTYPE html>
  <html lang="en">
    <body>
        <form>
            <input type="button" value="Upload" onclick="this.nextElementSibling.click()">
            <input type="file" name="upload" style="display:none;">
        </form>
    </body>
  </html>
  """))

driver.find_element_by_css_selector("[value=Upload]") \
    .upload('/tmp/image1.png', '/tmp/image2.png')

@twalpole
Copy link

twalpole commented Mar 30, 2018

@florentbr Yes, in your example chromedriver, by ignoring inter-actibility allows to update the file input which a user could do by clicking the button - however if the HTML were

<body>
  <form>
    <input type="file" name="upload" style="display:none;">
  </form>
</body>

chromedriver would allow updating that file input too, which a user would have no way to do.

@florentbr
Copy link

florentbr commented Mar 30, 2018

@twalpole, imo it doesn't matter since the interractibility can be evaluated with an additional assertion if it's your concern. On the other hand by restricting the interractibility, you are limiting the upload to a specific usage and making it impossible to upload a file on an hidden input, which is a valid usage.
So having to choose between two wrong, I would choose the one that makes it possible to get it right with some extra steps, which is in this case the implementation of chromedriver.

Now to address the interractibility, I see 2 options:

  • handle the file picker like an Alert. Though the dialog is from the OS which mean that it's going to be tricky and it will require different implementations for the different OS. So I don't see an implementation coming anytime soon.

  • intercept the file picker to cancel it and to assign the files directly. This requires to know the files beforehand, which mean having a specific command like in the example from my previous comment.

@whimboo
Copy link
Collaborator

whimboo commented Apr 3, 2018

@twalpole Last week I field bug 1448792, which will fix the issue with multiple file uploads in geckodriver. I hope that I will have time soon to get it implemented.

@whimboo
Copy link
Collaborator

whimboo commented May 30, 2018

Here just a working example of an <input type=file> element which is hidden and used by a styled div` for a file upload: https://output.jsbin.com/votumokoki/1

@shreyanshp
Copy link

shreyanshp commented Jun 18, 2018

@andreastt Thank you for raising this !

My 2 cents here -

While I believe that this is not a simple problem, and I completely disagree with arguments like 'since chromedriver does it', we may have to think of a solution that makes logical sense, and supports both cases

  • When element which is hidden
  • When element which is not hidden

As much as I like to adhere to standards and automating in the right way (that is mimicking human interactions and not just doing a dirty shortcut), this issue has been causing a lot of trouble to me for past 6 months, so anything that I can do, please let me know

@whimboo
Copy link
Collaborator

whimboo commented Jun 26, 2018

@shreyanshp if that is only a problem for you with geckodriver please check mozilla/geckodriver#1173 (comment) for a workaround.

@robsco-git
Copy link

robsco-git commented Jul 27, 2018

Here's the dirty hack I used:

// Display the input[type="file"]
if (BROWSER === 'firefox') {
  await driver.executeScript(
    `document.querySelector('input[type="file"]').style.display = "block";`
  );
}

// Set the selected file
const el = await driver.findElement(...);
await el.sendKeys(...);

// Hide the input[type="file"]
if (BROWSER === 'firefox') {
  await driver.executeScript(
    `document.querySelector('input[type="file"]').style.display = "none";`
  );
}

At least Chrome and Firefox now pass the same tests...

@SetTrend
Copy link

SetTrend commented Jul 29, 2018

I feel we need a standardized solution, working on each and every browser - always and regardless of the way the browser interacts with the user to show a File Open dialog.

Requiring the tester to manually manipulating the CSS rules to make a common test case work sounds rather like a cumbersome dirty hack than a true and clean solution.

As I suggested in #1281, please consider to add the ability to set values of hidden elements by WebDriver. That solution would solve a number of issues.

@whimboo
Copy link
Collaborator

whimboo commented Oct 26, 2018

Here the updated testcase: https://jsbin.com/zopicoxeba/1

@andreastt
Copy link
Member Author

andreastt commented Oct 29, 2018

The WG decided at TPAC in Lyon 2018 to relax interactability tests for <input type=file> to allow Firefox to reach conformance with Chrome. We will then introduce a new web element-scoped Upload File command that will work independently of Element Send Keys and have a saner API. Additionally we will add a strictFileInteractability capability defaulting to false which will enable the current, stricter behaviour.

@andreastt
Copy link
Member Author

andreastt commented Oct 29, 2018

The relevant specification changes are being made in #1325. We still need a PR for adding the new Upload File command endpoint.

jgraham added a commit that referenced this issue Oct 31, 2018
This breaks cases where authors have hidden the file input and
interact with it via js. Also add a strictFileInteractabilityCheck
capability to re-enable the previous behaviour of performing the
checks always.

Fixes: #1230
@whimboo
Copy link
Collaborator

whimboo commented Nov 7, 2018

Note that I filed issue #1355 for the new file upload command.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests