Re: multicol-margin-001 test: review comments

Le Mar 2 juillet 2013 20:01, Håkon Wium Lie a écrit :
> Hello Gérard,
>
>  > [nightly-unstable]
>  > http://test.csswg.org/suites/css3-multicol/nightly-unstable/html4/multicol-margin-001.htm
>  >
>  > [src]
>  > http://test.csswg.org/source/contributors/opera/submitted/multicol/multicol-margin-001.xht
>  >
>  > As far as I can understand, this test is supposed to be verifying
> that
>  > the top margin of the first block child element of a multicol element
>  > does not collapse with the top margin of the multicol element.
>
> Correct.
>
>  > line 11: body {
>  > 	margin: 0;
>  > }
>  > line 14: p {
>  > 	border-bottom: 2em solid white;
>  > 	margin-bottom: 1em;
>  > }
>  >
>  > These rules are not part of the test itself. The <p> has no content
>  > anyway in this non-self-describing test.
>  > We usually prefer to use/rely on browser defaults for body margins
> and p
>  > margins. This way, tests can be shorter and more straightforward and
>  > they also will work on browsers with different body margins and p
>  > margins.
>  > There is no need in such tests for resetting those (body and p)
> margins.
>
> Agree, they should be removed.
>
>  > line 19: 	font-family: ahem;
>  > line 20: 	font-size: 1em;
>  >
>  > Whenever the Ahem font is used, we need to set a font-size whose
>  > computed value will be dividable by 5px without a remainer. We do
> this
>  > to avoid rounding issues and to ensure accurate vertical positioning
> of
>  > content on the baseline, mostly for the Linux platform.
>  >
>  > "
>  > If the test uses the Ahem font, make sure its computed font-size is a
>  > multiple of 5px, otherwise baseline alignment may be rendered
>  > inconsistently (due to rounding errors introduced by certain
> platforms'
>  > font APIs). We suggest to use a minimum computed font-size of 20px.
>  > "
>  > http://wiki.csswg.org/test/format#acceptable-test-formats
>  >
>  > And so its current associated reftest would need to be updated to
>  > reflect this too.
>
> So, you suggest setting?:
>
>   div { font-size: 20px }
>

Hello Håkon,

Yes, div { font-size: 20px } would be okay. Also correct would be

div { font-size: 1.25em; }


> I still seee vertical lime stripes on my Linux screen. But that may be
> something else?

Yes, because current Firefox (v. 22.0) and Chrome (v. 28.0.1500.70)
stable releases do not support multi-column: they both require their
respective vendor prefix. Firefox 22 supports -moz-multi-count,
-moz-multi-width, -moz-multi-gap, -moz-multi-rule . Chrome 28.0.1500.70
supports -webkit-multi-count, -webkit-multi-width, -webkit-multi-gap,
-webkit-multi-rule .


>  > All the multi-column tests submitted by Opera will need to be
> adjusted
>  > on this font-size of Ahem font issue.
>
> Noted. Thanks for pointing this out. I'll try go through them.
>
>  > line 26: position: relative;
>  > line 37: div::after {
>  > 	 content: "";
>  > 	 background: white;
>  > 	 height: 1em;
>  > 	 width: 2em;
>  > 	 position: absolute;
>  > 	 right: 0;
>  > 	 bottom: 0;
>  > 	 display: block;
>  > line 46: }
>  >
>  > I do not see the reason for the generated 2x1 white rectangle in the
>  > test. If margin-top of first child collapses with the margin-top of
> the
>  > multi-column element, then we will see a bright green
>  > 32px-wide-by-16px-tall rectangle at the bottom right corner. If
>  > margin-top of first child does *not* collapse with the margin-top of
> the
>  > multi-column element, then we should see such bright green
>  > 32px-wide-by-16px-tall rectangle at the top left corner. And so, the
>  > comparison with the reftest should work by itself; the lime stripes
>  > position and dimension would need to be identical.
>  >
>  > So, I think that the div::after rule is not needed, not required in
> the
>  > test and that line 26 is also not needed, not necessary.
>
> Your analysis seems right to me; I don't know why the white rectangle
> was there.

I know now why this white 2x1 rectangle. If margin-top was collapsing,
then we would see a short bright green stripe too but located at the
bottom-right corner of the div. So, to help figure out manually the
test's pass/fail conditions, to clarify the test's pass/fail conditions,
the test passes if there is a short bright green stripe. If the whole
page is white, then it means that the test failed. That's most likely
the design logic followed here.

It could be possible to create a test where
a) if 2 margin-top are collapsing, then we would see red and
b) if 2 margin-top are *not* collapsing, then we would see only green.

Basically, set a red background on the multi-column element and then
relatively positioned a green area/shape only to overlap over the
upper-left quadrant.

This also meets another guideline of tests:
"
Since green-with-no-red is often used to indicate success, it's best to
also avoid green unless using the presence of red to indicate failures.
"
http://wiki.csswg.org/test/format#design-requirements

>  > As is, the test is still a correct one but it's imprecise, not
>  > straightforward, streamlined.
>
> Thanks for your detailed analysis.
>
> I'll update the test in the repository once I hear back about the font
> size.
>
> BTW, I'm also working my way through the tests -- here are my sketchy
> notes on conformance:
>
>   http://people.opera.com/howcome/2013/tests/multicol-table.html
>
> There are two good implemenetations that should help us through to PR.


Manually, it's possible - but long and tedious - to check those tests
with Firefox 22 and see how good its implementation is: you have to
manually edit the -moz- multi-column properties with the Style Editor.
Same thing with Chrome 28: it's possible to manually check those tests
and see how good its implementation is; you have to manually edit the
-webkit- multi-column properties.

> I'll need help in having the tests reviewed, though -- I can't review
> tests submitted by Opera, I believe.
>
> Cheers,

Gérard
-- 
Contributions to the CSS 2.1 test suite:
http://www.gtalbot.org/BrowserBugsSection/css21testsuite/

CSS 2.1 Test suite RC6, March 23rd 2011:
http://test.csswg.org/suites/css2.1/20110323/html4/toc.html

CSS 2.1 test suite harness:
http://test.csswg.org/harness/

Contributing to to CSS 2.1 test suite:
http://www.gtalbot.org/BrowserBugsSection/css21testsuite/web-authors-contributions-css21-testsuite.html

Received on Wednesday, 3 July 2013 04:01:58 UTC