Problem/Motivation
In #3313946: Update CKEditor 5 to 35.2.1, a work-around had to be introduced because \Behat\Mink\Driver\Selenium2Driver::setValue()
does:
// 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.
}
… but blurring is not always a desired event. Really, Selenium should've been triggering the input
event instead.
Related, in #3316274: Stabilize FunctionalJavascript testing AJAX: add ::assertExpectedAjaxRequest() it was discovered that calling ::setValue()
may not trigger the necessary AJAX system updates, or at least not in time, because core/misc/form.js
does:
// Initialize form behaviors, use $.makeArray to be able to use native
// forEach array method and have the callback parameters in the right
// order.
$.makeArray($forms).forEach((form) => {
const events = 'change.formUpdated input.formUpdated ';
const eventHandler = debounce((event) => {
triggerFormUpdated(event.target);
}, 300);
formFields = fieldsList(form).join(',');
form.setAttribute('data-drupal-form-fields', formFields);
$(form).on(events, eventHandler);
});
In other words: it listens to the input
event, which never gets triggered, and it debounces it to allow updates at most every 300 milliseconds. But in tests there is no need for debouncing, since there is no continuous typing at all. In other words: it'd be safe to not debounce; which we can simulate by triggering formUpdated
immediately, which would in turn slightly speed up functional JS tests. This part is optional though.
Steps to reproduce
Proposed resolution
Override ::setValue()
's behavior.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#15 | interdiff-10-15.txt | 1.42 KB | nod_ |
#15 | core-3316816-15.patch | 3.78 KB | nod_ |
| |||
#10 | 3316816-10.patch | 5.29 KB | Wim Leers |
| |||
#10 | interdiff.txt | 629 bytes | Wim Leers |
#9 | 3316816-9.patch | 5.69 KB | Wim Leers |
|
Comments
Comment #2
Wim LeersThese revert the changes introduced in #3313946: Update CKEditor 5 to 35.2.1.
Comment #3
Wim LeersFix.
Comment #4
Wim Leers… and the optional
formUpdated
improvement that overrides the unnecessary debouncing.Comment #5
Wim LeersClosed #3316754: Mink/Selenium setBlur() setValue() method triggers a blur event as a duplicate.
Comment #9
Wim LeersWe need the
change
event too.See https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/input_event and https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/change_event.
Comment #10
Wim LeersNow let's run this against all of core's functional JS tests.
Comment #11
nod_That looks good to me
Comment #12
nod_Just need to remove the changes to drupalci.yml and I'd call that RTBC
Comment #13
nod_So there will be two triggers of formUpdated event, the one called here, and the one debounced from listening to input/change events.
Does it cause any problem to have the 300ms delay? I'd rather avoid triggering the event twice if we can help it.
So if I follow correctly, after calling blur one time, we restore the previous blur behavior.
That means that calling blur does not actually call blur the first time it triggers.
That should be fine
Comment #14
Wim Leers#13.1: it's a race condition that this removes: rather than waiting up to 300 ms (and that's not really guaranteed either, could be longer if DrupalCI is under heavy load), it's instantaneous. IOW: reduces likelihood of random failures.
Comment #15
nod_Comment #17
catchThis seems OK. Feel like there is a wider question of how much benefit we're getting from the chromedriver tests in general, but not for here.
Committed/pushed to 10.1.x and cherry-picked back through to 9.4.x, thanks!
Comment #18
Wim LeersYay! Already leading to simpler code elsewhere — see #3316274-49: Stabilize FunctionalJavascript testing AJAX: add ::assertExpectedAjaxRequest().
Comment #20
danflanagan8I had some FunctionalJavascript tests in contrib that started failing on October 26, 2022. (#3326427: Crossword Functional JS tests failing)
I managed to stumble upon this change and determined that this commit results in the failures. I've failed to find any way to get things passing again.