- From: Arron Eicholz <Arron.Eicholz@microsoft.com>
- Date: Sat, 17 Jul 2010 23:13:24 +0000
- To: "css21testsuite@gtalbot.org" <css21testsuite@gtalbot.org>, "public-css-testsuite@w3.org" <public-css-testsuite@w3.org>
Gérard Talbot wrote
>
> 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.
>
Updated
> 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>
>
Updated
> 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">
>
Updated
> 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.}
>
Updated
> 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">
>
Updated
> 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
>
Updated
> 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>
>
Updated
> 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.
>
Updated
> 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.">
>
Updated
>
> 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">
>
Updated
>
> 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">
>
Updated
>
> 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.">
>
Updated
>
>
> 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;
> }
>
>
Updated
>
> 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.">
>
>
Updated
>
> 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.">
>
>
Updated
>
> 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.">
>
>
Updated
>
> 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.">
>
>
Updated
> 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.">
>
Updated
>
> 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.
>
Updated
>
> 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.
>
Per the template it states that these have to remain. So I left them but they really don't cause any issues so it really isn't a big deal in my opinion.
>
> 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.">
>
>
Updated
>
> 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.">
>
>
Updated
> 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.
>
Updated
>
> 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?FeedbackI
> D=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.
>
It actually is the later specified rule is in the test file itself. The CSS file that is applied using user stylesheet rules is actually an earlier specified style rule in the order.
> --------------------
>
> 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
>
Updated
>
> 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
>
Updated
> 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
>
Updated
> --------------------
>
> 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>
>
Updated
> --------------------
>
> 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/
>
>
All the Microsoft cases should be updated.
You asked for suggestions to this format... I actually like this format. It worked well. The only potential suggestion would be to group the cases by owner. It would help a little when going through everything.
--
Thanks,
Arron Eicholz
Received on Saturday, 17 July 2010 23:14:02 UTC