Problem/Motivation

I'm seeing this error in our project CI with drupal dialogs:

Error: cannot call methods on dialog prior to initialization; attempted to call method 'option'

This happens, when opening and immediately closing a modal (very fast).

Steps to reproduce

It's quite easy to reproduce with any modal and some JS code:
- click button to open modal,
- wait for .ui-dialog element to exist/be visible,
- and immediately close modal with cancel/close button.

The MR contains a test to reproduce.
If needed here is a sample JS code:

function waitForElement(selector) {
    return new Promise(resolve => {
        if (document.querySelector(selector)) {
            return resolve(document.querySelector(selector));
        }

        const observer = new MutationObserver(mutations => {
            if (document.querySelector(selector)) {
                observer.disconnect();
                resolve(document.querySelector(selector));
            }
        });

        observer.observe(document.body, {
            childList: true,
            subtree: true
        });
    });
}


document.querySelector('{open modal button selector}').click();
waitForElement('.ui-dialog').then((dialog) => {
    dialog.querySelector('{close button selector}').click();
});

Proposed resolution

It seems that in such cases, Drupal.dialog.resetSize is called from debounce after the jquery dialog instance has been destroyed. This happens within 20ms (debounce/setTimeout interval).

Add a check ensuring we have a valid jquery dialog instance at the top of Drupal.dialog.resetSize.

Also, per #25

dialog:beforeclose unbinds the resize handlers, but doesn't stop a already-scheduled debounced call. When the element is removed the jQuery UI instance is destroyed and the call tries to debounce on the destroyed dialog.

The fix adds a cancel() method to debounce.js and uses it when the dialog is closed, and adds test coverage.

Remaining tasks

?

User interface changes

None

Introduced terminology

None

API changes

None

Data model changes

None

Release notes snippet

None

CommentFileSizeAuthor
#9 3472624-nr-bot.txt1.05 KBneeds-review-queue-bot

Issue fork drupal-3472624

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

herved created an issue. See original summary.

herved’s picture

Issue summary: View changes
herved’s picture

Issue summary: View changes
herved’s picture

Status: Active » Needs review

Moving to needs review, thanks.

PS: It seems nightwatch has intermittent failures these days, phpunit as well

smustgrave’s picture

May be able to help decide the solution.

needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new1.05 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

smustgrave’s picture

Status: Needs work » Needs review

False positive

saidatom’s picture

Status: Needs review » Reviewed & tested by the community

Great! It looks good, moving to RTBC.

dxvargas’s picture

I just want to confirm the RTBC.
The new version of the MR, after the last two commits managed to fix the problems in the pipeline I had before.

claudiu.cristea’s picture

Issue tags: +JavaScript
nod_’s picture

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

I can't seem to replicate the test failures. When I run the tests without the immediate and early return fix from the dialog.position.js file it's still green.

joseph.olstad’s picture

While this patch deals with dialog
I was hoping it would also fix resizable

I'm seeing this one:

Uncaught Error: cannot call methods on resizable prior to initialization; attempted to call method 'option'

Using a theme (not claro) for a node form.
This worked up until Drupal 10.4.x

In Drupal 11 there's a js error related to the media dialog.

grimreaper’s picture

Hi,

I have the same problem on a D10.4, with CKE5 in Layout Builder offcanvas, on a text format with the drupal media and/or the icon (from ui_icons) plugins.

When I click on the media insert button or the icon insert button, the error is immediately present in the browser's console.

herved’s picture

#17 Hi @grimreaper,

Yes I also noticed these errors get triggered with CKeditor: https://git.drupalcode.org/project/drupal/-/merge_requests/9440#note_398735
Is this MR here fixing your issue?

There is also #3356667: Error: Cannot read properties of undefined (reading 'settings') with dialog.position.js which relates closely to this, maybe the patch there is is the proper fix?

grimreaper’s picture

Hi @herved,

I tried the MR changes without success.

But those errors were not blocking me it was something else related to dialogs which I had not been able to identify.

Like events not triggerred or incorectly triggered (not the right element, stuff like that).

So I pass to something else for until now.

goz’s picture

Hi, i have the same issue using ui_suite_bootstrap + layout_paragraphs + mercury_editor

In node edition with paragraph layout, start a new paragraph, choose a component (+ button), add your section paragraph in the first dialog, second dialog opened and error is logged in console

The MR does not solve this.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

albeorte’s picture

The current MR is not solving the problem.

longwave’s picture

Priority: Normal » Critical
Issue tags: -Needs subsystem maintainer review
Related issues: +#3515403: Update to selenium/standalone-chrome:133

This has got much worse since #3515403: Update to selenium/standalone-chrome:133, affecting multiple JS tests, so raising to critical. Removing subsystem maintainer tag for now as I don't think we need it, we just need to find a solution.

longwave’s picture

longwave’s picture

Status: Needs work » Needs review

Diagnosis and fix assisted by Claude Code.

dialog:beforeclose unbinds the resize handlers, but doesn't stop a already-scheduled debounced call. When the element is removed the jQuery UI instance is destroyed and the call tries to debounce on the destroyed dialog.

The fix adds a cancel() method to debounce.js and uses it when the dialog is closed, and adds test coverage.

longwave changed the visibility of the branch 3472624-tests-only to hidden.

godotislate’s picture

Status: Needs review » Reviewed & tested by the community

Repeat test (500x) of CKEditor5AllowedTagsTest::testFullHtml, which was randomly failing quite often: https://git.drupalcode.org/project/drupal/-/jobs/11377696. Passed about 350 times without failure before the job timed out.

Test only https://git.drupalcode.org/project/drupal/-/jobs/11377699 fails as expected.

lgtm!

Will be great to get this in because core FJ tests are failing randomly quite a lot.

quietone’s picture

Title: Error: cannot call methods on dialog prior to initialization; attempted to call method 'option' » Ensure the UI dialog instance is valid in Drupal.dialog.resetSize

Changing title to be a description of what is being fixed or improved per https://www.drupal.org/docs/develop/issues/fields-and-other-parts-of-an-issue/list-of-issue-fields#s-title

godotislate’s picture

Issue summary: View changes
Issue tags: -JavaScript +JavaScript
alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: -JavaScript +JavaScript, +Needs change record

We need a change record to tell everyone about debounce.cancel and when to use it. We also should check if there are other implementations that could use it in core.

godotislate’s picture

Status: Needs work » Needs review
Issue tags: -Needs change record

Per @longwave's MR comment, I think it's OK to go with the current solution.

Added a CR for cancel: https://www.drupal.org/node/3619513. It could use a review. I think it'd be nice for the CR to have an example of use, but I couldn't come up with one that is relatively simple.

claudiu.cristea’s picture

Status: Needs review » Reviewed & tested by the community

Remark has a reply and a CR has been added. Back to RTBC

alexpott’s picture

Version: main » 11.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed d0904d4ace3 to main and f0cc91d7ec9 to 11.x. Thanks!

This feels worth backporting to 11.x

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • alexpott committed f0cc91d7 on 11.x
    fix: #3472624 Ensure the UI dialog instance is valid in Drupal.dialog....

  • alexpott committed d0904d4a on main
    fix: #3472624 Ensure the UI dialog instance is valid in Drupal.dialog....