Problem/Motivation
(At least) Since olivero became the new default theme in the standard profile in #3219959: Update standard profile so Olivero is the default theme, we're seeing a (dramatic) increase of test-failures in \Drupal\Tests\quickedit\FunctionalJavascript\SettingsTrayIntegrationTest like these:
1) Drupal\Tests\quickedit\FunctionalJavascript\SettingsTrayIntegrationTest::testQuickEditLinks
WebDriver\Exception\UnknownError: element click intercepted: Element ... is not clickable at point (504, 188). Other element would receive the click: ...
(Session info: headless chrome=98.0.4758.102)
(Driver info: chromedriver=98.0.4758.102 (273bf7ac8c909cde36982d27f66f3c70846a3718-refs/branch-heads/4758@{#1151}),platform=Linux 4.9.0-0.bpo.6-amd64 x86_64)
/var/www/html/vendor/instaclick/php-webdriver/lib/WebDriver/Exception.php:156
/var/www/html/vendor/instaclick/php-webdriver/lib/WebDriver/AbstractWebDriver.php:165
/var/www/html/vendor/instaclick/php-webdriver/lib/WebDriver/AbstractWebDriver.php:245
/var/www/html/vendor/instaclick/php-webdriver/lib/WebDriver/Container.php:239
/var/www/html/vendor/behat/mink-selenium2-driver/src/Selenium2Driver.php:806
/var/www/html/vendor/behat/mink-selenium2-driver/src/Selenium2Driver.php:792
/var/www/html/core/tests/Drupal/FunctionalJavascriptTests/DrupalSelenium2Driver.php:146
/var/www/html/core/tests/Drupal/FunctionalJavascriptTests/DrupalSelenium2Driver.php:205
/var/www/html/core/tests/Drupal/FunctionalJavascriptTests/DrupalSelenium2Driver.php:157
/var/www/html/vendor/behat/mink/src/Element/NodeElement.php:153
/var/www/html/core/modules/settings_tray/tests/src/FunctionalJavascript/SettingsTrayTestBase.php:54
/var/www/html/core/modules/quickedit/tests/src/FunctionalJavascript/SettingsTrayIntegrationTest.php:117
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:726
Why?
Over on #3219959-83: Update standard profile so Olivero is the default theme I've created a patch that shows a screenshot of (what I think is) the issue, attached it here as well.
The QuickEdit Toolbar (barely) covers a little bit of the "Powered By"-block. (Depending on the characters in the name of the user that was created to create the blocks, there's the randomness.)
We perform a click on that block. That's a rather big place to do a click on, somehow the click() command seems to like to click exactly on the tiny spot that is covered by the QuickEdit-toolbar. Which in turn causes the block not receiving the click like the error above states.
Proposed resolution
There's a difference in the number of blocks in the core/themes/olivero/config/optional directory between 9.4/9.5 and 10.0. In #3219959: Update standard profile so Olivero is the default theme we moved blocks for 9.4 and 9.5, whilst in 10.0 we copied them.
Looking at the screenshots I've made locally and attached here for Olivero and Bartik whilst running \Drupal\Tests\quickedit\FunctionalJavascript\SettingsTrayIntegrationTest, copying looks better and passes the test.
Let's restore the blocks in core/themes/olivero/config/optional so we have:
- the same amount of blocks between 9.4/9.5 and 10.0
- a passing \Drupal\Tests\quickedit\FunctionalJavascript\SettingsTrayIntegrationTest
Previously attempted resolution
Move the block that is tested to be the bottom block on the page. The QuickEdit Toolbar is always _above_ the field that is being edited. If our block is below it, it will never be covered.
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | d94-screen-olivero.png | 31.33 KB | spokje |
| #14 | d10-screen-olivero.png | 42.27 KB | spokje |
| #14 | d94-screen-bartik.png | 22.17 KB | spokje |
| #14 | d10-screen-bartik.png | 41.88 KB | spokje |
| #4 | screen-pre-#block-nwuzhnat-1651137598[1].png | 32.09 KB | spokje |
Issue fork drupal-3278215
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
spokjeComment #3
eojthebraveWe fixed another section of this test when Olivero became the default theme. Here https://www.drupal.org/project/drupal/issues/3219959#comment-14498697
I'm not sure if it's related or not, but in case it is. Line 118 of the current test is:
Which is almost identical to the lines we changed just below that to:
Comment #4
spokjeComment #5
spokjeComment #6
spokjeComment #7
spokjeNote: The patches change
core/drupalci.yml, please use the MR for committing (if it comes to that).Comment #8
spokjeComment #10
spokje9.3.x-devis not affected, since [##3219959] was only committed to 9.4, 9.5 and 10.0.No clue why
10.0.x-devis not showing the test failures, but then again: The fix also doesn't show test-failures, so it isn't breaking anything there.Comment #11
spokjeLet's postpone this on #3278342: Theming blocks bonanza, that _might_ solve the root cause, instead of fighting the fallout in here.
Comment #12
spokjeBack to NR, since #3278342: Theming blocks bonanza won't make it (at least for now).
Comment #13
spokjeComment #14
spokjeComment #16
spokjeLet's give the ol' "20-times-break" and "20-times-fix" approach another go with the new proposed solution.
No interdiff, because it's a new approach.
Comment #18
spokjeComment #19
spokjeComment #20
spokjeComment #21
spokjeComment #22
lauriiiMarking as critical given the impact of this issue for the stability of the CI tests for 9.4.x and 9.5.x.
Comment #23
spokjeComment #24
spokjeComment #26
spokjeD9.4/9.5: MR!2217
D10.0: MR!2220
Comment #27
berdirThis feels like a somewhat strange fix that has a far bigger impact on everything, not just this specific test? Having the blocks in the theme means that all tests and also all real sites, new installations not using standard and existing ones will get these blocks by default.
Could we instead place the block that's added in the test in a different way? give that a negative weight, so it's above the conflicting block?
Any chance this is somehow also related to the default-region change in 9.4 that was done for olivero as it doesn't have sidebar_first? this caused a lot of problems in paragraphs.module.
Comment #28
lauriiiThis change will indeed help address the random failure in the
SettingsTrayIntegrationTest, but it will do more than that. I think it's expected with Olivero gets certain blocks placed because it is a much more opinionated theme than Bartik. I think we are currently assuming that any install profile using plain Olivero, would want a similar block placement to what Olivero ships with.I'd also like to note that this is simply taking this back to where it was before #3219959: Update standard profile so Olivero is the default theme. If we want to provide additional flexibility on this, we could do that in a follow-up issue.
Comment #29
lauriiiOpened an issue for discussing #27: #3278674: Decide how blocks should be shipped for Claro and Olivero. Berdir confirmed on Slack that he would be fine with discussing this in a follow-up.
I've tested both MR's manually and reviewed that all of the block configurations after the MR's are now in Olivero
config/optional.Comment #30
catchWe discussed this and related issue at length in slack.
The original weight fix in here would resolve the specific random test failure (more like random test pass) in this issue, but all the inconsistencies between Claro/Olivero/9.4.x/10.0.x blocks are likely to keep biting us. So getting everything aligned and then going from there is good.
Committed/pushed the 10.0.x MR to 10.0.x. Committed/pushed to 9.5.x MR and cherry-picked to 9.4.x, thanks!