Re: fixed new inline-block alignement test (complete review)

Le 2015-07-10 23:37, 塩澤 元 a écrit :
> Hi Gérard,
> 
> I have fixed inline-block alignment test.
> https://hg.csswg.org/test/rev/865ca430216b
> 
> <Change Point>
> 1. fixed the test while referring to your feedback.
> 
> 2.added other test case
> - for central baseline
> -- inline-block-alignment-new-003.xht (vertical-lr + mixed)
> -- inline-block-alignment-new-004.xht (vertical-rl + upright)
> -- inline-block-alignment-new-005.xht (vertical-lr + upright)
> - for alphabetical baseline
> -- inline-block-alignment-new-007.xht (vertical-lr + sideways)
> -- inline-block-alignment-new-008.xht (vertical-rl + sideways-right)
> -- inline-block-alignment-new-009.xht (vertical-lr + sideways-right)
> -- inline-block-alignment-new-010.xht (vertical-rr + sideways-left)
> -- inline-block-alignment-new-011.xht (vertical-lr + sideways-left)
> 
> 3. added ref file
> - inline-block-alignment-new-003-ref.xht (for vertical-lr 
> central-baseline)
> - inline-block-alignment-new-007-ref.xht (for vertical-lr
> alphabetical-baseline)
> 
> Could you review it?
> 
> I have known that sideways-left is at risk now and may be dropped 
> during
> the CR period^[1].
> However, anyway, I have created test for sideways-left because deleting
> test case is very easy :-)
> 
> [1]: "Status of this document" from
> http://dev.w3.org/csswg/css-writing-modes-3/
>> The following features are at-risk, and may be dropped during the CR
> period:
>>    * The sideways-left of text-orientation
>>    * The use-glyph-orientation of text-orientation
>>    * The digits value of text-combine-upright.
>>    * The look-ahead/look-behind sequencing rules for 
>> text-combine-upright.
> 
> 
> Hajime.
> 
> 
> 2015-07-06 4:30 GMT+09:00 Gérard Talbot <css21testsuite@gtalbot.org>:
> 
>> Le 2015-07-05 02:25, 塩澤 元 a écrit :
>> 
>>> Gérard,
>>> 
>>> I have fixed the inline-block alignement test in reference to your
>>> review^[1]
>>> 
>>> https://hg.csswg.org/test/rev/4d6bad11f62e
>>> 
>>> Could you review it?
>>> 
>>> After your review and approval, I will create other variation of
>>> inline-block alignment.
>>> 
>>> [1]:
>>> http://lists.w3.org/Archives/Public/public-css-testsuite/2015Jun/0007.html
>>> 
>>> Hajime.
>>> 
>> 
>> Here is what I came up with:
>> 
>> 
>> http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/inline-block-alignment-new-002-Hajime.xht
>> 
>> Your test:
>> 
>> http://test.csswg.org/source/css-writing-modes-3/inline-block-alignment-new-002.xht
>> 
>> 1- (line 15 in your test)
>> It's always safer to use a numerical line-height (1) instead of a
>> font-size (1em) because computed line-height is inherited by default; 
>> a
>> numerical line-height will scale with relevant font-size. In your 
>> test, 1em
>> was okay since other inline boxes were taller.
>> 
>> 2- (lines 24 and 25 in your test)
>> Creating an asymetrical vertical padding on inline boxes can better 
>> reveal
>> an implementation bug.
>> I've added a /* comment */ explaining the purpose of such logical 
>> vertical
>> padding
>> 
>> 3- (line 26 in your test)
>> I've removed color: fuchsia.
>> 
>> 4- (line 33 in your test)
>> Since line-height is inherited, then you do not need to redeclare it 
>> for
>> its descendants.
>> 
>> 5- (lines 36 and 41 in your test)
>> I've used id instead of classes for first-line-box and last-line-box
>> 
>> 6- (lines 49 and 50 in your test)
>> Asymetrical vertical padding on that inline plus a /* comment */
>> 
>> If you now load that test into the latest most recent Firefox 42 
>> nightly
>> build, you can see 2 bugs occuring. The left padding and right padding 
>> on
>> the inline boxes should not affect baseline alignment of text (the
>> horizontal position of those orange squares with respect to the blue
>> square) on the dominant baseline... whatever such dominant baseline is 
>> and
>> however how baseline-alignment is implemented.
>> 
>> Adapted reference file:
>> 
>> http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/inline-block-alignment-new-002-Hajime-ref.xht
>> 

Hajime,

Sorry for the long delay. Here is a preliminary review:

1-

http://test.csswg.org/source/css-writing-modes-3/inline-block-alignment-new-003-ref.xht

You need to swap ( 並び替えます ) the yellow and blue images, like this:


<div>
    <img src="support/swatch-orange.png" width="60" height="60" 
alt="Image download support must be enabled" /><br /><!--
--><img class="left" src="support/swatch-yellow.png" width="120" 
height="120" alt="Image download support must be enabled" /><!--
--><img class="right" src="support/swatch-blue.png" width="120" 
height="120" alt="Image download support must be enabled" /><br /><!--
--><img src="support/swatch-orange.png" width="30" height="30" 
alt="Image download support must be enabled" /></div>


2-

http://test.csswg.org/source/css-writing-modes-3/inline-block-alignment-new-005.xht

line 9: (...) and when 'text-orientation' is 'upright', then (...)
line 18:   text-orientation: mixed;

-- continuation --


3-

     span#orange30
     {
       display: inline-block;


     span#fuchsia30
     {
       display: inline-block;


In all your tests, you have been declaring 'display: inline-block' onto 
the smallest Ahem glyph (square) which follows the tested inline-block. 
Please explain why. I do not see the need to do this. It does not make 
your tests incorrect or unreliable ... but this declaration is 
extraneous to me. I would remove this.

4-

<img class="left" src="support/swatch-blue.png" width="120" height="120" 
alt="Image download support must be enabled" /><!--
--><img class="right" src="support/swatch-yellow.png" width="120" 
height="120" alt="Image download support must be enabled" /><br />

Please remove class="left" and class="right" from the reference files
http://test.csswg.org/source/css-writing-modes-3/inline-block-alignment-new-002-ref.xht
http://test.csswg.org/source/css-writing-modes-3/inline-block-alignment-new-003-ref.xht
as they are not defined and they are not needed.

5-

http://test.csswg.org/source/css-writing-modes-3/inline-block-alignment-new-009.xht

I believe inline-block-alignment-new-009.xht's reference file should be

inline-block-alignment-new-007-ref.xht and not 
inline-block-alignment-new-006-ref.xht

and inline-block-alignment-new-009's pass-fail-conditions should be

<p>Test passes if the <strong>right edge</strong> of an irregular 
polygon is straight and unbroken.</p>

and not left edge.


6-

http://test.csswg.org/source/css-writing-modes-3/inline-block-alignment-new-010.xht

I believe inline-block-alignment-new-010.xht's reference file should be

inline-block-alignment-new-006-ref.xht and not 
inline-block-alignment-new-007-ref.xht

and inline-block-alignment-new-010's pass-fail-conditions should be

<p>Test passes if the <strong>left edge</strong> of an irregular polygon 
is straight and unbroken.</p>

and not right edge.

Gérard
-- 
Test Format Guidelines
http://testthewebforward.org/docs/test-format-guidelines.html

Test Style Guidelines
http://testthewebforward.org/docs/test-style-guidelines.html

Test Templates
http://testthewebforward.org/docs/test-templates.html

CSS Naming Guidelines
http://testthewebforward.org/docs/css-naming.html

Test Review Checklist
http://testthewebforward.org/docs/review-checklist.html

CSS Metadata
http://testthewebforward.org/docs/css-metadata.html

Received on Tuesday, 14 July 2015 08:09:18 UTC