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

WebKit export of https://bugs.webkit.org/show_bug.cgi?id=257092 #41259

Merged
merged 1 commit into from
Jul 31, 2023

Conversation

mdubet
Copy link
Contributor

@mdubet mdubet commented Jul 31, 2023

No description provided.

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a 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.

@nt1m nt1m merged commit 125540f into web-platform-tests:master Jul 31, 2023
17 checks passed
<html>
<head>
<meta charset="utf-8">
<title>Tests that an empty font family name is handled correctly</title>
Copy link
Contributor

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes indeed

Copy link
Member

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>
Copy link
Contributor

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?

Copy link
Contributor Author

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

Copy link
Contributor

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.

Copy link
Contributor Author

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

Copy link
Contributor

@drott drott 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 the quick reply.

<html>
<head>
<meta charset="utf-8">
<title>Tests that a non-ident font family name is handled correctly</title>
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants