Re: Review Report on sections 9.1.x to 9.2.3 ~= 78 testcases

bz, I made substantive changes to a couple of your tests to fix validation:
   run-in-abspos-between-003
   run-in-breaking-001
   run-in-text-ref

and metadata changes to:
   run-in-basic-001
   run-in-basic-002
   run-in-basic-003
   run-in-block-between-003

On 08/31/2010 07:21 PM, "Gérard Talbot" wrote:
>
> Section 9.1.x and 9.2 (partial) review report
> =============================================
>
> **************************
> Section 9.1.1 The viewport
> **************************
>
> Author: Ian Hickson
>
> http://test.csswg.org/suites/css2.1/20100815/html4/root-canvas-001.htm
>
> I do not think this is a good test because if there was presence of red,
> it would not verify what the testcase was about, was aiming at testing
> to begin with: "what size is the viewport on the object element." And
> such question is answered in section 10.3.10 ->  10.3.2.
> http://www.w3.org/TR/CSS21/visudet.html#inlineblock-replaced-width
>
> The red (leading to, causing test failure) would have to come from
> padding on the wrapping div or because the object has a margin and/or
> padding or if the embedded document
> http://test.csswg.org/suites/css2.1/20100815/html4/support/root-canvas-001a.html
> does not apply the margin or padding rules for root and body elements.

The test is actually testing half of what it says it's testing. It's
making sure that the viewport and initial containing block of the
internal document is no smaller than the size of the <object> element.
This is a valid test. Ideally it would be paired with another test
that makes sure the viewport and initial containing block of the
internal document is no larger than the size of the <object> element.

> ----------------------
>
> Author: Boris Zbarsky
>
> http://test.csswg.org/suites/css2.1/20100815/html4/anonymous-boxes-001.htm
>
> I do not see why there is
> <link rel="help"
> href="http://www.w3.org/TR/CSS21/visudet.html#block-root-margin">
>
> Why isn't there a
> <link rel="help"
> href="http://www.w3.org/TR/CSS21/visudet.html#the-height-property">
> for that testcase?
>
> The testcase should be declaring a transitional DTD to avoid validation
> markup errors.

Alternately, the instructions could be placed in a <p>?
Should be fixed.

> Author: Boris Zbarsky
>
> From
> http://test.csswg.org/suites/css2.1/20100815/html4/block-in-inline-append-001.htm
> to
> http://test.csswg.org/suites/css2.1/20100815/html4/block-in-inline-whitespace-001b.htm
> (60 testcases)
> and
> http://test.csswg.org/suites/css2.1/20100815/html4/table-in-inline-001.htm
>
> have not been reviewed for several reasons:
> - absence of expected results, clear pass/fail conditions

These are reftests. Their pass/fail condition is matching the appropriate
reference, which is linked from the "Refs" column of the table of contents.

> - validation markup errors
> -&ndash; in the<title>
> Not reviewed

Ok

>
> Author: Ian Hickson
>
> http://test.csswg.org/suites/css2.1/20100815/html4/inlines-001.htm
>
> The test should state that there should be no red... obviously.
>
> Proposed replacement:
> http://www.gtalbot.org/BrowserBugsSection/css21testsuite/inlines-001.htm
>
> Rejected

Fixed.

> ----------------------
>
> Author: Ian Hickson
>
> http://test.csswg.org/suites/css2.1/20100815/html4/inlines-002.htm
>
> I spent a lot of time on this apparently simple testcase, reading E.2
> painting order. The test is about in-flow, non-positioned, inline box
> children painting along the z-axis over the parent. The green border is
> not even required for the test. I think this test should be removed.
> The inline parent<span>  has no content and no padding; so the whole
> <strong>  element should display.
>
> Rejected

Why is this rejected? The <span> does have content: its contents is
the <strong> element. The test is showing that the <strong> element's
background paints over the <span>.

It would probably be a stronger test if the <span> had 0.5em padding.
I will add that. Then the border comes into play.

> Reviewed and approved
>
> ----------------------
>
> Author: Ian Hickson
>
> http://test.csswg.org/suites/css2.1/20100815/html4/inlines-009.htm
> http://test.csswg.org/suites/css2.1/20100815/html4/inlines-010.htm
>
> I agree with these testcases... although I'm sure not everyone does...
> and, it seems, there is no normative rule on how replaced element should
> be displayed.
>
> {
> How a replaced element's content is rendered is not defined by this
> specification. Rendered content may also be alternate text for an
> element (e.g., the value of the XHTML "alt" attribute)
> }
> http://www.w3.org/TR/CSS21/conform.html#defs
>
> Maybe the testcase should have the "may" flag
>
> Undefined

So... I think what the testcase is trying to do is make sure the line
height doesn't get truncated when table cells are involved.
E.g. in Quirks mode, the spacing between the baseline and the bottom
of the line gets ignored by the table cell. The testcase should be
rewritten to use the Ahem font's É glyph for the text. I've checked
in new versions of both of those.

> ----------------------
>
> Author: Ian Hickson
>
> http://test.csswg.org/suites/css2.1/20100815/html4/inlines-011.htm
>
> I am sure this test should not be about section 9.2.2
> <link rel="help"
> href="http://www.w3.org/TR/CSS21/visuren.html#inline-boxes">
> Images sit on the baseline and background-color will paint the descent
> area.
>
> Proposed replacement:
> http://www.gtalbot.org/BrowserBugsSection/css21testsuite/inlines-011.htm

Fixed.

>
> Author: Ian Hickson
>
> http://test.csswg.org/suites/css2.1/20100815/html4/inlines-012.htm
>
> Same thing here: this test should not be in section 9.2.2
>
> Proposed replacement:
> http://www.gtalbot.org/BrowserBugsSection/css21testsuite/inlines-012.htm

Fixed.

> ----------------------
>
> Author: Ian Hickson
>
> http://test.csswg.org/suites/css2.1/20100815/html4/inlines-013.htm
>
> <img src="support/1x1-white.png">  is missing the alt.
>
> <link rel="help" title="9.5.1 Positioning the float: the 'float'
> property" href="http://www.w3.org/TR/CSS21/visuren.html#float-position">
> should be added
>
> Rejected because of that

Fixed.

> ----------------------
>
> Author: Ian Hickson
>
> http://test.csswg.org/suites/css2.1/20100815/html4/inlines-014.htm
> http://test.csswg.org/suites/css2.1/20100815/html4/inlines-015.htm
>
> alt attribute missing

Fixed.

> ----------------------
>
> Author: Ian Hickson
>
> http://test.csswg.org/suites/css2.1/20100815/html4/inlines-016.htm
>
> <link rel="help" title="16.6.1 The 'white-space' processing model"
> href="http://www.w3.org/TR/CSS21/text.html#white-space-model">
> <meta name="flags" content="ahem">
> <meta name="assert" content="If a space (U+0020) at the beginning of a
> line has 'white-space' set to 'normal', then it is removed.">
>
> should be added to that testcase and
>
> <link rel="help"
> href="http://www.w3.org/TR/CSS21/visuren.html#inline-boxes">
> <p
> class="ahem">Ahem_font_required_for_this_test._See_directory_listing_for_details.</p>
>
> should be removed.

Fixed.

> Author: Boris Zbarsky
>
> <script>  should be<script type="text/javascript">

Fixed throughought.

> ----------------------
>
> Author: Boris Zbarsky
>
> http://test.csswg.org/suites/css2.1/20100815/html4/run-in-abspos-between-003.htm
>
> <script>  should be<script type="text/javascript">
>
> <span id="r" class="abspos">Some text.</span>
>
> would require to be adjusted to pass validation. More work would be
> required here.
>
> Rejected

Fixed.

> ----------------------
>
> Author: Boris Zbarsky
>
> http://test.csswg.org/suites/css2.1/20100815/html4/run-in-basic-001.htm
>
> The assert text says:
> "There must be no nodes in the DOM between the run-in and the following
> block.">
>
> but section 9.2.3 says
>
> "
> If *a* sibling block box (that does not float and is not absolutely
> positioned) follows the run-in box (...)
> "
>
> so, it seems like it does not have to be the very first following block:
> it could be another.
>
> The assert text should be corrected.
>
> Rejected

Changed to
   Run-ins run into a following block if there is nothing
   between the run-in and the following block.

> ----------------------
>
> Author: Boris Zbarsky
>
> http://test.csswg.org/suites/css2.1/20100815/html4/run-in-basic-002.htm
>
> The assert text says:
> "There must be                  whitespace in the DOM between the run-in
> and the following                  block."
>
> but I think it should be saying
>
> "There can be whitespace in the DOM between the run-in and a following
> block."
>
> Reviewed and approved

Changed to
   Run-ins run into a following block if there is collapsed
   whitespace between the run-in and the following block.

> ----------------------
>
> Author: Boris Zbarsky
>
> http://test.csswg.org/suites/css2.1/20100815/html4/run-in-basic-003.htm
>
> The assert text says
> "Tests that run-ins actually run in if there are comments
>    between the run-in and the following block.  There must be
>       whitespace and comments in the DOM between the run-in and the
>             following block."
>
> but I think it should be saying
>
> "Tests that run-ins actually run in if there are comments between the
> run-in and the following block. There can be whitespace and comments in
> the DOM between the run-in and the following block."
>
> Reviewed and approved

Fixed.

> ----------------------
>
> Author: Boris Zbarsky
>
> http://test.csswg.org/suites/css2.1/20100815/html4/run-in-basic-005.htm
>
> The spec says:
> "A run-in cannot run in to a block that already starts with a run-in"
> and this is exactly what happens in the testcase. The spec does not
> continue with an "... otherwise...". So, why should the run-in header be
> inside the bordered #target? It seems to me that run-in is underdefined,
> underdevelopped in the spec., at least on this precise issue.
>
> Undefined

This one's been clarified in the errata. Hopefully the updated spec
will be published soon.

> ----------------------
>
> Author: Boris Zbarsky
>
> http://test.csswg.org/suites/css2.1/20100815/html4/run-in-basic-006.htm
>
> [there should be no space between the word "header" and the word "Start".]
>
> IMO, there should be a bullet list-marker between the word "header" and
> the word "Start".

The list marker is outside the box, so it can't be between the word "header"
and the word "Start". It is undefined for list-style-position: inside,
however...

>
> Author: Boris Zbarsky
>
> http://test.csswg.org/suites/css2.1/20100815/html4/run-in-basic-014.htm
> http://test.csswg.org/suites/css2.1/20100815/html4/run-in-basic-015.htm
> http://test.csswg.org/suites/css2.1/20100815/html4/run-in-basic-016.htm
> http://test.csswg.org/suites/css2.1/20100815/html4/run-in-basic-017.htm
>
> <script>
> should be
> <script type="text/javascript">
>
> The spec is underdefined. Why a single preserved (non-breaking) white
> space should prevent a run-in from running into a block?

Because a non-breaking space causes the creation of an anonymous inline,
which creates a line box around it. Preserved white space is not
transparent to layout effects.

> ----------------------
>
> Author: Boris Zbarsky
>
> http://test.csswg.org/suites/css2.1/20100815/html4/run-in-block-between-001.htm
>
>    <meta name="assert" content="Tests that run-ins don't run in if
> there's a block between                  them and the block.">
>
> How about saying instead/rather
>
>    <meta name="assert" content="Tests that run-ins run in with the first
> (closest) sibling block that follows the run-in box.">
>
> Isn't that what happens in that testcase?

The wording here is very confusing. The #target box shouldn't be there:
it has nothing to do with the test. The assertion should be "Run-ins
run into a block even if it's empty." Fixed.

>    <meta name="assert" content="Tests that run-ins run in with the first
> (closest) sibling block that follows the run-in box, even if/when
> dynamically inserted.">
>
> Isn't that what happens in that testcase?

Changed to
   Run-ins run into a following block even if the block is
   dynamically inserted and empty.

> ----------------------
>
> Author: Boris Zbarsky
>
> http://test.csswg.org/suites/css2.1/20100815/html4/run-in-breaking-001.htm
> http://test.csswg.org/suites/css2.1/20100815/html4/run-in-breaking-002.htm
>
> <span class="run-in">Run-in<br>header</span>
> should be
> <div class="run-in">Run-in<br>header</div>

Fixed.

> ----------------------
>
> Author: Boris Zbarsky
>
> http://test.csswg.org/suites/css2.1/20100815/html4/run-in-text-ref.htm
>
>      <div class="run-in">Run-in header</div>
>
>      Some text.
>
> should be
>
>      <div class="run-in">Run-in header</div>
>
>      <div>Some text.</div>

Fixed.

~fantasai

Received on Friday, 17 September 2010 07:17:46 UTC