Skip to content

Modified and corrected CSS2/box-display/containing-block-017.xht #38687

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

TalbotG
Copy link
Contributor

@TalbotG TalbotG commented Feb 23, 2023

Previous containing-block-017.xht test:
https://wpt.live/css/CSS2/box-display/containing-block-017.xht

or (at my website)

http://www.gtalbot.org/BrowserBugsSection/css22testsuite/previous-CSS2-box-display-containing-block-017.xht

New containing-block-017.xht test replacing previous:
http://www.gtalbot.org/BrowserBugsSection/css21testsuite/new-containing-block-017.xht

Notes:

  • The number one reason for this replacement is that in CSS2.x spec, §10.1, parg 4, list item 1 , if an inline parent is acting as a containing block for positioned elements and if such inline element is split across multiple lines, then the containing block is undefined. The previous test generates 3 line boxes. The new and replacing test generates only and exactly 1 line box.
  • I removed the margin-bottom: 20px at line 16 because it was useless, pointless, unneeded, irrelevant
  • I added font-size: 20px; line-height: 30px; because, right now, this new and replacing test (just like the previous test) has no reference file. I tried to create a reference file for it but it is not obvious, it is tricky to do and it would require more thinking, conception time. If one day a reference file can be created, it should (otherwise will definitely) benefit from having already a defined font-size and line-height declarations
  • I slightly renamed the class names and identifiers for easier examination and understanding
  • I tried to keep the changes in the code to minimum to facilitate reviewing
  • Merging this PR should close Issue 38651
  • The only reason why Firefox 102.8.0ESR fails this test appears to be the preservation of the white space between the last inline box and its relatively-positioned parent, the span#containing-block , between line 71 and line 72
  • The previous test has been saved on my website here:
    http://www.gtalbot.org/BrowserBugsSection/css22testsuite/previous-CSS2-box-display-containing-block-017.xht
    so that we can try to adjust it or improve it or rehabilitate it once Issue 8284 has been resolved and then to add it into CSS3 Positioned Layout test suite
  • More info and explanations:
    [RC6] containing-block-017 : proposed improvements (9 Mar 2012)
    https://lists.w3.org/Archives/Public/public-css-testsuite/2012Mar/0008.html

@TalbotG TalbotG added the CSS2 label Feb 23, 2023
@TalbotG TalbotG requested review from frivoal and fantasai February 23, 2023 22:55
@TalbotG TalbotG self-assigned this Feb 23, 2023
@svgeesus svgeesus removed their request for review March 1, 2023 13:58
Copy link
Contributor

@fantasai fantasai left a comment

Choose a reason for hiding this comment

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

  1. Rather than removing the testcase, move it to the css-position/ folder so it can serve as a test for [css-position-3] Containing block formed by fragmented inlines w3c/csswg-drafts#8284 and add this new one.
  2. In the new CSS2 testcase, use background-image to position the red squares and to make the reference file. Since CSS2 doesn't have multiple background images, you will need an extra wrapper SPAN around #test to hold one of the background images.
  3. Since we're testing single-line content, shorten the words on the line and remove the div's width so that the text doesn't wrap if the font changes to something wider.
  4. There is no need to rename any of the selectors, they were fine.
  5. You can remove the comments about left-to-right. If we want a right-to-left test, we should include it in this file. (In this case, we would have to change the selectors from # to .)

@dbaron dbaron removed their request for review June 23, 2023 15:52
@TalbotG
Copy link
Contributor Author

TalbotG commented Dec 18, 2023

  1. Rather than removing the testcase, move it to the css-position/ folder so it can serve as a test for [css-position-3] Containing block formed by fragmented inlines w3c/csswg-drafts#8284 and add this new one.

When I am able to submit-git-push a commit that will include a reference file as well, I will move the improved containing-block-017 test to the css-position/ folder

Done at my website:
http://www.gtalbot.org/BrowserBugsSection/CSS3PositionedLayout/newer-containing-block-017.xht

  1. In the new CSS2 testcase, use background-image to position the red squares and to make the reference file.

I agree that we should try to use a background-image for displaying the red squares. So far, I have not succeeded. I am still working on this.

Since CSS2 doesn't have multiple background images, you will need an extra wrapper SPAN around #test to hold one of the background images.

So far, I have not succeeded. I am still working on this.

  1. Since we're testing single-line content, shorten the words on the line

Done. The words
"First inline box Last inline box"
are now
"First inline Last inline"
In case we need to shorten further the words,
then we can use
"First Last"

remove the div's width

Done. I also changed the border color of the div.

  1. There is no need to rename any of the selectors, they were fine.

I have renamed the selectors back to what they were in the original containing-block-017.xht test:

.abs-pos is now back to .position
#containing-block is now back to #test
#overlapping-green-TL is now back to .top-left
#overlapping-green-BR is now back to .bottom-right
in this file:

http://www.gtalbot.org/BrowserBugsSection/CSS3PositionedLayout/newer-containing-block-017.xht

  1. You can remove the comments about left-to-right. If we want a right-to-left test, we should include it in this file. (In this case, we would have to change the selectors from # to .)

I have removed the comments about left-to-right

Additionnally:

  • I have removed the "dom interact" flags
  • I have removed the font-size and line-height declarations. In an ideal coding context, situation, a test of this sort should succeed regardless of font-size and line-height. If this is not possible, then I will redeclare these.

@TalbotG
Copy link
Contributor Author

TalbotG commented Dec 19, 2023

  1. In the new CSS2 testcase, use background-image to position the red squares and to make the reference file. Since CSS2 doesn't have multiple background images, you will need an extra wrapper SPAN around #test to hold one of the background images.

I tried to implement your proposal above but could not. See

http://www.gtalbot.org/BrowserBugsSection/CSS3PositionedLayout/draft-containing-block-017.xht

Vertical margins (margin-top, margin-bottom) have no effect on non-replaced inline elements and css width and css height do not apply either on non-replaced inline elements.

@TalbotG
Copy link
Contributor Author

TalbotG commented Dec 21, 2023

At my website:

http://www.gtalbot.org/BrowserBugsSection/CSS3PositionedLayout/newest-containing-block-017.xht

http://www.gtalbot.org/BrowserBugsSection/CSS3PositionedLayout/reference/containing-block-017-ref.html

Somehow, it does not seem possible to keep the "TL" and "BR" text in the reference file. I added a few minor comments in the test to explain some values.

@TalbotG
Copy link
Contributor Author

TalbotG commented Dec 21, 2023

One Continuous Integration check, wpt-decision-task, failed and I do not know why. I will close this PR and will create another one, hoping that it will succeed.

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.

[CSS2] Modifying and replacing CSS2/box-display/containing-block-017 test
4 participants