Drupal core will call attached behaviors passing two arguments: context and settings (which contains all the settings injected server side). Furthermore behaviors might be called not only after the DOM is loaded, but also in other scenarios like an AJAX request.

The first time that Drupal.attachBehaviors() is called, the context variable contains the document object representing the DOM, but for the rest of the calls context will hold the affected piece of HTML.

In a case where the JQuery UI Dialog library is loaded in the website through including "dialog.ajax.js" and "dialog.js" in a custom module, every time after the first time where the behavior is called (e.g. when a HTML snippet is added by an AJAX callback), the variable settings doesn't hold the same properties as for the DOM, being the property dialog one of the missing ones.

This leads to a Javascript error, since the Drupal behavior in "dialog.ajax.js" is expecting from settings to hold the missing property dialog.

So when we send an ajax response with two commands, one InsertCommand and one InvokeCommand, an error will be thrown after the InsertCommand and the InvokeCommand will not be executed.

The bug is at lines 38-44 in "dialog.ajax.js" under "core/misc/dialog/". Here Javascript is trying to get the element close from dialog of settings:

var originalClose = settings.dialog.close;

Since dialog is undefined (doesn't exist), you get the error:

Uncaught DrupalBehaviorError: attach ; dialog: Cannot read property 'close' of undefined

A solution would be to check before if settings.dialog is set, like this:

if (settings.hasOwnProperty(dialog)) {
    // do something with settings.dialog
}
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

davidum’s picture

Component: ajax system » javascript
Issue tags: +Ajax, +javascript error
davidum’s picture

Issue summary: View changes
hchonov’s picture

Status: Active » Needs review
FileSize
902 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 95,495 pass(es). View

Lets test it.

hchonov’s picture

Issue summary: View changes
hchonov’s picture

Issue summary: View changes
larowlan’s picture

mgifford’s picture

Re-uploading patch for the bots.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

sean.walker@nreca.coop’s picture

Version: 8.1.x-dev » 8.2.x-dev
Status: Needs review » Reviewed & tested by the community
Issue tags: +neworleans2016, +Needs backport to 8.1.x
FileSize
382.87 KB
335.92 KB

I tested this out in Firebug console by attempting to run 3 Ajax commands at the same time. I received the same error messages described by mgifford before applying the patch for all Ajax calls after the first one. I then downloaded and applied the patch, tested again with the same 3 Ajax calls in the console and received no errors. This looks solid, so I am marking as reviewed by the community to keep moving this along. Please find screenshots of my experience attached.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

We now have JavascriptTestBase in core so javascript fixes should come with tests so we don't break it in the future.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

dpagini’s picture

Issue tags: +Baltimore2017

I decided to look into this issue as part of Drupalcon Baltimore 2017. I'm a first time sprinter at the Major Issue Triage table. I don't expect to finish this today, but my plan is to reproduce the issue locally and take a crack at a test. I will report back how far I can get.

dpagini’s picture

Ok, I had some time to look into this tonight. I actually cannot replicate this problem any longer. I am running 8.4.x-dev, and will outline my test steps below. If I'm not following the right steps, please correct me if I'm wrong...

I have stood up a test site (standard install profile) and installed a custom module with a custom controller/form. My form puts a link and a button on the page. The link calls a modal (any admin page route in a modal) by way of the link render array class->use-ajax, data-dialog-type->modal. The button calls a custom ajax response to call the InsertCommand and an InvokeCommand. My InsertCommand adds a second link to the footer similar to the modal link just specified. The InvokeCommand just adds some text after each link. I also used a javascript "console.log" command to tell me every time the code in this patch (pre-patch code) was executed. It is executed on page load, and then again with each of the ajax commands above.

I'm not sure how else to reproduce this error. I have a suspicion that this issue may have been fixed by some other code updates so that it is no longer a problem.

@davidum, @hchonov et al - not exactly sure what to do with these findings at this point, but I hope this is helpful. If there's anything else I can do to investigate this issue, please let me know! I won't touch the status, but I assume "Closed (cannot reproduce)" is the appropriate status here?

droplet’s picture

Status: Needs work » Postponed (maintainer needs more info)

To remove (clean up all) the drupalSetting, isn't it a custom code mistake? I can imagine a lot of code will be broken if you empty the drupalSetting.

cilefen’s picture

I am updating credit for the triage work done by dpagini. Going forward, you should know that we like to see documented triage steps (even if brief). It is the only way to know if the triage has actually been completed. Here are some made-up examples of documented triage steps:

  • I tested the steps to reproduce and they did (or did not) work (so I am tagging it "Needs issue summary update").
  • I searched for duplicate issues but could not find any.
  • I checked the issue summary and it is accurate and up-to-date.
  • Etc...

Thank you!

cilefen’s picture

Whoops - I forgot to actually check the box!

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.