Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added a 'column-fill: auto' and 'widows' test #26442

Merged
merged 6 commits into from Nov 21, 2020

Conversation

TalbotG
Copy link
Contributor

@TalbotG TalbotG commented Nov 8, 2020

columnfill-auto-widows-002.html
reference/columnfill-auto-widows-002-ref.html

This test is currently failed by Firefox 78+ (because Firefox does not support widows nor orphans; see https://bugzilla.mozilla.org/show_bug.cgi?id=137367 ) but is passed by Chromium 83+ and Epiphany 3.32.1.2 (WebKitGTK+ 2.28.4).

When column boxes are filled sequentially, their content should be adjusted (can be broken) so that they honor the 'widows' declaration. This test checks exactly that. In the test, since the 3rd column box was going to get only 1 line box, then a class B break point is allowed and should occur before the last line box of the 2nd column box and after the next-to-last line box of the 2nd column box so that there is a minimum of 2 line boxes at the top of the 3rd column box.

Rule 3: Breaking at a class B break point is allowed only if (...) the number of line boxes between the break and the end of the box is the value of widows or more.
https://www.w3.org/TR/css-break-3/#unforced-breaks

Note that this columnfill-auto-widows-002.html test meets the 600px wide window viewport requirement.

Other related tests:

http://www.gtalbot.org/BrowserBugsSection/CSS3Multi-Columns/column-fill-balance-0xx.html

http://www.gtalbot.org/BrowserBugsSection/CSS3Multi-Columns/columnfill-auto-widows-001.html

@TalbotG TalbotG self-assigned this Nov 8, 2020
@TalbotG TalbotG requested review from fantasai, rachelandrew and mstensho and removed request for rachelandrew and mstensho November 8, 2020 01:56
@TalbotG TalbotG changed the title Added 1 important column-fill: auto test Added a 'column-fill: auto' and 'widows' test Nov 8, 2020
@rachelandrew
Copy link
Contributor

@TalbotG @fantasai @frivoal I wonder if this test should be stored with other fragmentation tests rather than with multicol. I've got a whole bunch of multicol tests that relate to fragmentation which don't really have any multicol spec text to link to. Not sure how much it matters as long as they are somewhere but there are also already some multicol tests under css-break too https://github.com/web-platform-tests/wpt/tree/master/css/css-break

@TalbotG
Copy link
Contributor Author

TalbotG commented Nov 8, 2020

In my opinion, this test really checks how content distribution inside a multi-column container with 'column-fill: auto' can be affected and should be affected by 'widows'. So, it involves an interaction of 2 properties originating from 2 distinct specs. So, I think links to both specs should be listed.

whole bunch of multicol tests that relate to fragmentation (...) Not sure how much it matters

I think those multicol tests should have links to both specs but it is not a serious mistake if they have not. It does not matter very much as long as the tests are correct, reliable, trustworthy, testing something relevant, demonstrating an implementation failure, etc...

@TalbotG
Copy link
Contributor Author

TalbotG commented Nov 8, 2020

@TalbotG
Copy link
Contributor Author

TalbotG commented Nov 8, 2020

In the 349b287 commit, I have improved the title text a bit, I included a text assert and I have removed alphabetic text and used instead only a digit along with an explicit line break. The reference file has been updated accordingly.

Copy link
Contributor

@mstensho mstensho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I too think that this is a generic block fragmentation test, and that it doesn't really have anything in particular to do with multicol (it's just that multicol is the most convenient block fragmentation type to use in tests). Using column-fill:auto to disable column balancing strongly supports this.

I think this test would look good as css/css-break/widows-orphans-006.html

css/css-multicol/columnfill-auto-widows-002.html Outdated Show resolved Hide resolved
@TalbotG
Copy link
Contributor Author

TalbotG commented Nov 15, 2020

{
it doesn't really have anything in particular to do with multicol
}

The test demonstrates that content distribution among column boxes in the way/manner it is prescribed for 'column-fill: auto' can be affected, can be influenced by 'widows' property. The test is about the interaction of 2 distinct properties. In my opinion, it should have the link to
https://www.w3.org/TR/css-multicol-1/#filling-columns

{
this test would look good as css/css-break/widows-orphans-006.html
}

'orphans' is not tested, is not involved and is not "in play" in the test. Why the filename should have "orphans" in it then ... instead of "columnfill-auto" as of now?

@mstensho
Copy link
Contributor

Non-auto height and column-fill:auto is an excellent (and in many ways the only feasible one) way of explicitly setting the fragmentainer block-size. The rest of this test is pure block fragmentation. It just so happens that multicol is usually the best way of testing block fragmentation. We could write a printing test to achieve the same, but that's just more tedious.

You're right, though: it doesn't test orphans, just widows. So... css/css-break/widows-001.html

@TalbotG
Copy link
Contributor Author

TalbotG commented Nov 16, 2020

@mstensho
Okay. I will fix the test (later today) so that it does not link to the Multi-Column spec; link only to CSS Fragmentation level 3 Test.

{
So... css/css-break/widows-001.html
}

Already taken:
http://test.csswg.org/suites/css21_dev/20110323/html4/widows-001.htm

I am working on a test which will verify various combinations of both 'orphans' and 'widows' within an auto-height multi-column container. It will most likely use Ahem font and will comply with the 600px wide window viewport requirement. Draft test right now:

http://www.gtalbot.org/BrowserBugsSection/CSS3Break/widows-orphans-011.html

It will most likely be split in 2 tests (4 sub-tests per test).

[Edited]
The test
css/css-multicol/columnfill-auto-widows-002.html
and its reference
css/css-multicol/reference/columnfill-auto-widows-002-ref.html

will migrate to and will be filename-renamed as

css/css-break/widows-008.html
css/css-break/reference/widows-008-ref.html
in a few min.
[/Edited]

@wpt-pr-bot
Copy link
Collaborator

There are no reviewers for this pull request. Please reach out on W3C's irc server (irc.w3.org, port 6665) on channel #testing (web client) to get help with this. Thank you!

@TalbotG
Copy link
Contributor Author

TalbotG commented Nov 17, 2020

rename css/{css-multicol/reference/columnfill-auto-widows-002-ref.html => css-break/reference/widows-008-ref.html} (96%)

rename css/{css-multicol/columnfill-auto-widows-002.html => css-break/widows-008.html} (80%)

Copy link
Contributor

@mstensho mstensho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why widows-008.html ? There are no widows-* in that directory.

css/css-break/widows-008.html Outdated Show resolved Hide resolved
css/css-break/widows-008.html Outdated Show resolved Hide resolved
css/css-break/widows-008.html Outdated Show resolved Hide resolved
Copy link
Contributor

@mstensho mstensho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@TalbotG TalbotG merged commit 32e7029 into web-platform-tests:master Nov 21, 2020
@TalbotG TalbotG deleted the CSS3Multicolumn-GT-PR2 branch November 21, 2020 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants