Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Conversation

@chenxix
Copy link

@chenxix chenxix commented Aug 21, 2014

Review on Reviewable

@syncbot
Copy link
Collaborator

syncbot commented Aug 21, 2014

Automatic validation checks of commit 4058628 passed.

@chenxix chenxix force-pushed the chenxix/csswg-test/font-font branch from 4058628 to c8efab9 Compare November 7, 2014 09:00
@syncbot
Copy link
Collaborator

syncbot commented Nov 7, 2014

Automatic validation checks of commit c8efab9 passed.

@chenxix chenxix force-pushed the chenxix/csswg-test/font-font branch from c8efab9 to fddd33f Compare November 8, 2014 09:34
@syncbot
Copy link
Collaborator

syncbot commented Nov 8, 2014

Automatic validation checks of commit fddd33f passed.

@chenxix chenxix force-pushed the chenxix/csswg-test/font-font branch from fddd33f to 6a15582 Compare November 8, 2014 09:37
@syncbot
Copy link
Collaborator

syncbot commented Nov 8, 2014

Automatic validation checks of commit 6a15582 passed.

Copy link
Member

Choose a reason for hiding this comment

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

Lines 10~12 use the same technology to be tested. This is not a good reference file.

Copy link
Author

Choose a reason for hiding this comment

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

This case is to check if the 'font' shorthand property can accept and set font-variant, font-size and font-family at the same time. So I set these 3 attributes one by one. Is there some other way to set the same font style ?

@zqzhang
Copy link
Member

zqzhang commented Nov 11, 2014

For the Ahem font usage, please refer to http://testthewebforward.org/docs/test-style-guidelines.html#special-fonts

@chenxix chenxix force-pushed the chenxix/csswg-test/font-font branch from 6a15582 to 473dafa Compare November 11, 2014 09:09
@Ms2ger
Copy link

Ms2ger commented Sep 18, 2015

Reviewed 11 of 11 files at r1.
Review status: all files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.


css21/fonts/font-033-ref.html, line 7 [r1] (raw file):
I don't think anything guarantees that caption is equivalent to sans-serif (feel free to correct me if I'm wrong). If I'm right, though, I don't think we can write a useful reference for this range of tests.


css21/fonts/font-044-ref.html, line 8 [r1] (raw file):
I'm afraid this isn't really a useful reference, since a bug would likely cause the reference to fail in the same way as the test itself.


Comments from the review on Reviewable.io

@Ms2ger
Copy link

Ms2ger commented Sep 18, 2015

Thank you for your PR.

I landed the font-003 reference in #869. I don't believe the approach takes here for other tests can work, so I'm closing this PR. If you can come up with a workable approach, please do!

@Ms2ger Ms2ger closed this Sep 18, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants