closest() breaks use of bpmn-js inside shadow DOM

[mmartensson]

The closest() implementation loops until parentNode is undefined or document. For the general case this is fine, but when using shadow DOM we will stumble onto a document fragment on the way that is the shadow root. Performing the matches() code on that fragment causes TypeError: Illegal Invocation.

Many of the forks of the closest repository have fixes for this. The current version 1.0.1 of the component fork seems to have a slightly different function prototype so if I were to submit a patch to them then there would still be work done for all code using min-dom. Maybe the best fix is to actually include an implementation in the min-dom itself rather than importing/exporting that of component.

This change I made directly in node_modules, giving me a working bpmn-js modeller (MoveCanvas was broken):

function (element, selector, checkYoSelf) {
  var parent = checkYoSelf ? element : element.parentNode;
  while (parent && parent !== document && !(parent instanceof DocumentFragment)) {
    if (matchesSelector(parent, selector)) return parent;
    parent = parent.parentNode;
  }
}

This is another implementation I found in a fork later on:

function (element, selector, checkYoSelf) {
  var parent = checkYoSelf ? element : element.parentNode

  while (parent && parent.nodeType !== document.DOCUMENT_NODE && parent.nodeType !== document.DOCUMENT_FRAGMENT_NODE) {
    if (matches(parent, selector)) return parent;
    parent = parent.parentNode
  }
}

[nikku]

Could you add a PR that includes the implementation in min-dom itself? We'd need accompanying test cases, too, otherwise we cannot accept the contribution.

[mmartensson]

Have patched code and written a test, but think maybe there is a less invasive solution.

The min-dom depends on matches-selector ^1.2.0 whereas closest 0.0.1 depends on (and ends up using) matches-selector 0.0.1. Starting with version 0.0.2, matches-selector directly uses Element.matches() where available and that makes my original problem go away.

I noticed this since my test that threw the expected TypeError stopped doing that immediately as I copied the code over from closest 0.0.1. My DocumentFragment check was not required since the matches-selector ^1.2.0 of min-dom solved it for me.

Element.matches() is supported by all browsers supporting Shadow DOM, so just getting the closest of min-dom to use a reasonable matches-selector should do the trick.

Depending on something like https://www.npmjs.com/package/component-closest/v/0.1.4 (old enough to be compatible) instead of https://www.npmjs.com/package/closest/v/0.0.1 is an option I'll look into next.

[mmartensson]

Going with component-closest caused issues with delegate-events which also uses closest. Simplest fix was to force transitive version of matches-selector used by the current closest.

Pull request #6

[oguzeroglu]

The problem with this issue is that we don't have built in shadow related DOM functionalities such as attachShadow within PhantomJS context. For instance:

var header = document.createElement('header');
console.log(header.attachShadow);

would print undefined. The original PR has a single test case which covers document fragments rather than shadow dom. So even though we support shadow dom, we won't be able to test this.

I'll try to use different headless browsers.

[oguzeroglu]

Update:

Using "karma-chrome-launcher" instead of "karma-phantomjs-launcher" fixes the issue I mentioned above.

[nikku]

The reason we use phantomjs for testing is the fact that we it is the as close as possible to shitty IE we get. Maybe it makes sense to keep it around and simply ignore the shadow DOM related tests in phantomjs?

[oguzeroglu]

Makes sense. So I'll fix the issue, verify it without tests, will include document fragment tests just in case.

[nikku]

So I'll fix the issue, verify it without tests, will include document fragment tests just in case.

Conditionally include document fragment tests:

(IS_FRAGMENT_SUPPORTED ? it : it.skip)('execute document fragment related tests', function() {
  ...
});

[oguzeroglu]

Good news: The issue can be reproduced with the following test (using Chrome headless browser):

  it.only('should get closest parent inside shadow DOM', function() {
    var rootDiv = document.createElement('div');
    var childDiv = document.createElement('div');
    var shadow = childDiv.attachShadow({mode: 'open'});
    var para = document.createElement('p');
    shadow.appendChild(para);
    rootDiv.appendChild(childDiv);

    rootDiv.className = 'root';
    childDiv.className = 'child';
    para.className = 'para';

    console.log(closest(childDiv, '.root')); // prints rootDiv
    console.log(closest(para, '.root')); // TypeError: Illegal invocation
  });

[tobiaskahl]

I have same Issue with Function "match$1" while embedding bpmn-io in vaadin flow:

function match$1(el, selector) { if (vendor$1) return vendor$1.call(el, selector); var nodes = el.parentNode.querySelectorAll(selector); for (var i = 0; i < nodes.length; ++i) { if (nodes[i] == el) return true; } return false; }

You fixed function with nearly the same name:
function match(el, selector) { if (!el || el.nodeType !== 1) return false; if (vendor) return vendor.call(el, selector); var nodes = el.parentNode.querySelectorAll(selector); for (var i = 0; i < nodes.length; i++) { if (nodes[i] == el) return true; } return false; }

Why do both exists? And why has just the first one the new if-statement with nodeType?

Thank you

[nikku]

Please open a new issue that clearly makes your case.

Use the bug report template and provide all information necessary for us to help you out.