core: Right clicking the text of an alert crashes the browser

[astorije]

Describe the bug

Right clicking the text of an alert crashes the browser.

How to reproduce

Steps to reproduce the behavior:

1. Go to https://stackblitz.com/edit/react-ts-m87m8o
2. Click on "Open in New Window (Live)". Make sure to be on Chrome.
3. Right click on any text of the first alert.
4. The browser crashes

Expected / Actual behavior

Screen.Recording.2021-06-29.at.17.47.04.mov

Versions

Clarity project:

• Clarity Core
• Clarity Angular/UI

Clarity version:

• v3.x
• v4.x
• v5.x (Reproduced on v5.3.0 and v5.4.1)

Framework:

• Angular
• React
• Vue
• Other:

_I have not tested if that's a bug with the web components as well or only the React components.°

Framework version:

React 17.0.2

Device:

• Type: MacBook
• OS: Catalina 10.15.17
• Browser: Chrome
• Version: 91.0.4472.114 (latest)

[mathisscott]

This is a React issue, documented here.

There are a few workarounds on that comment thread. But tl;dr, it's been fixed in later versions of React so path one is to update, path two is to use one of the workarounds on that thread, or path three is wrapping cds-alert content in a div.

Closing because React already has a fix and it's outside the scope of Clarity.

[astorije]

@mathisscott I don't believe this is related: the Stackblitz shared above is on the latest React (17.0.2). Apologies for incorrectly reporting this on the issue description.

Would you mind reopening this issue? 🙏

[mathisscott]

I don't see latest react on that stackblitz...

[mathisscott]

Additionally, this is a React issue. You'd get the most traction by opening an issue with them. If they do or do not fix it, there are three paths to workaround listed above.

[astorije]

Alright, my Stackblitz got hosed with:

Error in /turbo_modules/@cds/core@5.4.1/internal/base/button.base.js (6:25)
Unexpected token 'export'

So I had to create a CodeSandbox instead (forking didn't fix it): https://codesandbox.io/s/affectionate-raman-qpdof

It's using React 17.0.2, and on https://qpdof.csb.app/ you can see that the bug still exists.
And yeah I knew about the div workaround since, well, that's what I added as a workaround in my sandbox 😅

How did you connect this to the React issue you linked to? The dev console does not have any warning so I'm not sure how to debug this, or even know for sure that it's a React issue.

[mathisscott]

How did you connect this to the React issue you linked to? The dev console does not have any warning so I'm not sure how to debug this, or even know for sure that it's a React issue.

After it crashes in stackblitz, you can see the SharedArrayBuffer error in the console.

As far as us fixing this, we already wrap that content in an element. Wrapping it in another element (a div) on our end won't help. Because it seems clear that React is having trouble with the shadow boundary. The best fix is wrapping the content in a JSX "div" as you've shown. Or engaging with the React team to provide better support for custom elements...

[astorije]

Or engaging with the React team to provide better support for custom elements...

That seems like a can of worms far bigger and out-of-topic for that specific bug, so I'll pass on the opportunity 😅

If anyone implements the alert as documented on the Clarity website and uses @cds/react (which we have), they will run into this bug. We now have many dozens of alerts to fix all over our codebases with that undocumented div workaround. This doesn't sound ideal.

Could @cds/react inject an extra div within the child of <CdsAlert> instead? It seems like @cds/react would be the perfect home for this rather than letting this broken until React supports custom elements if ever. @coryrylan, do you think this is a feasible solution?

[github-actions]

Hi there 👋, this is an automated message. To help Clarity keep track of discussions, we automatically lock closed issues after 14 days. Please look for another open issue or open a new issue with updated details and reference this one as necessary.