Remove syn (closes #5)

[uuf6429]

Fixes #5.

[uuf6429]

Other than the testBlur failure, there are 4 other failures caused by removing this line.

I already described the problem here. I could bring back $this->blur($xpath) here, but I believe this behaviour (and the failing tests) are wrong. Please let's decide on this since it's unclear for me how Mink should behave in this case. @stof

[aik099]

Here is what I remember (it might not be too detailed, but you can look it up via Git Blame in Selenium2Driver) behind the blur event firing from the setValue call:

1. we're sending the TAB key after a typed text to cause the native change event to be fired (if the value was actually changed)
2. then there was a bug in Chrome 53 that caused TAB to be displayed in the input
3. the above problem was fixed by firing the change event manually
4. unconditional change event firing caused another problem, that in some cases the change event was fired twice (the native event and our fake event)
5. to fix that we've replaced the change event firing with the blur event firing that in turn, if the element is still in focus, would cause the change event to be fired
6. this made testing of the auto-complete controls impossible because the blur event caused auto-completed suggestions to disappear
7. I don't know what happened with the auto-complete control testing, but no changes in the Mink Driver were made (probably some PR's are lying around asking to add a method, that does only sendKeys WebDriver call without any artifacts to Mink)

Current Selenium2 Driver code for the blur firing

// Remove the focus from the element if the field still has focus in
// order to trigger the change event. By doing this instead of simply
// triggering the change event for the given xpath we ensure that the
// change event will not be triggered twice for the same element if it
// has lost focus in the meanwhile. If the element has lost focus
// already then there is nothing to do as this will already have caused
// the triggering of the change event for that element.
$script = <<<JS
var node = {{ELEMENT}};
if (document.activeElement === node) {
  document.activeElement.blur();
}
JS;

// Cover case, when an element was removed from DOM after its value was
// changed (e.g. by a JavaScript of a SPA) and therefore can't be focused.
try {
    $this->executeJsOnElement($element, $script);
} catch (StaleElementReference $e) {
    // Do nothing because an element was already removed and therefore
    // blurring is not needed.
}

I'm not seeing that code in this driver anymore? Was it removed on purpose?

[uuf6429]

I've read your points several times and somehow it feels more convincing not to have the old/selenium2driver behaviour.

1. Sending tab automatically is unexpected/strange behaviour, which is bound to cause problems like point 2
2. Points 3 to 5 sounds like a hack and further fighting with unexpected and non-standard behaviour
3. Point 6 ("triggering blur causes problems"), seems to contradict the comment referring to it, where you seem to say that point 6 "is a good thing". Otherwise I don't get what you meant there.
4. Regarding point 7, it is in my opinion the responsibility of the user or library vendor. Not autocomplete components behave in the same way, and it would be way too much work for us to maintain that for them.

I'm not seeing that code in this driver anymore? Was it removed on purpose?

We kept the old logic of triggering blur (before removing syn), but not checking if the element has been removed - so maybe we might be missing a test for that.
But anyway, I don't know why this logic is significant/required. I don't expect "change" or "blur" events after changing some types of fields - I don't see why the driver/mink needs to do that.

If the driver user specifically needs that, they can just call the blur() after setValue() themselves. Deciding for them means that we will unconditionally affect everyone else, including those people that don't want this non-standard behaviour.

[aik099]

I've read your points several times and somehow it feels more convincing not to have the old/selenium2driver behaviour.

All these 7 points above are just a history to bring you up to speed with Selenium2 driver development.

As a Mink maintainer, my main concern is to make library users happier and ensure, that the library is evolving in the right direction. Ensuring that all drivers behave the same way, even if the test suite doesn't report any inconsistencies.

If the driver user specifically needs that, they can just call the blur() after setValue() themselves.

If we use raw Mink, then we probably can. I know, that the change/blur event firing thing isn't an expected behavior of the driver, but it's a good thing to have.

Maybe tab/blur/change code integrated into the setValue method is a leftover from times, when Mink was used only inside a Behat, where scenarios were saying to change field value and not to change field value and fire a change event.

[stof]

Setting the value without a change event being fired also breaks all cases where you have JS code running on the change event. That's why our testsuite ensures that setting the value actually reports it to JS that the value has changed.

There was a discussion a few years ago (that has never been implemented) about introducing a new Mink API to write in a field (which would not remove the existing text first and would keep the focus in the field) to allow testing such autocomplete-based UI.

[aik099]

Setting the value without a change event being fired also breaks all cases where you have JS code running on the change event. That's why our testsuite ensures that setting the value actually reports it to JS that the value has changed.

That's the \Behat\Mink\Tests\Driver\Js\ChangeEventTest::testSetValueChangeEvent test, which confirms, that the change event is being fired once after the setValue call.

[uuf6429]

Let's continue the discussion here: #21 (comment)