-
Notifications
You must be signed in to change notification settings - Fork 111
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
toSvg
regression for explicit CSS values equal to inherit
#90
Comments
Whoa, nice catch, sorry for not seeing this when first PRed. I'll introduce a regression test and make it pass :) Seems your approach to the fast-path should work. |
I was of the impression iframe insertion would break with the newish Trusted Type API. (There is a The library continued to work fine with the following code and corresponding CSP:trustedTypes.createPolicy('default', {
// This code block needs security review, as it's capable of causing DOM XSS.
createHTML(dirty) { return DOMPurify.sanitize(dirty, {RETURN_TRUSTED_TYPE: true}) }
}) This means we are all green to rely on |
Strangely enough, I think the edge case in this issue is described in the comments:
Need to make a tracking issue to see if there's a really robust method for working with CSS inheritance (both A. stripping extraneous inheritable CSS props, and B. adding back values when CSS explicitly sets |
I'm addressing this in a new PR... would love to give credit on the package and readmd, but all I have is your github ID |
Fixes `toSvg` regression for explicit CSS values equal to `inherit` #90 by killing off the entire raster/vector decision path and always using the `copyUserComputedStyleFast` method. Cleaned up unit testing a bit more and moved to the npm packaged version of imagediff.
Addressed this more completely in #91 |
Fixes `toSvg` regression for explicit CSS values equal to `inherit` #90 by killing off the entire raster/vector decision path and always using the `copyUserComputedStyleFast` method. Cleaned up unit testing a bit more and moved to the npm packaged version of imagediff.
Fixes `toSvg` regression for explicit CSS values equal to `inherit` #90 by killing off the entire raster/vector decision path and always using the `copyUserComputedStyleFast` method. Cleaned up unit testing a bit more and moved to the npm packaged version of imagediff.
Fixes `toSvg` regression for explicit CSS values equal to `inherit` #90 by killing off the entire raster/vector decision path and always using the `copyUserComputedStyleFast` method. Cleaned up unit testing a bit more and moved to the npm packaged version of imagediff.
Fixes `toSvg` regression for explicit CSS values equal to `inherit` #90 by killing off the entire raster/vector decision path and always using the `copyUserComputedStyleFast` method. Cleaned up unit testing a bit more and moved to the npm packaged version of imagediff.
Use case: description, code
jsfiddle
Expected behavior
Related issues & PRs: #37 #71
It was taken as true that
unset
had no gotchas. I tried usingtoSvg
on.Box
in https://github.com/pulls.copyUserComputedStyle
is an over-eager optimisation and has value collisions.In this case, link color overrides that match
inherit
(as in.Box .Link--primary
) are stripped away prematurely.Here's an explanation of the
copyUserComputedStyle
regression:-webkit-link
/-moz-hyperlinktext
inherit
copyUserComputedStyle
assignsunset
inline which is computed asinherit
unset
and and the explicit color value match, that value is not added to the inline stylecolor
style. So instead of2
, we're literally back to square1
. 😃Actual behavior (stack traces, console logs etc)
When you open up the
#result
image src from the jsfiddle demo, it appears purple.Library version
Regression confirmed in https://github.com/1904labs/dom-to-image-more/releases/tag/v2.12.0 and present since https://github.com/1904labs/dom-to-image-more/releases/tag/v2.9.1.
Browsers
The text was updated successfully, but these errors were encountered: