Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mutation observer enters infinite loop if the same element is observed in callback after disconnect #3172

Closed
vparchas opened this issue Apr 9, 2021 · 2 comments · Fixed by #3173

Comments

@vparchas
Copy link

vparchas commented Apr 9, 2021

Basic info:

  • Node.js version: 12.18.3
  • jsdom version: 16.5.2

Minimal reproduction case

let callbackCount = 0;
const div1 = document.createElement('div');
document.body.appendChild(div1);
const observer = new MutationObserver(() => {
    callbackCount += 1;
    // prevents infinite loop
    if (callbackCount >= 10) {
        return;
    }
    observer.disconnect();
    observer.observe(div1, {attributes: true});
});
observer.observe(div1, { attributes: true });
div1.setAttribute('data-test', 'test');

// flush queue
new Promise(resolve => resolve()).then(() => {
    console.log('callback calls: ' + callbackCount);
});

Expected: callback calls: 1
Actual: callback calls: 10

How does similar code behave in browsers?

Outputs callback calls: 1

https://jsbin.com/hepidugamo/edit?js,console

This is similar to #3096 however not quite the same, as this refers observing the same element and not a different element.

@vparchas
Copy link
Author

vparchas commented Apr 9, 2021

A simple fix for this would be to check if the node exists in the nodeList of the MutationObserver before pushing it:

observe(target, options) {
    [...]

    const existingRegisteredObserver = target._registeredObserverList.find(registeredObserver => {
      return registeredObserver.observer === this;
    });

    if (existingRegisteredObserver) {
      for (const node of this._nodeList) {
        node._registeredObserverList = node._registeredObserverList.filter(registeredObserver => {
          return registeredObserver.source !== existingRegisteredObserver;
        });
      }

      existingRegisteredObserver.options = options;
    } else {
      target._registeredObserverList.push({
        observer: this,
        options
      });

      // Add this check here
      if (this._nodeList.indexOf(target) < 0)
        this._nodeList.push(target);
    }
  }

TimothyGu added a commit to TimothyGu/jsdom that referenced this issue Apr 9, 2021
The code was misindented relative to the spec.

Fixes jsdom#3096. Fixes jsdom#3172.
TimothyGu added a commit to TimothyGu/jsdom that referenced this issue Apr 9, 2021
The code was misindented relative to the spec.

Fixes jsdom#3096. Fixes jsdom#3172.
TimothyGu added a commit to TimothyGu/jsdom that referenced this issue Apr 9, 2021
The code was misindented relative to the spec.

Fixes jsdom#3096. Fixes jsdom#3172.
TimothyGu added a commit that referenced this issue Apr 9, 2021
The code was misindented relative to the spec.

Fixes #3096. Fixes #3172.
@TimothyGu
Copy link
Member

Thanks for your help in debugging this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants