Problem/Motivation

Drupal\Tests\views\FunctionalJavascript\Plugin\views\Handler\FieldTest::testModalDialogTitle failed unexpectedly in CI job #12920130.

This appears to be an intermittent failure and needs investigation.

Remaining tasks

  • Confirm the failure is intermittent.
  • Identify and fix the underlying cause.

Issue fork drupal-3631339

Command icon 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

joelpittet created an issue. See original summary.

joelpittet’s picture

joelpittet’s picture

pancho’s picture

Assigned: Unassigned » pancho

OK, after being hit by a version of this (in one CI run, the next one being fine) when working on tests for #2744923: noAJAX version of Views dialogs broken, I think I found what might be the underlying cause here:

jQuery UI first positions the dialog with position: absolute.

It's only after the dialog has already opened that Drupal's autoResize code changes it to position: fixed, however without recalculating its coordinates. On a scrolled page, this can put the title outside the viewport.

Non-modal dialogs with percentage-based height

For non-modal dialogs this is not consequential. The delayed resizing will eventually correct the position by triggering _position(), but even in the meantime, the title is accessible for Mink's getText().

Non-modal dialogs with numeric height

In combination with a numeric maxHeight , the dialog is ok for automated testing, but misplaced for human use. This is because the height value reaches our js as string "400" and is passed as such to jQuery UI which explicitly tests typeof() and discards it rather than silently converting it into numeric 400.
And as resetPosition() removes collision: 'fit', the top of the dialog moves out of the viewport, but is still accessible for getText().

Modal dialogs with percentage-based height

For modal dialogs, bodyScrollLock.lock() however sets overflow: hidden on the root element. The misplaced title is then clipped outside the viewport, so ChromeDriver treats it as hidden and provides Mink's getText() with an empty string.
Now even for modal dialogs, the delayed resizing will eventually correct the position by triggering _position(), at least with a percentage-based maxHeight given.
In what looks like a race condition, but is not yet well understood, the test may in some cases still check the title before that happens.

Modal dialogs with numeric height

In combination with a numeric maxHeight, that correction won't happen at all, so the fact that string "400" doesn't translate to numeric 400, isn't consequential in this case. Tests will reliably fail producing the title, but that's a rather synthetic case as Views UI's standard is 95%.

So besides avoiding numeric maxHeight, in testing we may wait long enough until the dust has settled. Or we could carefully fix the underlying causes, which seems absolutely doable.

pancho’s picture

Assigned: pancho » Unassigned

An MR that should fix this issue here as well is posted at #3633792: Dialogs subtly jump into position after opening.

joelpittet’s picture

@pancho Was this "...which seems absolutely doable." on purpose?

pancho’s picture

@joelpittet: LOL, no.
But may I ask you to review my MR in #3633792?
The only thing I still couldn't figure out is why – without those changes – the CI keeps delaying the repositioning so it would enter into a race condition time and time again, while I didn't manage to reproduce the same result on my own machine...

joelpittet’s picture

@pancho, I would be happy to look, though my attention is spread thin these days (which my attention wasn't on the top of the list all the time).

I did think about this problem a bit (napkin style)...
if we had a way/event to hook/listen into, as a "end" point in rendering we can wait on that would be ideal to stabilize the tests, no?