-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
WebKit export of https://bugs.webkit.org/show_bug.cgi?id=257092 #41259
WebKit export of https://bugs.webkit.org/show_bug.cgi?id=257092 #41259
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.
The review process for this patch is being conducted in the WebKit project.
<html> | ||
<head> | ||
<meta charset="utf-8"> | ||
<title>Tests that an empty font family name is handled correctly</title> |
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.
This title does not match the original test, I assume you meant "non-ident" here as well?
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.
Yes indeed
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.
Fixed it in 2d87052
<html> | ||
<head> | ||
<meta charset="utf-8"> | ||
<title>Tests that a non-ident font family name is handled correctly</title> |
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.
Could you explain what you need in this test that's not covered for example by css/css-fonts/font-palette-13.html
- is it the space in the name?
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.
Yes, hypen is part of an ident token in CSS syntax but space is not
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.
But anything that's in quotes is a CSS string token, isn't it? So I got a bit confused by the test title and would perhaps suggest to clarify that.
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.
Blink/WebKit have a peculiar behaviour about font-family serialisation (which is also happening with font-family name matching unfortunately) cf w3c/csswg-drafts#5846
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.
Thanks for the quick reply.
<html> | ||
<head> | ||
<meta charset="utf-8"> | ||
<title>Tests that a non-ident font family name is handled correctly</title> |
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.
But anything that's in quotes is a CSS string token, isn't it? So I got a bit confused by the test title and would perhaps suggest to clarify that.
No description provided.