-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(snapshot): Fix CSS expansion of add
CSS property
#223
fix(snapshot): Fix CSS expansion of add
CSS property
#223
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* There is a bug in chrome (https://issues.chromium.org/issues/41416124) with the `all` property where we can't use `cssText` because it is wrong. | ||
* Instead, attempt to serialize the css string using `CSSStyleDeclaration`. | ||
*/ | ||
export function fixAllCssProperty(rule: CSSStyleRule) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need checks for !important
in this function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah good catch
Includes the following fixes: - fix: Catch calls to iframe content document ([#222](getsentry/rrweb#222)) - fix(snapshot): Fix CSS expansion of add CSS property ([#223](getsentry/rrweb#223))
Includes the following fixes: - fix: Catch calls to iframe content document ([#222](getsentry/rrweb#222)) - fix(snapshot): Fix CSS expansion of add CSS property ([#223](getsentry/rrweb#223))
Adds a workaround for this Chrome bug where the CSS property
all
gets expanded by Chrome and can potentially override user defined CSS.Ref https://github.com/getsentry/team-replay/issues/504
Example
CSS as defined by user
After Chrome serialization
After fix