Add tests for handling of HTML5 formaction and formmethod

[acoulton]

Currently MinkBrowserKit does not support the formaction / formmethod html5 attributes that control submission of the form depending on the clicked button. They are supported by all modern browsers.

This PR adds tests to go with the bugfix committed to the BrowserKit driver separately.

[aik099]

Does BrowserKit itself (not the MinkBrowserKitDriver) support them?

[acoulton]

@aik099 I don't think so - but anyway by the time BrowserKitDriver triggers the form submit, the context of what button was clicked has already been lost (within the mink code) so I don't think it could.

Currently BrowserKitDriver::click() just calls $this->submit($crawler->form();, which does some pre-processing of its own before calling BrowserKit\Client::submit($form). And all that does is grab the action, method and values from the passed-in form object and send a request with them.

[aik099]

Glad, that you see they way how this can be implemented.

[stof]

See https://github.com/symfony/symfony/pull/20467 for the support in BrowserKit directly (and without mutating nodes). This will be included in a future version of Symfony (probably not 3.2 as we are past the feature freeze, but 3.3 for sure).

We can also implement support for formmethod cleanly in the driver for any version of Symfony.

But implementing formaction support in the Mink layer is not possible without side effects (which could create bugs). So IMO, older versions of Symfony would not support it (a not so modern browser then).

[acoulton]

@stof thanks for adding the symfony PR - I hadn't realised the Form object was actually a button in this instance...

I appreciate technically this is a new feature for DomCrawler. But IMO it's still a bug in Mink. Any of the drivers that use real browsers - including Selenium with PhantomJS - handle the formaction and formmethod correctly unless you went out of your way to specify an ancient browser. Mink is therefore behaving inconsistently (and massively so, since it's posting to totally different URLs) across drivers.

AFAICS, it's only Goutte (which I'm actually using) and BrowserKit that don't properly implement the html5 spec - yet these attributes are just as widely supported as many of the other html5 attributes that have been implemented. It doesn't feel right that this can't be addressed till a symfony release in May 2017, tbh.

Worse, there are few good options for end-users to work around this in the meantime:

• If my site actually needed a fallback for the tiny number of ancient non-compliant real-world browsers (it doesn't), likely I would do that in JS, but that's obviously no use for this.
• I could implement some horrible old-school server-side re-despatch logic - but that's a lot of effort and means behat will actually be testing code paths that are never used in production.
• I could run all the scenarios in selenium/phantomjs - but that's a big performance hit just to be able to parse standard text html5 attributes.
• I could extend a custom driver - but since all the relevant parts of BrowserKitDriver are private, I'd have to be re-implementing big chunks of the driver and keeping that up to date manually for 6 months (and it doesn't help anyone else in the meantime).

Is there really no way this - especially formaction which is much more useful than formmethod - can be resolved ahead of the symfony 3.3 release?

I'm still not convinced there are significant risks of bugs/side-effects from mutating the nodes immediately before the http request is sent and they are all discarded anyway. When is there a case where they would ever be in-scope, accessed or used after the form submit is triggered?

If this can't be resolved in the short term, then at the very least I think BrowserKitDriver should throw an UnsupportedDriverException when clicking a button with either of these attributes so that it's clear the test code isn't doing what you think it is.

[stof]

@acoulton I agree that we should throw the UnsupportedDriverAction in this case. I will open a PR on BrowserKitDriver doing what can be done.

[stof]

Here is the driver support:

• Selenium2Driver: supported if the browser supports it, no UnsupportedDriverAction if unsupported but usage of the form attributes
• BrowserKitDriver: unsupported, behaving like old browsers currently (no exception)
• GoutteDriver: behavior inherited from BrowserKitDriver
• ZombieDriver: unsupported, behaving like old browsers currently (no exception)

In BrowserKitDriver, it could be possible to throw this exception (while painful to detect the version of Symfony DomCrawler supporting the feature to stop triggering the exception). But it won't be possible for Zombie and Selenium2 with an old browser. For Zombie, I requested the feature at https://github.com/assaf/zombie/issues/1094

[stof]

FYI, the third-party PhantomJSDriver also supports the feature.

[stof]

@aik099 what do you think about this ? Should we trigger unsupported exceptions or let it behave like older browsers (which is what will happen for real) ?

[acoulton]

@stof I think it's expected that the real-browser-based drivers will / won't support this behaviour depending on the browser you've configured it to use, without throwing.

If you're using Selenium, most likely it's because you want to know how a real browser will behave. If you a) have a project that uses these attributes and b) have deliberately configured selenium to use an old browser - and you have to go pretty old - then it's because you want to test how your application degrades in that browser. Just ignoring the attribute is the expected case here - then whatever fallback application code you have/haven't written will get triggered and your scenario will/won't pass in that browser accordingly.

With BrowserKit / Goutte / Zombie IMO you aren't as such interested in how a specific real browser behaves, you think you are whipping through your site with an emulation of a generic standards-compliant html5 browser. Obviously you don't get javascript with Goutte/BrowserKit, but that's a well-known and expected limitation.

In these browsers, not supporting formaction/formmethod is essentially the same as not properly supporting <form action=". The only difference is the age of the attribute.

If you're building a site that relies on these attributes and the browser emulator doesn't support them, that's a problem, and one that may be non-obvious to debug if it doesn't occur that the emulator just doesn't support them.