zooniverse / Panoptes-Front-End

Front end for zooniverse/Panoptes
https://www.zooniverse.org
Apache License 2.0
64 stars 75 forks source link

"Failed to execute 'removeChild' on 'Node'" error crashes page, likely due browser translations #7078

Open shaunanoordin opened 2 months ago

shaunanoordin commented 2 months ago

Browser-Specific(?) Bug

Users have been reporting of an odd Failed to execute 'removeChild' on 'Node' error that crashes on certain PFE pages (Talk pages, project home pages)

https://github.com/zooniverse/Panoptes-Front-End/assets/13952701/8e2b912d-063b-4b87-acb7-0161c7c6d6bc

Replication

Replicated with macOS + Chrome 123. I haven't fully tested with other browsers, but Jim has indicated no issues with Firefox.

Notes:

History

Status

Investigation has confirmed that issue exists. ⚠️ Root cause has not yet been determined. ⚠️ Full extent of bug has not yet been determined.

I'm estimating moderate-to-high impact to volunteers - the problem may "only" be affecting a small subset of users (since we're not seeing multiple reports yet), but it will make the platform nigh unusable to them.

Workaround:

Awaiting prioritisation of dev effort.

goplayoutside3 commented 2 months ago

I came across a React discussion about this bug related to Google Translate (likely used in Chrome's Translate feature). There could be an element on the PFE project pages to sleuth as the root cause, but because we have a couple suggested solutions to communicate to volunteers, and PFE project pages are being deprecated as soon as possible, dev work to solve this bug is pretty low priority. We'll chat in FE standup this week about it too.

eatyourgreens commented 1 month ago

There's a workaround, via that React discussion, which sends the errors to the console (rather than crashing the page.)

Screenshot of a Zooniverse project translated into Spanish by Google Translate. Crashing DOM errors are sent to the console, without crashing the React app.
eatyourgreens commented 1 month ago

I opened #7096 with Dan Abramov's patch for node.removeChild and node.insertBefore. It seems to fix the issue when I try it out locally.

eatyourgreens commented 1 month ago

Insightful comment from the Chrome team: https://issues.chromium.org/issues/41407169#comment10

Even if a website owner wanted to give full and exclusive control of the DOM to a VDOM library, it's not practical if you also have end-users with DOM-modifying chrome extensions (ie. Grammarly, password managers, etc) or if you have users with browsers that modify the DOM (ie. Chrome's built-in translation functionality). There's a large ecosystem dependent on DOM manipulation - that ecosystem is an unintended victim of the adoption of VDOM frameworks. I have nothing against React or VDOM (I personally use and like them), but entertaining the idea of changing chromium in response to compatibility issues that arise with React is setting a curious precident.

eatyourgreens commented 1 month ago

One other thought: these errors should be logged to Sentry, which will give you an idea of how frequently these crashes occur.