- From: Gérard Talbot <css21testsuite@gtalbot.org>
- Date: Mon, 5 Jul 2010 14:11:49 -0700
- To: "public-css-testsuite@w3.org" <public-css-testsuite@w3.org>
Hello all,
Here's my review report of testcases in chapter 6 (118 testcases):
http://test.csswg.org/suites/css2.1/20100701/html4/chapter-6.html
Boris Zbarsky and Ian Hickson should do a text search for their name to
know which of their respective testcases require to check my review.
Arron, you need to read this report too; 90% of this report regards to
Microsoft submitted testcases. Most of the time, it's only an inaccurate
<link rel="help"> or some better wording, small inaccuracies, etc.
1-
http://test.csswg.org/suites/css2.1/20100701/html4/default-stylesheet-001.htm
<title>CSS Test: Overriding the default style sheet</title>
<link rel="help" href="http://www.w3.org/TR/CSS21/cascade.html">
<meta name="assert" content="Default style sheet settings can be
overridden.">
should be
<title>CSS Test: Overriding the user agent default style
sheet</title>
<link rel="help"
href="http://www.w3.org/TR/CSS21/cascade.html#cascade">
<meta name="assert" content="User agent default style sheet
settings can
be overridden.">
Why?
"Default style sheet" can be ambiguous, does not mean a lot without
expliciting stylesheet origin;
<link rel="help": accurate reference to spec.
2-
http://test.csswg.org/suites/css2.1/20100701/html4/html-attribute-001.htm
<link rel="help" href="http://www.w3.org/TR/CSS21/cascade.html">
(...)
<table>
<caption align="right">Filler Text</caption>
<tr>
<td></td>
</tr>
</table>
<table>
<caption align="right">Filler Text</caption>
<tr>
<td></td>
</tr>
</table>
I believe the testcase intended to be instead
<link rel="help"
href="http://www.w3.org/TR/CSS21/cascade.html#preshint">
(...)
<table>
<caption align="left">Filler Text</caption>
<tr>
<td></td>
</tr>
</table>
<table>
<caption align="right">Filler Text</caption>
<tr>
<td></td>
</tr>
</table>
3-
http://test.csswg.org/suites/css2.1/20100701/html4/html-attribute-001.htm
to
http://test.csswg.org/suites/css2.1/20100701/html4/html-attribute-029.htm
<link rel="help" href="http://www.w3.org/TR/CSS21/cascade.html">
should be instead
<link rel="help" href="http://www.w3.org/TR/CSS21/cascade.html#preshint">
4-
http://test.csswg.org/suites/css2.1/20100701/html4/html-attribute-005.htm
should declare a doctype declaration referencing a strict DTD as border
is not a deprecated attribute for table.
http://www.w3.org/TR/html4/struct/tables.html#adef-border-TABLE
{Note to self: Also, note that the attribute minimization (eg. <table
border>) has not been tested. Maybe this should be tested.}
5-
http://test.csswg.org/suites/css2.1/20100701/html4/html-attribute-003.htm
<link rel="help"
href="http://www.w3.org/TR/CSS21/cascade.html#value-def-inherit">
is wrong
should be
<link rel="help" href="http://www.w3.org/TR/CSS21/cascade.html#preshint">
6-
http://test.csswg.org/suites/css2.1/20100701/html4/html-attribute-013.htm
valign is not deprecated: so the testcase should be declaring a doctype
declaration referencing a strict DTD.
http://www.w3.org/TR/html4/struct/tables.html#adef-valign
7-
http://test.csswg.org/suites/css2.1/20100701/html4/html-attribute-021.htm
I think you can safely remove
line 9: <style type="text/css"></style>
8-
http://test.csswg.org/suites/css2.1/20100701/html4/html-attribute-023.htm
Why is this HTMLonly?
<meta name="flags" content="HTMLonly">
I propose to make the hr thicker:
From
<style type="text/css">
p
{
border-color: transparent;
}
*
{
border-color: green;
}
</style>
</head>
<body>
<p>Test passes if the line below is solid green.</p>
<hr noshade="noshade">
to
<style type="text/css">
html, body, p
{
background-color: white;
}
*
{
background-color: lime;
border-color: lime;
}
</style>
</head>
<body>
<p>Test passes if the line below is solid bright green.</p>
<hr noshade="noshade" size="6">
http://www.w3.org/TR/html4/present/graphics.html#adef-size-HR
because right now, this testcase invites the user to compare/distinguish 2
rather dark colors (green vs gray) for a 2px tall ruler.
Also, note that noshade is deprecated: so this one should declare a
transitional DTD.
9-
http://test.csswg.org/suites/css2.1/20100701/html4/html-precedence-001.htm
I suggest to change
<link rel="help" href="http://www.w3.org/TR/CSS21/cascade.html">
(...)
<meta name="assert" content="The element selector takes precedence over
the font
element 'color' attribute.">
to
<link rel="help" href="http://www.w3.org/TR/CSS21/cascade.html#preshint">
(...)
<meta name="assert" content="color="red" is an HTML attribute
specification with a specificity equal to 0 which be overridden by
subsequent
style sheet rules.">
10-
http://test.csswg.org/suites/css2.1/20100701/html4/inherited-value-001.htm
<link rel="help" href="http://www.w3.org/TR/CSS21/cascade.html">
to
<link rel="help"
href="http://www.w3.org/TR/CSS21/cascade.html#inheritance">
11-
http://test.csswg.org/suites/css2.1/20100701/html4/non-inherited-value-001.htm
<link rel="help" href="http://www.w3.org/TR/CSS21/cascade.html">
to
<link rel="help"
href="http://www.w3.org/TR/CSS21/cascade.html#specified-value">
12-
http://test.csswg.org/suites/css2.1/20100701/html4/specificity-001.htm
<link rel="help" href="http://www.w3.org/TR/CSS21/cascade.html">
(...)
<meta name="assert" content="The element selector has the highest
specificity - specificity 0010 takes precedence over 0001.">
should be
<link rel="help"
href="http://www.w3.org/TR/CSS21/cascade.html#specificity">
(...)
<meta name="assert" content="The attribute selector has an higher
specificity over the element selector - specificity 0010 takes
precedence over 0001.">
13-
http://test.csswg.org/suites/css2.1/20100701/html4/specificity-002.htm
<link rel="help" href="http://www.w3.org/TR/CSS21/cascade.html">
(...)
<style type="text/css">
#id1 /* a=0 b=0 c=1 d=0 -> specificity = 0,1,0,0 */
{
color: red;
}
div:first-child /* a=0 b=0 c=1 d=1 -> specificity = 0,0,1,1 */
{
color: red;
}
should be instead
<link rel="help"
href="http://www.w3.org/TR/CSS21/cascade.html#specificity">
(...)
<style type="text/css">
#id1 /* a=0 b=1 c=0 d=0 -> specificity = 0,1,0,0 */
{
color: red;
}
div:first-child /* a=0 b=0 c=1 d=1 -> specificity = 0,0,1,1 */
{
color: red;
}
14-
http://test.csswg.org/suites/css2.1/20100701/html4/specificity-003.htm
should be instead
<link rel="help"
href="http://www.w3.org/TR/CSS21/cascade.html#specificity">
<meta name="assert" content="The ID attribute in a selector has the
higher specificity than the combined pseudo-class and element selectors
- specificity 0100 takes precedence over 0011.">
15-
http://test.csswg.org/suites/css2.1/20100701/html4/specificity-004.htm
should be instead
<link rel="help"
href="http://www.w3.org/TR/CSS21/cascade.html#specificity">
<meta name="assert" content="The combined pseudo-class and element
selectors have a higher specificity than the attribute selector -
specificity 0011 takes precedence over 0010.">
16-
http://test.csswg.org/suites/css2.1/20100701/html4/specificity-005.htm
should be instead
<meta name="assert" content="An element selector with a pseudo-element
selector wins over a pseudo-element selector - specificity 0002 takes
precedence over 0001.">
17-
http://test.csswg.org/suites/css2.1/20100701/html4/specificity-006.htm
should be instead
<meta name="assert" content="The combination of a pseudo-element
selector with an element selector takes takes precedence over simple and
single element selector - specificity 0002 takes precedence over 0001.">
18-
http://test.csswg.org/suites/css2.1/20100701/html4/specificity-007.htm
should be instead
<meta name="assert" content="The element selector has a higher
specificity over the universal selector - specificity 0001 takes
precedence over 0000.">
19-
http://test.csswg.org/suites/css2.1/20100701/html4/specificity-008.htm
first-line is a pseudo-element (d=1); first-child is a pseudo-class
(c=1). So
div:first-child:first-line /* a=0 b=0 c=1 d=2 -> specificity = 0,0,1,2 */
should be instead
<title>CSS Test: Calculating specificity - specificity 0012 vs.
0002</title>
<link rel="help"
href="http://www.w3.org/TR/CSS21/cascade.html#specificity">
<meta name="assert" content="The combined specificity of an
element selector, with a pseudo-class selector and a
pseudo-element selector achieves an higher specificity than the
combined specificity of an element selector with a
pseudo-element selector.">
div:first-child:first-line /* a=0 b=0 c=1 d=2 -> specificity = 0,0,1,2 */
If the intent was specifically to test (d=3) 0003 versus (d=2) 0002
specificity, then this test should be redone.
20 a)
Line 9: <style type="text/css"></style>
is in all user-stylesheet-001 to 018.htm files. I think such empty style
block should be removed.
20-
http://test.csswg.org/suites/css2.1/20100701/html4/user-stylesheet-003.htm
<title>CSS Test: User defined stylesheet cannot override property
defined in style tag</title>
<meta name="assert" content="The user style sheet cannot override
property defined in style tag.">
This is true if rules involve normal (not !important) declarations. As
worded, it may sound like such statement is generally true. It's even
inaccurate.
I suggest the following replacements
<title>CSS Test: Rules with normal declarations in an user defined
stylesheet can be overriden by rules with normal declarations from
author in a style block</title>
<meta name="assert" content="A CSS rule (with normal - not !important -
declarations) in an author style sheet have more weight than a CSS rule
(with normal - not !important -declarations) in an user style sheet.">
21-
http://test.csswg.org/suites/css2.1/20100701/html4/user-stylesheet-004.htm
<title>CSS Test: User defined stylesheet cannot override inline
style</title>
<meta name="assert" content="The user style sheet cannot override
property defined in inline style attribute.">
This is true if rules involve normal (not !important) declarations. As
worded, it sounds like such statement is generally true. Again, such
statement is not exact.
I suggest the following replacements
<title>CSS Test: Rules with normal declarations in an user defined
stylesheet can be overriden by normal declarations from author in an
inline style attribute</title>
<meta name="assert" content="A normal - not !important - declaration in an
inline style from author have more weight than a CSS rule (with normal
- not !important -) declarations in an user style sheet.">
22-
http://test.csswg.org/suites/css2.1/20100701/html4/user-stylesheet-012.htm
(Just a thought: How can it be certain that all browsers treat a link with
href="#" as an unvisited webpage when such link is resolved as the same
URI. href="#" works in IE8 with user-stylesheet-012.htm)
Reviewed and approved.
23-
http://test.csswg.org/suites/css2.1/20100701/html4/c13-inh-underlin-000.htm
Ian Hickson
<!-- XXX Content-Style-Type -->
missing.
class="a" along with id="a" is not helpful or simplifying but I think this
may be part of the testcase.
<em style="text-decoration: underline;">
could be replaced
<em style="text-decoration: underline;">
24-
http://test.csswg.org/suites/css2.1/20100701/html4/c13-inheritance-000.htm
Ian Hickson
There is no class="c" defined in the test. This may be in fact part of the
testcase.
Reviewed and approved.
25-
http://test.csswg.org/suites/css2.1/20100701/html4/c548-ln-ht-003.htm
Reviewed and approved.
Ian Hickson
The only difference between
/20100701/html4/c548-ln-ht-003.htm
and
/20100701/html4/c548-ln-ht-004.htm
is that line-height is expressed in terms of unit versus percentage
.nine {line-height: 2;} at line 12 and 19 of
/20100701/html4/c548-ln-ht-004.htm
while
.eight {line-height: 200%;} at line 12 and 19 of
/20100701/html4/c548-ln-ht-003.htm
Both testcases are actually the same.
Ian Hickson
26-
http://test.csswg.org/suites/css2.1/20100701/html4/inherit-border-padding-000.htm
Reviewed and approved.
27-
http://test.csswg.org/suites/css2.1/20100701/html4/dynamic-top-change-001.htm
Boris Zbarsky
<script type="application/javascript">
Why type="application/javascript"? Why not just
type="text/javascript" instead?
MSIE 8 does not support type="application/ecmascript" and
type="application/javascript" which meet RFC 4329. {The valid types are
"application/javascript" and "application/ecmascript". All modern
JavaScript interpreters being implementations of ECMAScript, it seems
like it would be best to use "application/ecmascript". Except that our
good friend IE doesn't recognize them and ignores the script altogether
if you use them. So once again, thanks to one more non-standard behavior
of IE, we can't afford to follow the standard.} by Bertrand Le Roy.
RFC 4329 was published in April 2006 and RFC 4329 was discussed and
approved as soon as June 2005. This bug has been reported at connect's
IE beta feedback as bug 338278: this bug has been closed by the IE team
and has not been reopened.
https://connect.microsoft.com/IE/feedback/ViewFeedback.aspx?FeedbackID=338278
I am inclined to reject this testcase because this is a test suite on
CSS compliance, not on Scripting Media Types and its RFC 4329.
The thing is this: even when trying
http://www.gtalbot.org/BrowserBugsSection/css21testsuite/dynamic-top-change-001-gt.htm
IE8 still fails but #grandparent's offset top is being modified, updated
accordingly while in
http://test.csswg.org/suites/css2.1/20100701/html4/dynamic-top-change-001.htm
it is not, meaning that
<script type="application/javascript">
is not supported in IE8.
This will need to be settled. FWIW,
http://www.w3.org/Style/CSS/Test/guidelines.html#content-style-type
recommends
<meta http-equiv="Content-Script-Type" content="text/javascript"/>
for inline script (event handler) attributes but RFC4329 has obsoleted
"text/javascript" for years now.
28-
http://test.csswg.org/suites/css2.1/20100701/html4/dynamic-top-change-001.htm
Boris Zbarsky
001 to 004: all assume, presume that the <p> has a margin-top of 1em.
Maybe the tests should make this explicit. Or use <div> instead. Or the
list of common assumptions for the test suite should state this.
--------------------
http://test.csswg.org/suites/css2.1/20100701/html4/dynamic-top-change-005.htm
Boris Zbarsky
after spending quite a bit of time on this testcase ...
For starters,
<meta name="assert" content="The position of a positioned element which
inherits its 'top' value from its absolutely positioned grandparent
changes when the grandparent's 'top' value is changed.">
should be rather/instead
<meta name="assert" content="The position of a positioned element
which inherits its 'top' value from its relatively positioned parent
changes when the parent's 'top' value is changed.">
I did not understand why
body { font-size: 0; line-height: 0;}
I also think the testcase could be reduced by changing/replacing
#grandparent { position: absolute; top: 0; }
body { font-size: 0; line-height: 0; position: relative; }
into
#grandparent { position: absolute; top: 8px; left: 8px; }
Here's my proposal:
http://www.gtalbot.org/BrowserBugsSection/css21testsuite/dynamic-top-change-005-gt.htm
I had to set body {margin: 8px;} because some browsers (eg Konqueror
4.4.5) set a larger margin for body and then the #overlapping-green
square left's offset is not 8px but more.
--------------------
29-
http://test.csswg.org/suites/css2.1/20100701/html4/inherit-001.htm
<meta name="assert" content="Children elements inherit the
parents color given the 'inherit' keyword.">
div
{
color: red;
}
body div
{
color: inherit;
}
The div inherits the green color because of specificity (0,0,0,2) too.
Children elements inherit the parent's color by default, without the
recourse of 'inherit' keyword; the test assertion suggests otherwise.
I am not sure that this is a good testcase: it certainly can be improved.
30-
http://test.csswg.org/suites/css2.1/20100701/html4/inherit-002.htm
reviewed and approved
31-
http://test.csswg.org/suites/css2.1/20100701/html4/inherit-003.htm
reviewed and approved
32-
http://test.csswg.org/suites/css2.1/20100701/html4/c11-import-000.htm
reviewed and approved
33-
http://test.csswg.org/suites/css2.1/20100701/html4/c21-pseu-cls-000.htm
reviewed and approved
34-
http://test.csswg.org/suites/css2.1/20100701/html4/c21-pseu-id-000.htm
reviewed and approved
35-
http://test.csswg.org/suites/css2.1/20100701/html4/cascade-001.htm
to remove
<style type="text/css"></style>
reviewed and approved
http://test.csswg.org/suites/css2.1/20100701/html4/cascade-002.htm
reviewed and approved
http://test.csswg.org/suites/css2.1/20100701/html4/cascade-003.htm
reviewed and approved
http://test.csswg.org/suites/css2.1/20100701/html4/cascade-004.htm
http://test.csswg.org/suites/css2.1/20100701/html4/cascade-005.htm
reviewed and approved
--------------------
36-
http://test.csswg.org/suites/css2.1/20100701/html4/cascade-006.htm
<meta name="assert" content="The latter specified rule wins when having
the same specificity and weight in user styles sheets.">
needs to be tuned, tweaked. The last redefining rules comes from author
<style type="text/css">
.cascadegreen
{
color: green;
}
</style>
and is a normal (not !important) rule: so it wins over the user
stylesheet rules
<div class="cascadegreen cascadered">Filler Text</div>
<meta name="assert" content="The latter specified rule wins when having
the same specificity and weight in user styles sheets.">
This cascade-006.htm test does not test what it was intended/aiming at
testing actually.
--------------------
http://test.csswg.org/suites/css2.1/20100701/html4/cascade-007.htm
reviewed and approved
http://test.csswg.org/suites/css2.1/20100701/html4/cascade-008.htm
should <link rel="help"> to
http://www.w3.org/TR/CSS21/cascade.html#important-rules
<link rel="help"
href="http://www.w3.org/TR/CSS21/cascade.html#important-rules"
title="6.4.2 !important rules">
reviewed and approved
http://test.csswg.org/suites/css2.1/20100701/html4/cascade-009.htm
should <link rel="help"> to
http://www.w3.org/TR/CSS21/cascade.html#important-rules
reviewed and approved
http://test.csswg.org/suites/css2.1/20100701/html4/cascade-010.htm
should <link rel="help"> to
http://www.w3.org/TR/CSS21/cascade.html#important-rules
<link rel="help"
href="http://www.w3.org/TR/CSS21/cascade.html#important-rules"
title="6.4.2 !important rules">
reviewed and approved
--------------------
http://test.csswg.org/suites/css2.1/20100701/html4/cascade-011.htm
should <link rel="help"> to
http://www.w3.org/TR/CSS21/cascade.html#important-rules
<link rel="help"
href="http://www.w3.org/TR/CSS21/cascade.html#important-rules"
title="6.4.2 !important rules">
Proposed corrections:
<meta name="assert" content="User stylesheet rules with !important
override normal user rules.">
<style type="text/css"></style>
(...)
<div class="cascadered cascadegreenimportant">Filler Text</div>
to change to
<meta name="assert" content="User stylesheet rules with !important
declarations override user stylesheet rules with normal declarations.">
(...)
<div class="cascadegreenimportant cascadered">Filler Text</div>
--------------------
http://test.csswg.org/suites/css2.1/20100701/html4/c31-important-000.htm
Proposed adjustments:
The <title> should better indicate what is intended to be
tested. There is no test assertion.
Here, !important declarations wins over specificity. For the same origin,
!important declarations wins, override specificity. This is what the
<title> and test assertion should indicate, state.
--------------------
http://test.csswg.org/suites/css2.1/20100701/html4/c32-cascading-000.htm
reviewed and approved
http://test.csswg.org/suites/css2.1/20100701/html4/c64-uri-000.htm
reviewed and approved
============
I am open to suggestions on how to better format and create such review
reports.
regards, Gérard Talbot
--
Contributions to the CSS 2.1 test suite:
http://www.gtalbot.org/BrowserBugsSection/css21testsuite/
CSS 2.1 test suite (beta 1; July 1st 2010):
http://test.csswg.org/suites/css2.1/20100701/html4/toc.html
CSS 2.1 test suite contributors:
http://test.csswg.org/source/contributors/
Received on Monday, 5 July 2010 21:12:26 UTC