Views ajax modals stop working in certain scenarios: due to JS settings caching, AJAX responses may set wrong ajaxPageState.libraries, causing problems for subsequent AJAX requests/responses

I have just encountered this same issue.

It would be great to be able to turn the modal off completely.

You can workaround that by always force your browser to open it up in a new tab.

Regarding the actual bug, can you turn off JS aggregation and then see what kind of error message you get in your browser? There should be one, I hope.

maybe also related to #2559241: Closing an #ajax dialog triggers Javascript errors when scrolling (although I could still open the dialog after though)

I also had this issue, but it seems a little bit random. (Not always related to contextuals filters, I think I encounter that even on some views without any contextual filter).

Clearing the caches worked as a temporary fix for me (until we find a way to replicate this for sure)

Edit: Never mind, couple of cache clears and everything was working.

Works for me, well, probably genetic reasons.

@Haza,

I know its crazy random. I was good for the last 10 hrs or so now its back again. Ahhhhhh!

The only fix that works for me is if I switch between different Admin Themes. So if I go from Seven to Bartik, everything works for a little bit. Then after 10 mins or so I have to switch back to Seven.

Man this is tough building sites right now :(

Edit: Updated reproduce steps, explanation for what's going on in the next comment

Been clicking around a lot and trying to reproduce this. Using clean 8.0.x-dev with bartik.

Only once managed to kill it completely where no dialog would ever show up even after page reloads and cache clears, there it helped to switch off JS Aggregation and clear cache and reload the edit page.

Semi-reproducible :
- Switch off js aggregation (more for easy debugging then anything else)
- clear cache
- go to any view edit
- Click on any Add button
- Close the dialog
- Click on any Add button again
- reload the page
- Click 'update preview'
- Click Add
- No Dialog

Sometimes I need to redo this a couple of times to break it and maybe some steps aren't needed. But I always end up in a state where, if I start with 'update preview', all dialogs won't open after that.

When looking at the ajax request/response, they look normal, with the response containing the

"command": "openDialog",
    "selector": "#drupal-modal",

But dialog.ajax.js never gets loaded in the broken state. Where in a working state this gets added in the ajax response with

"command": "insert",
    "method": "append",

In both cases dialog.ajax.js is in the ajaxPageState.libraries in the response.

I have no idea if any of that is relevant, but maybe somebody can make something of it.....

I can now consistently reproduce this with the steps in #10.

What's going on:
AssetResolver::getJsAssets() uses $libraries_to_load to set it's $cid, but in this case that is insufficient context.

Following the steps in #10:
- First time you click add, $libraries_to_load is an array of js assets to load and there is no cache for that, so works fine and cache get sets
- Second time you click add, $libraries_to_load is an empty array, there is no cache for that, so works fine and cache is set
- Reload
- Preview auto-load does ajax request for 'update preview', $libraries_to_load is set to one library to load, no cache for that, so works and cache gets set
- Click 'update preview', $libraries_to_load is an empty array, cache for that was set in step 2, so cache gets reloaded, but with settings as if you clicked the 'add' button like in step 2! So now javascript settings thinks it contains the libraries that were loaded on a add click.
- Click add, if you check the settings now you see that javascript says that dialog.ajax.js has already been loaded (it hasn't) so it's never loaded and the dialog won't open.

Ok, so first stab add a fix. No idea what the impact of this change is or if it's in the right direction, but it fixes my issues when manually testing.

@Lendude
Yeah it is not entirely obvious what is going on here and how adding this additional line changes anything. Some code in that gigantic if probably depends on it.
I can just guess it this call\Drupal\Core\Asset\AssetResolver::getLibrariesToLoad but, yeah, this is just a guess of course.

@dawehner a test to illustrate the point I'm trying to make. The added test tries to load two completely different libraries but without the change in #11 this only leads to one CID (because you end up with two empty arrays as libraries to load the current code assumes these two requests are the same), that sounds strange to me.

But since my knowledge of caching is limited to say the least, feel free to correct me :)

First: thanks for your thoroughness! Much appreciated :)

So, the CID used in AssetResolver::getJsAssets contains:

1. the theme name, because themes can override/extend asset libraries, so we must vary by theme
2. the current interface language, because JS contains strings that can be translated, so we must vary by language
3. whether any JS settings are attached and hence to be loaded
4. whether the JS assets should be optimized or not
5. the $libraries_to_load, which represents the exact set of libraries that should be loaded; it's for this list of libraries that we will load

These 5 things all affect the return value.

As far as it was previously programmed/analyzed, this was considered correct/sufficient. Because in the end, what matters is which libraries are going to be loaded, and that's exactly what $libraries_to_load is supposed to cover.

+++ b/core/tests/Drupal/Tests/Core/Asset/AssetResolverTest.php
@@ -148,6 +148,11 @@ public function providerAttachedAssets() {
+      'different libraries to set, both empty sets to load' => [
+        (new AttachedAssets())->setAlreadyLoadedLibraries(['core/drupal'])->setLibraries(['core/drupal'])->setSettings(['currentTime' => $time]),
+        (new AttachedAssets())->setAlreadyLoadedLibraries(['core/jquery'])->setLibraries(['core/jquery'])->setSettings(['currentTime' => $time]),
+        2
+      ],

This is wrong. These should have the same CID, because in both cases we end up loading *no* JS assets, and the same JS settings. Therefore they should result in the same CID.

That being said, reading #10 & #11 also makes me think there's a bug.

At first I thought you were right that the bug is in AssetResolver, but after having looked at the code and having gone through the reasoning again, the code in AssetResolver is correct AFAICT.

I'll need to reproduce this and step through it with a debugger to figure out what's going on. However, I wouldn't be surprised if the root cause of the bug was actually in the AJAX system, which is sadly rather brittle.

Keeping assigned to me to reflect I must do that reproducing/debugging work. But feel free to work on this in the mean time regardless.

@Wim Leers thanks for taking a look at this!

When stepping through this the underlying problem I feel was that AssetResolverputs $settings in the cache

if ($cached = $this->cache->get($cid)) {
  list($js_assets_header, $js_assets_footer, $settings, $settings_in_header) = $cached->data;
}

So that assumes that ANY call that leads to the same libraries being loaded (or in this case, no libraries being loaded) is done with the same settings. Since the AJAX system stores any previously loaded libraries in the same settings in ajaxPageState.libraries, this becomes an issue.

So if I'm still following this rightly (and I might very well not be ;-), the AJAX system should not store ajaxPageState.libraries in those settings, or the cache should not make the assumption that not loading any libraries is always done with the same settings.

Curious what your digging will turn up!

Nice research, all. Let's get the summary updated with it.

Just ran into this bug, took a bit to find this thread. Hopefully there's a solution. Having to drush cr regularly while in the views ui is both unintuitive and frustrating.

drush cr (rebuild caches) resolved the issue for me.
Opening the dialogs in new tab was not an working option to me.
It opened in the new tab fine, but changing anything would not stick.
So the save of the settings did not work in the new tab.
But drush cr resolved it, I guess until the next time... I had it twice by now, not knowing if anything I did before was causing it.

It would be great to have some integration level test coverage for the described bug in the views UI.

Agreed, tests would really help if possible.

The committers (@xjm, @alexpott, @catch, @effulgentsia, @cottser) discussed this issue again today, this time with the theme system maintainers (@lauriii, @joelpittet). We agreed this issue is a major bug based on the recent clarifications

Thanks everyone for your ongoing work and research here; intermittent frustrations like this can be difficult to solve.

I tried to write normal web tests for this, but failed because the only way I can reproduce this currently is by doing consecutive AJAX requests on the same page. So...Functional Javascript test! Not a lot of examples of that in core yet ;) So the test could probably be WAY better, but it's mostly to illustrate the problem.

I added my fix from #13, but again, mostly to illustrate the problem. I don't necessarily feel it's the right fix for the issue, but it's A fix.

This all worked locally for me (thanks @alexpott!), but lets see what happens here.

So...Functional Javascript test! Not a lot of examples of that in core yet

Well, this is why we introduced it, that people actually write tests for it. Thanks a ton! There is honestly so much value in just that.

+++ b/core/modules/views_ui/tests/src/FunctionalJavascript/LibraryCachingTest.php
@@ -0,0 +1,71 @@
+    $preview->click();
+    $this->getSession()->wait(5000, '(0 === jQuery.active)');
...
+    $preview->click();
+    $this->getSession()->wait(5000, '(0 === jQuery.active)');

Can't we figure out that the preview appears in a better way?

Updated the issue summary.

Can't we figure out that the preview appears in a better way?

The problem with that is that currently the test just reloads the preview without making any changes. So the rendered preview doesn't change, so there is not a lot of change to detect. But we could check the ajax-progress spinner disappearing. Also cleaned up some other stuff.

We have some tests now, yeah! I think this is good to go as it is.

Committed 452667c and pushed to 8.1.x. Thanks!

Javascript testing ftw!

I didn't see this issue in my issue queue, despite it being apparently assigned to me :( Sorry about that.

But I'm not at all satisfied with the committed patch. It should've been possible to add a failing test case to \Drupal\Tests\Core\Asset\AssetResolverTest. (Was introduced in #2603138: CSS/JS asset caching can easily be trashed, which displayed problems similar to this one.)

It should've been possible to add a failing test case to \Drupal\Tests\Core\Asset\AssetResolverTest

@wim leers Yeah, I completely agree that it should possible. I tried but couldn't get it to work/fail, somehow the data needed to reproduce the issue is a little less straight forward then just suppling some combination of settings (at least all the settings i tried didn't fail). So I figured it might be a problem in the AJAX system like you pointed out in #16 so I went for the javascript test. And that at least gave me a failing test, so figured that was a lot better then nothing.

Yes, this totally is better than nothing! But it scares me we committed a solution without understanding what the actual root cause is.

Just digging some more:

The $settings['ajaxPageState']['libraries'] logic lives in system_js_settings_build which gets called by

        foreach ($this->moduleHandler->getImplementations('js_settings_build') as $module) {
          $function = $module . '_' . 'js_settings_build';
          $function($settings, $assets);
        }

in AssetResolver::getJsAssets() when 'core/drupalSettings' is set as a library to load.

When adding this library to the unit tests in \Drupal\Tests\Core\Asset\AssetResolverTest and change the mock module handler do

    $this->moduleHandler->expects($this->any())
      ->method('getImplementations')
      ->willReturn(['system']);

this will just fatal because it's a unit test and system.module isn't included....

So not sure we can reproduce the issue highlighted in the javascript test in a unit test, I've attached my attempt at one.

On the other hand, the failure to do this via unit test, might point to the root of the problem: system_js_settings_build. This method is setting up data that should not be cached (loaded libraries in the current page, ) in a hook that sets up data to be cached. So should that logic not be moved to system_js_settings_alter so it doesn't get cached?

The patch reverts the change from #27 and the javascript test still passes locally. Let's see what this does. And how @Wim Leers feels about this!

This has started happening again :( Same problem, dialog.ajax.js in ajax libraries but not actually loaded.

Haven't found any dependable ways to reproduce this again.

Possible hint #2783161: Links in views UI not working after you select 'ID : content ID from URL in contextual filter'

Credit for @charles1988 for pointing me in the right direction for a failing scenario.

Add a contextual filter with a default value. This only goes wrong with automatic preview enabled.

The new fix in #35 handles this scenario well, the current HEAD doesn't.

Interdiff to #35 is the test only patch with a minor reroll to make it apply again.

Oh, these will both fail because of #2771547: In Browser and FunctionalJavascript tests SIMPLETEST_USER_AGENT cookie needs to be set every 5 seconds, but with the latest patch applied in that issue, they fail/pass as expected.

edit: ok, maybe they work on d.o :)

While trying to add a new page display to an existing view, i also got the described error. A error was shown, that i had to provide a path for the display, but nothing happened while clicking on path settings. After applying patch #41 i got the popup and could enter the patch w/o any problems.

#41 worked sweetly for me. Thanks!

1. +++ b/core/modules/system/system.module
   @@ -730,6 +721,18 @@ function system_js_settings_alter(&$settings, AttachedAssetsInterface $assets) {
   +  if (isset($settings['ajaxPageState']) || in_array('core/drupal.ajax', $library_dependency_resolver->getLibrariesWithDependencies($assets->getAlreadyLoadedLibraries()))) {
   

   This condition should have some form of explanation. Its not entirely obvious what is going on.

2. +++ b/core/modules/system/system.module
   @@ -730,6 +721,18 @@ function system_js_settings_alter(&$settings, AttachedAssetsInterface $assets) {
   +    // Provide the page with information about the individual asset libraries
   +    // used, information not otherwise available when aggregation is enabled.
   +    $minimal_libraries = $library_dependency_resolver->getMinimalRepresentativeSubset(array_merge(
   +      $assets->getLibraries(),
   +      $assets->getAlreadyLoadedLibraries()
   +    ));
   +    sort($minimal_libraries);
   +    $settings['ajaxPageState']['libraries'] = implode(',', $minimal_libraries);
   

   Its also a bit sad that we no long can provide these values in the non alter case

3. +++ b/core/modules/views/tests/src/FunctionalJavascript/Plugin/views/Handler/ContextualFilterTest.php
   @@ -0,0 +1,75 @@
   +
   +namespace Drupal\Tests\views\FunctionalJavascript\Plugin\views\Handler;
   +
   ...
   + */
   +class ContextualFilterTest extends JavascriptTestBase {
   +
   

   I'm wondering whether this somehow should make it clear that we deal with a UI related test.

@dawehner thanks as always!

1. Moved it into the if above it, which was checking the same thing. I think this makes it clearer what's going on, but if you feel it needs more explanation let me know.
3. I put some more UI references in it and cleaned up some of the comments. I would argue that the test needs to go into Views UI, but the JTB Field handler adding test is in Views so felt it was better to keep them together.

I had the described error with view, based on Drupal Commerce products and Views slideshow as view formatter.

Patch #47 fixed my issue, and all works fine now:

Drupal 8.1.8
Drupal Commerce 8.x-2.0-alpha4+172-dev
Views slideshow 8.x-4.0

@sun-fire Would you like to double-check the things in #46 were completed in #47, double check the review list and mark this RTBC so it can get in?

It would be great if @wim leers could have a look at it gain, I mean where are the point of its assignment anyway :)

Its also a bit sad that we no long can provide these values in the non alter case

+100.

I don't understand why this change is necessary.

+++ b/core/lib/Drupal/Core/Asset/AssetResolver.php
@@ -214,7 +214,7 @@ public function getJsAssets(AttachedAssetsInterface $assets, $optimize) {
-    $cid = 'js:' . $theme_info->getName() . ':' . $this->languageManager->getCurrentLanguage()->getId() . ':' . Crypt::hashBase64(serialize($libraries_to_load) . serialize($assets->getLibraries())) . (int) (count($assets->getSettings()) > 0) . (int) $optimize;
+    $cid = 'js:' . $theme_info->getName() . ':' . $this->languageManager->getCurrentLanguage()->getId() . ':' . Crypt::hashBase64(serialize($libraries_to_load)) . (int) (count($assets->getSettings()) > 0) . (int) $optimize;

Nor do I understand why this change is necessary.

I could figure out the explanations for both if I stepped through it with a debugger, but I assume Lendude has already done that, he found the solution after all. So, Lendude, can you explain both?

@Wim Leers I'll give it a shot....not promising anything coherent.

The change is that setting $settings['ajaxPageState']['libraries'] moves from system_js_settings_build to system_js_settings_alter. The settings from system_js_settings_build get cached, those from system_js_settings_alter don't get cached. The already-loaded libraries shouldn't be cached, because the ajax request that were done before the current ajax request are not a constant, and the current system assumes that it is.

I'm not saying this is the right way to fix this. If there is another way to do this, fine. I just saw that a number of ajaxPageState settings were done before caching and a number were done after caching. This just moves setting of the already loaded libraries from one to the other.

The change in core/lib/Drupal/Core/Asset/AssetResolver.php just reverts the 'fix' that got committed in #29. As you pointed out before, that was never really solving the real problem (as the new failing test also illustrates). Since the new fix also fixes the old test, that change really isn't needed, so we might as well revert it.

Hopefully this helps a bit....if not, let me know

The already-loaded libraries shouldn’t be cached, because the ajax request that were done before the current ajax request are not a constant, and the current system assumes that it is.

Ahhh! That makes sense! Except for one thing: where does $settings['ajaxPageState']['libraries'] get cached?

This already helped a lot! :)

It's been a while since I looked at this, so a bit rusty on the exact sequence of events.

But if I see it correctly:
settings are always cached in \Drupal\Core\Asset\AssetResolver (see $settings)

$this->cache->set($cid, [$js_assets_header, $js_assets_footer, $settings, $settings_in_header], CacheBackendInterface::CACHE_PERMANENT, ['library_info'])

Looking at this again, my statement that it doesn't get cached with this fix is wrong. It's just that even if the settings are retrieved from the cache this still happens:

 if ($settings !== FALSE) {
      // Attached settings override both library definitions and
      // hook_js_settings_build().
      $settings = NestedArray::mergeDeepArray([$settings, $assets->getSettings()], TRUE);
      // Allow modules and themes to alter the JavaScript settings.
      $this->moduleHandler->alter('js_settings', $settings, $assets);
      $this->themeManager->alter('js_settings', $settings, $assets);

So $settings['ajaxPageState']['libraries'] is still cached, but they will get overwritten with the right values in the alter call.

Hm… I re-read #2506369: Cache CSS/JS asset resolving to understand why we are even caching settings. I thought we weren't. Clearly, I misremembered.

It looks like we collectively overlooked one thing there: the fact that the "already loaded libraries" can actually be request-dependent… at least for AJAX requests/responses. Yet we claim this:

/**
 * Implements hook_js_settings_build().
 *
 * Sets values for the core/drupal.ajax library, which just depends on the
 * active theme but no other request-dependent values.
 */
function system_js_settings_build(&$settings, AttachedAssetsInterface $assets) {

This is simply not true.

Therefore it makes sense to move all this logic to system_js_settings_alter().

What I'm still missing then, is explicit test coverage for this. The test coverage in the current patch proves it doesn't work, but in a very indirect manner. If the Views UI implementation changes, it could very well be that this test would not fail anymore with this particular fix. An explicit regression test prevents that from regressing again.

So, I think we need a test where we are testing the value of drupalSettings.ajaxPageState.libraries in the AJAX response. And it should be different depending on the "already loaded libraries" that the AJAX requests passes in via ajaxPageState.libraries. That's it. (So let's say the AJAX response must load A, B and C. And in one AJAX request we say that only A was loaded. Then in another AJAX request we say that both A and B were loaded. In the first case, the AJAX response must load B and C, in the second case it must load only C.)

Thanks so much for digging into this! Very glad I'm finally fully understanding this problem.

Attempt to make the title actually describe the root cause.

@Wim Leers, how about just inserting a fake library and making sure it's gone and stays gone after a page reload? It might be possible to not do this in a javascript test, but since we are testing the Ajax framework, a javascript test seems appropriate.

Interdiff is the test only patch.

Oops, left some screenshots in, will clean up if we feel this is on the right track.

Hah, that is an interesting way of tackling it. It tests the same thing, just slightly the other way around. Works for me. I'd rename the existing AjaxThemeTest to AjaxTest and add a testDrupalSettingsCachingRegression() method.

Nice work! :)

@Wim Leers thanks so much for looking at this.

Adding the test to the existing test makes sense, also cleaned it up a bit.

Can you reroll that patch with git diff -M10%? That'll show the moved hunks. Which will make it much easier to see what exactly is being changed.

Ah nice, didn't know that one, tried -C but that just made things worse :)

+++ b/core/tests/Drupal/FunctionalJavascriptTests/Ajax/AjaxTest.php
@@ -42,4 +42,42 @@ public function testAjaxWithAdminRoute() {
+  /**
+   * Test that AJAX loaded libraries are not retained between requests.
+   */
+  public function testDrupalSettingsCachingRegression() {

Can you just add a

@see https://www.drupal.org/node/2647916

line to this docblock?

Then this is ready. Because that's not too much to ask of a committer, I'm already RTBC'ing this.

Actually, somewhere along the way, this lost an important change. Quoting #52:

The change in core/lib/Drupal/Core/Asset/AssetResolver.php just reverts the 'fix' that got committed in #29.

Let's bring that back. Then this is really RTBC :)

@wim Leers nice catch!

Added the @see and put the revert back in.

Awesome, thank you :)

Committed 6707617 and pushed to 8.3.x. Thanks!

Setting to 'patch to be ported' for cherry-pick to 8.2.x after 8.2.0 is released.

I wish a new issue had been opened for this. Continuing on in the same issue always makes things hard to work out.

diff --git a/core/modules/system/system.module b/core/modules/system/system.module
index 821e0e3..1f553da 100644
--- a/core/modules/system/system.module
+++ b/core/modules/system/system.module
@@ -720,7 +720,7 @@ function system_js_settings_alter(&$settings, AttachedAssetsInterface $assets) {
           ->get($active_theme_key);
       }
     }
-   // Provide the page with information about the individual asset libraries
+    // Provide the page with information about the individual asset libraries
     // used, information not otherwise available when aggregation is enabled.
     $minimal_libraries = $library_dependency_resolver->getMinimalRepresentativeSubset(array_merge(
       $assets->getLibraries(),

Indentation fixed on commit.

Committed ed87af4 and pushed to 8.2.x. Thanks!

Thank you for commiting this major UI problem fix

Indeed! Anything that stabilizes any asset library edge cases is great, because it's super frustrating/super difficult to report for end users observing this.

Fixed some issues - still have some issues with "Sort criteria" in views
it seems to be related with integer-fields as sort-criteria - modal popup does not show up, or cannot be closed by "Save" - however it works by opening in new tab.
I am on 8.3.x-dev (2016-Nov-05)

Patch for Drupal 8.1.1

The issue is present again with the latest release of drupal core 8.4.5

Uncaught TypeError: Cannot read property 'dialog' of undefined
at :9:30
at :34:3
at p (jquery.min.js?v=3.2.1:2)
at Function.globalEval (jquery.min.js?v=3.2.1:2)
at text script (jquery.min.js?v=3.2.1:4)
at Qb (jquery.min.js?v=3.2.1:4)
at A (jquery.min.js?v=3.2.1:4)
at XMLHttpRequest. (jquery.min.js?v=3.2.1:4)
at Object.send (jquery.min.js?v=3.2.1:4)
at Function.ajax (jquery.min.js?v=3.2.1:4)

VM1154:41 Uncaught TypeError: $element.dialog is not a function
at openDialog (:41:16)
at Object.showModal (:31:9)
at Drupal.AjaxCommands.openDialog (:92:14)
at Drupal.Ajax.success (ajax.js?v=8.4.5:407)
at Object.success (ajax.js?v=8.4.5:222)
at i (jquery.min.js?v=3.2.1:2)
at Object.fireWith [as resolveWith] (jquery.min.js?v=3.2.1:2)
at A (jquery.min.js?v=3.2.1:4)
at XMLHttpRequest. (jquery.min.js?v=3.2.1:4)

Clearing caches temporarily lets use the view confing links.

Hi,

Funnily, I stumbled across this bug on 8.6.2.

widget-min.js?v=1.12.1:4 Uncaught TypeError: c is not a constructor
    at Function.a.widget (widget-min.js?v=1.12.1:4)
    at :9:5
    at :33:3
    at p (jquery.min.js?v=3.2.1:2)
    at Function.globalEval (jquery.min.js?v=3.2.1:2)
    at text script (jquery.min.js?v=3.2.1:4)
    at Qb (jquery.min.js?v=3.2.1:4)
    at A (jquery.min.js?v=3.2.1:4)
    at XMLHttpRequest. (jquery.min.js?v=3.2.1:4)
    at Object.send (jquery.min.js?v=3.2.1:4)
a.widget @ widget-min.js?v=1.12.1:4
(anonymous) @ VM1145:9
(anonymous) @ VM1145:33
p @ jquery.min.js?v=3.2.1:2
globalEval @ jquery.min.js?v=3.2.1:2
text script @ jquery.min.js?v=3.2.1:4
Qb @ jquery.min.js?v=3.2.1:4
A @ jquery.min.js?v=3.2.1:4
(anonymous) @ jquery.min.js?v=3.2.1:4
send @ jquery.min.js?v=3.2.1:4
ajax @ jquery.min.js?v=3.2.1:4
r._evalUrl @ jquery.min.js?v=3.2.1:4
Ja @ jquery.min.js?v=3.2.1:3
append @ jquery.min.js?v=3.2.1:3
insert @ ajax.js?v=8.6.2:538
(anonymous) @ ajax.js?v=8.6.2:435
Drupal.Ajax.success @ ajax.js?v=8.6.2:433
success @ ajax.js?v=8.6.2:234
i @ jquery.min.js?v=3.2.1:2
fireWith @ jquery.min.js?v=3.2.1:2
A @ jquery.min.js?v=3.2.1:4
(anonymous) @ jquery.min.js?v=3.2.1:4
VM1143:33 Uncaught TypeError: $element.dialog is not a function
    at openDialog (:33:16)
    at Object.dialog.showModal (:50:7)
    at Drupal.AjaxCommands.openDialog (:96:14)
    at ajax.js?v=8.6.2:435
    at Array.forEach ()
    at Drupal.Ajax.success (ajax.js?v=8.6.2:433)
    at Object.success (ajax.js?v=8.6.2:234)
    at i (jquery.min.js?v=3.2.1:2)
    at Object.fireWith [as resolveWith] (jquery.min.js?v=3.2.1:2)
    at A (jquery.min.js?v=3.2.1:4)

Remove Tampermonkey from Chrome extensions or disable it.