tbranyen/diffhtml

executeScripts does not work in Firefox

CetinSert opened this issue ยท 8 comments

With

<div id=x></div>
<script type=module>
  console.warn(1);
  import { innerHTML } from 'https://diffhtml.org/es';
  innerHTML(x, '<script>console.warn(2)<\/script>');
</script>

Firefox does not execute scripts!
image

while other browsers do
image


Firefox also does not execute non-inline scripts:

<script src=//test.domain/warn.js></script>

if (config.executeScripts) {
// Execute deferred scripts by cloning them and reattaching into the same
// position.
scriptsToExecute.forEach((_, vTree)=> {
const oldNode = NodeCache.get(vTree);
const newNode = /** @type {any} */ (oldNode).cloneNode(true);
if (!oldNode || (hasModule() && newNode.type === 'nomodule')) {
return;
}
// If the script is now the root element, make sure we cleanup and
// re-assign.
if (StateCache.has(oldNode)) {
release(oldNode);
StateCache.set(newNode, state);
}
// Replace the node association.
NodeCache.set(vTree, newNode);
// Replace the scripts to trigger default browser behavior.
oldNode.parentNode && oldNode.parentNode.replaceChild(newNode, oldNode);
});
}

z = document.createElement('script'); z.textContent = `console.warn(3)`; document.body.append(z);

works in all browsers.

Thanks for bringing up this bug, it's a big one. Very strange that Firefox behaves differently than Chrome here.

The reason we clone a new script vs creating with createElement is that when you load the page with a content security policy you won't be able to modify certain scripts after the first page load. Therefore we clone and retain them until they are needed.

It appears Firefox struggles to re-use an existing script once it has been set with a type of no-execute, unlike Chrome.

Ah, I think I found a workaround. If we clone the original script before we attach the type change, we should be able to reuse that one later vs cloning at the end of the render.

Opened a fix here: #250. I ended up using your advice to createElement and use textContent.

@tbranyen โ€“ hopefully this does not introduce regressions with all the subtle <script> variations we have to deal with these days. (type=module; inline vs via src=; defer, async; etc.)

Once you confirm the fix and close this issue, I will be looking forward to a new CDN (module) link including this fix.
Like the last time, thank you for the near instant response time!

@tbranyen โ€“ hopefully this does not introduce regressions with all the subtle <script> variations we have to deal with these days. (type=module; inline vs via src=; defer, async; etc.)

All original props and attributes are mirrored so I think we are covered on that front ๐Ÿ‘

https://diffhtml.org/core/version points to 1.0.0-beta.23 now which contains the fix. Please reopen if you find any issues with the new behavior.

Thanks for reporting!