Skip to content
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

Handle SVG mask URL with encoded anchor #98

Merged
merged 1 commit into from
Jun 22, 2022

Conversation

thibaudgg
Copy link
Contributor

This patch ensures that CSS assets with inlined SVG containing a mask
URL with an HTML encoded anchor are properly handled by the
CssAssetUrls compiler.

This avoids printing warning messages like:

Unable to resolve '%23c' for missing asset '%23c' in XYZ.css

@brenogazzola
Copy link
Collaborator

Hello @thibaudgg, thanks for checking Propshaft. Two questions:

1 - The test for this compiler should use only CSS rules and attributes, since those are the files that the compiler is applicable to, but you seem to have used an HTML tag. Can you show a longer example of how this would be used in an CSS file?

2 - %23c seems like an odd thing to hard code in the regex. Is this like data that is part of an specification and will always be that value?

@thibaudgg
Copy link
Contributor Author

thibaudgg commented Jun 22, 2022

Hey @brenogazzola, thanks for your quick reply.

1 - The test for this compiler should use only CSS rules and attributes, since those are the files that the compiler is applicable to, but you seem to have used an HTML tag. Can you show a longer example of how this would be used in an CSS file?

Yeah, my bad, the example using an svg tag is confusing, it looks like something like that in the CSS file:

background-image:url("data:image/svg+xml;charset=utf-8,%3Csvg width='88' height='23' viewBox='0 0 88 23' xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink' fill-rule='evenodd'%3E%3Cdefs%3E%3Cpath id='a' d='M11.5 2.51-2.42 2.51z'/%3E%3C/defs%3E%3Cmask id='c'%3E%3Crect width='100%25' height='100%25' fill='%23fff'/%3E%3Cuse xlink:href='%23a'/%3E%3Cuse xlink:href='%23b'/%3E%3C/mask%3E%3Cg opacity='.3' stroke='%23000' stroke-width='3'%3E%3Ccircle mask='url(%23c)' cx='11.5' cy='11.5' r='9.25'/%3E%3Cuse xlink:href='%23b' mask='url(%23c)'/%3E%3C/g%3E%3Cg opacity='.9' fill='%23fff'%3E%3Cuse xlink:href='%23a'/%3E%3Cuse xlink:href='%23b'/%3E%3C/g%3E%3C/svg%3E")

Should we just go with mask='url(%23c)' for the test then?

2 - %23c seems like an odd thing to hard code in the regex. Is this like data that is part of an specification and will always be that value?

Mmm, the regex only checks %23 which is # encoded, only the spec uses %23c as an example.

@brenogazzola
Copy link
Collaborator

Thanks, that makes it easier to understand.

Let's shorten your example (<svg mask='url(#myMask)'></svg>) and then try to keep this test as similar to the others as possible. Not sure how that was encoded so I'm doing this by hand, but it would be something like this?

compiled = compile_asset_with_content(%({ background: url("data:image/svg+xml;charset=utf-8,%3Csvg mask='url(%23cMyMask)'%3E%3C/svg%3E"); }))

This patch ensures that CSS assets with inlined SVG containing a mask
URL with an HTML encoded anchor are properly handled by the
`CssAssetUrls` compiler.

This avoids printing warning message like:
```
Unable to resolve '%23c' for missing asset '%23c' in XYZ.css
```
@thibaudgg
Copy link
Contributor Author

Yeah, your example is better. I updated the PR and went with just %23MyMask (without the c as it was just the name of the mask previously).

@brenogazzola brenogazzola merged commit 467f2a1 into rails:main Jun 22, 2022
@brenogazzola
Copy link
Collaborator

Thank you!

@thibaudgg
Copy link
Contributor Author

@brenogazzola thank you for reviewing and merging my PR so quickly! 👍🏻

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.

2 participants