Re: [css-writing-modes-3] Review comments on orthogonal-parent-shrink-to-fit-001a

Thank you for reviewing the tests.

On Thu, May 14, 2015 at 1:38 PM, Gérard Talbot
<css21testsuite@gtalbot.org> wrote:
> Koji,
>
> http://test.csswg.org/source/css-writing-modes-3/orthogonal-parent-shrink-to-fit-001a.html
>
> line 11: font:25px Ahem;
>
> Whenever you set a font-size for the Ahem font, you need to set the
> line-height as well. Otherwise, line-height is, by default, 'normal' and
> 'normal' for IE11 and Webkit-based browsers for the Ahem font is 1.2 while
> it is 1.0 for Firefox.
>
> I suggest
>
> line 11: font:25px/1 Ahem;
>
> Ahem Usage
> http://testthewebforward.org/docs/test-style-guidelines.html#special-fonts

Done, thank you.

> line 12: clear:both;
>
> 'clear' can only apply and can only have a rendering effect in presence of
> floats and if there is a float earlier in the source document. So, here, at
> line 12, 'clear: both' has no impact, no influence, no relevance. You can
> safely remove 'clear: both'.
>
> 'clear: both' makes sense though in orthogonal-parent-shrink-to-fit-001.html
> but not in orthogonal-parent-shrink-to-fit-001?.html
>
> ----------
>
> line 13: margin-top:.2em;
>
> You can safely remove this declaration since the preceding <p>'s
> margin-bottom is 1em and so there will be a margin-collapse of both blocks'
> margin. max(1em, 0) or max(1em, 0.2em) is going to be the same anyway.

I just keep all variants in sync, this helps me when there were fixes.
I changed to use <h3> to make test descriptions more visible, hope
this is ok for you.

> http://test.csswg.org/source/css-writing-modes-3/orthogonal-parent-shrink-to-fit-001.html
>
> I think you should not create a "combo" test, a meta-test for all these
> orthogonal scenarios and situations. Each of these sub-tests are somewhat
> complex (definitely not basic tests) and not easily obvious ... at least to
> me...
>
> I would filename-rename those 1a-1g tests into separate, distinct tests and
> not make them part of a "combo" test.

Re-read the file name guideline:
http://testthewebforward.org/docs/css-naming.html
saying:

| There may also be a letter affixed after the number,
| which can be used to indicate variants of a test.
|
| For example, float-wrap-001l.xht and float-wrap-001r.xht
| might be left and right variants of a float test.

This is a test for shrink-to-fit, with different types of children as
variants. Isn't this a good fit to use the suffix pattern?

/koji

Received on Sunday, 17 May 2015 13:16:25 UTC