Skip to content

Conversation

@Tyriar
Copy link
Member

@Tyriar Tyriar commented Aug 15, 2023

@Tyriar Tyriar added this to the 5.3.0 milestone Aug 15, 2023
@Tyriar Tyriar self-assigned this Aug 15, 2023
@Tyriar Tyriar enabled auto-merge August 15, 2023 13:53
@Tyriar Tyriar merged commit 2eca26b into xtermjs:master Aug 15, 2023
@jerch
Copy link
Member

jerch commented Aug 15, 2023

Eww, is this style element handling browser-alpha ground? Tbh I dont like where this is going - should we revert the style stuff to a more proven code pattern?

On a sidenote - not even sure how recent engines need to be to support this - did we just raise the version requirements to newest browsers only by adding by #4611?

Edit:
Indeed #4611 will only work with newest Safari released in April, see https://developer.mozilla.org/en-US/docs/Web/API/CSSStyleSheet?retiredLocale=en, also Firefox needs be quite new. I'd suggest to revert #4611, or we lose support on some browser engines older than 6 months. 😱

@Tyriar
Copy link
Member Author

Tyriar commented Aug 15, 2023

We can now actually reproduce this in Chromium, so I'm also thinking we should just revert the whole CSP change as it doesn't seem like it's ready for primetime. FYI @SimonSiefke

@Tyriar Tyriar linked an issue Aug 15, 2023 that may be closed by this pull request
@SimonSiefke
Copy link
Contributor

Sorry for the breakage caused by my changes. I also agree that reverting the changes is the best option!

On a sidenote - not even sure how recent engines need to be to support this - did we just raise the version requirements to newest browsers only by adding by #4611?

The change uses try/catch and falls back to <style> elements, It seems to me it should be backwards compatible - apart from crashing chromium ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Terminal crash

3 participants