Runtime errors caused by Chrome extensions

Hi everyone! I want some feedback about a well-known problem.

Context

Some Chrome extension breaks the Virtual DOM system and it causes infinite runtime errors. I recently found this is a very serious problem because Sentry notifies runtime errors every day (from the production app, of course).

There has been some discussion about this problem but not solved for a long time. discourse1 discourse2 discourse3 github

Help wanted to gather information

In the discussions above, @evancz requests more information before fixing it. 1 2

Before suggesting fixes based on whatever, I think there is information to gather:

  1. Make a list of the browser extensions that are known to cause problems.
  2. Figure out how they are they modifying the <body> exactly. Adding at top? Adding at bottom? Adding indiscriminately somewhere in the DOM?
  3. Sort the list of extensions by how they work.
  4. Figure out how many people use these extensions, so we can weigh the importance in practice. Maybe certain categories of problem only show up in weird ones, but one category is super common and reasonable to work around in virtual-dom.

From there, it will be much easier to find a fix that makes sense for the actual reality of the situation.

Can folks in this thread work on gathering this information and come back with the lessons?

I think the best answer would be done by this format. (Edit: The latest version is here)

Plugin (Users) Where When Workaround
Grammarly (10,000,000+) middle in <body> focus on <textarea> data-gramm_editor="false"
ChromeVox (161,918) top in <body> load, focus on something patch to output
Viber (133,220) top, bottom in <body> load patch to output

(!) The workaround is not fully tested. Use at your own risk.

Do you know any other popular extensions? Let’s add them too! (It is not necessary to strictly follow this format. Describe as you like.)

Tests

I also created a project to test the problematic cases.

This test covers various patterns of breaking DOM.

  • How to break? (insert, remove, etc.)
  • How to initialize the app? (Browser.application or Browser.element)
  • Where in the Virtual DOM is dangerous to update? (child, next element, attribute, etc.)

After this problem is fixed, all the test cases should be green. Also, you can try some of the tests online. Turn on and off your extensions to see how results change.

Any feedback about this project will also be welcome.

29 Likes

Great initiative, thank you very much :heart:

Maybe you could add a <style> node test inside the test page, because Dark Reader extension (1 763 020 users) for example will add its own style node just after an existing one if it is found in the DOM tree, leading to runtime exceptions.
A known workaround is to wrap style nodes into their own div.

Google translate (Chrome builtin extension, > 10 000 000 users, most outside US) is also known to break some VDOM sites because it adds some <font> elements.
For example the update button of your test page:

<button>update</button>

is replaced by

<button><font style="vertical-align: inherit;"><font style="vertical-align: inherit;">mise à jour</font></font></button>

when translating in french.

I have not yet be able to reproduce the runtime error with your test page though, but it should not be hard with a few modifications. I know however that it can be disabled with <meta name="google" content="notranslate"> in the <head>.

2 Likes

Nice! I’ll add this information soon :wink:

Edit: Done. The latest version is here.

1 Like

Should the table not have a suggestion for Elm too? I.e. not taking over the body element fixes 99% of the extensions.

Well, I made that number up, but that surely goes a long way. So if we add that, we can get some idea of easy fixes.

From my experience in this community, Evan tends to want a lot of information rather than suggestions. I guess he already has all the possible solutions and their trade-offs in his head. So let’s concentrate on gathering facts here.

I’m referring in particular to what Evan wrote:

but one category is super common and reasonable to work around in virtual-dom.

So that’s why I’m suggesting to add something like a root cause, or fix. For example for your ChromeVox you describe a work-around, but it’s not clear why that work-around is needed or why it works.

So what about adding a column that describes to root cause, i.e. what does Elm do/not do that causes this problem? So for ChromeVox I would add: "taking over ". This way we explicitly mention the category, something Evan asked for.

I’m not sure I fully get what you mean, but if the “root cause” mean something like “this variable in this line is potentially undefined, so…”, it’s relatively easy to find (I believe). It should be clearer once someone starts debugging hard.

For example for your ChromeVox you describe a work-around, but it’s not clear why that work-around is needed or why it works.

It avoids conflicting Elm and Chrome extension by separating the areas they control. Elm will treat a dummy element as the <body> element.

I believe that the root cause is always the same, messing-up with elements controlled by the virtual DOM, usually adding children elements, leading to invalid modifications during an update after diffing. That’s why the most important factor is where is the DOM modification done:

  • in the body itself or in another node?
  • at the begining of the node, in the middle or at the end?

and this information is in the table.

Maybe we could add the kind of modification done, but I suspect that most of the time when it breaks elm, it is due to some added nodes (are there some agressive extensions that remove nodes?).

The potential most interesting fix(es) would have to be evaluated once we have enough data.

AFAICT this is exactly what the “patch to output” does, it moves elm to a div inside the body instead of letting it taking the whole body (for an application), so when it is in the table, it means that not replacing the body is a work-around.

1 Like

@jinjor thinking more about it:

  • Maybe the table could show explicitly if the modification is in the body node itself or in a child node (or both)?
  • Maybe ways to disable an extension should be separated from workarounds that allow the extension to work (different column)?

@dmy That may be good, but could be a bit complex for the first read? Anyway worth trying.

Thank you for bringing this up once again. This is the number one problem in Elm! I mean how can I convince my team to invest in Elm if many popular extensions will break the app? Sooner or later. My arguments about easy refactoring and zero runtime errors will make me look silly in this context. This is the show stopper for the whole language.

4 Likes

I agree! It’s a shame that only Elm has this problem in many JS frameworks like React and Vue.

When we upgraded our SPA sites to 0.19 and Browser.application we saw an increase of virtual dom related run time errors in our log tool (New Relic).
This happened in all browsers, not only Chrome.
This was due to people (not us) using Google Tag Manager to add 3rd party scripts (chats, marketing, re-targeting etc) to the <body> tag. This is a very common practice in e-commerce at least (which is what we do) and not something we have control over.

We rolled back to using Browser.element and ports+js for navigation and history and that made the errors go away.

3 Likes

Yes. The cases of Browser.application have been discussed a lot, but I didn’t know the use of analytics tools can be uncontrollable. It’s good to know for me.

Status Update

I finally found potential way to fix this problem. This patch should make elm/virtual-dom work safely with extensions (still WIP though). It works as follows:

  1. Mark the DOM node as created_by_elm
  2. If unknown nodes have been inserted, skip them.
  3. If existing nodes have been removed, re-create them by old vdom.
  4. If existing nodes have been replaced with unknown nodes, re-create them by old vdom.

See more details here. Also, you can try it online.

Since the elm/virtual-dom is the core of all Elm apps, enough tests should be done. Now I’m writing more tests (example output), but it’s still not enough.

If you find any downside of this fix, please reply in this thread.
(More information about extensions is still welcome too!)

5 Likes

Wouldn’t adding a mount node argument to Browser.application's init function on the javascript side solve this?
Like Browser.element has?
It could even be optional so that existing apps don’t break. If node: is not set the Elm.Main.init it just uses <body> like today.

For our sites, switching to Browser.element did solve it.

<body>
  <div id="elm"></div>
  <script>
  var app = Elm.Main.init({
    node: document.getElementById('elm') // optional, uses body if not set
  });
  </script>
</body>

Wouldn’t adding a mount node argument to Browser.application 's init function on the javascript side solve this?

The limitation that Browser.application always mount on <body> is by design. So we cannot expect it to be “fixed”.

Also, some extensions insert elements in the middle of the contents, not only at the start or end of <body>.

1 Like

It might be useful to also have some benchmarks attached to this patch to show what would be the performance implications of this patch.

Thanks for the suggestion :+1:

I think this fix causes a big loss because it behaves (almost) as usual if there is no interruption. If there is, it re-creates DOM once but not more than once unless the extension continuously interrupts.
Anyway, it is worth trying. There might be a mistake.

This topic was automatically closed 10 days after the last reply. New replies are no longer allowed.