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

Escaped quotes parsing #646

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

raxbg
Copy link
Contributor

@raxbg raxbg commented Jul 10, 2024

Often times font definitions include escaped quotes. Currently these are not parsed correctly. This PR resolves this.

Copy link
Contributor

@JakeQZ JakeQZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this.

However, I don't think the solution is correct. From MDN:

The name of a font family. This must be either a single <string> value or a space-separated sequence of <custom-ident> values. String values must be quoted but may contain any Unicode character. Custom identifiers are not quoted, but certain characters must be escaped.

So \"Liberation Serif\" actually represents two space-separated <custom-idents>, the first of which begins with a literal quote, the second of which ends with one. Thus it should be rendered back out with those literal quotes preserved.

In the case of the url function, the parameter is a <string> where quotes are optional. Similarly, special characters which are part of the URL can be escaped (whether or not the URL is enclosed in quotes), so, again, an escaped quote should be parsed as being part of the actual URL.

I think the initial (more common) problem to sort out is when there are escaped quotes inside a quoted string, e.g.

font-family: 'Baker\'s Dozen', serif;
background-image: url("/glyph-image?glyph=\"");

Then a separate issue would be to fix any problems with escaping in unquoted strings or <custom-idents>.

Does this make sense?

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