Skip to content

Implement margin-trim for floats in block containers that contain only block boxes. #8910

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

Conversation

sammygill
Copy link
Contributor

@sammygill sammygill commented Jan 21, 2023

f679766

Implement margin-trim for floats in block containers that contain only block boxes.
https://bugs.webkit.org/show_bug.cgi?id=249207
rdar://103285803

Reviewed by Alan Baradlay.

Another patch will be added to provide support for block containers that
contain only inline level boxes.

There are 3 different pieces that we need to implement:

1.) Trimming the block start, inline start, and inline end margins
2.) Trimming the block-end margin, which is slightly different from
the others
3.) Making sure the trimmed margins do not contribute to the inrinsic
sizing of the container

When a candidate position for a float is determined in the float
positioning code, we can determine whether it is possible to trim its
margins. For each candidate position, we can determine whether any edges
of the margin box would touch the container and then trim that edge if
it is specified in margin-trim. This can be done for the inline-start,
inline-end, and block-start edges of the container.

Trimming the block-end margin is slightly different because it is not
in the context of the containing block like the other margins but of the
block formatting context. This means that we can only trim these margins
after its block formatting context is done with layout. In
RnderBlockFlow we can check to see if it establishes a block formatting
context and then proceed to trim the block-end margins of its floats
where needed. As we iterate over each float, we check to see if its block
end location is lower than the lowest piece of content in the BFC. If
so, we trim it up to the necesssary amount where it will not extend the
height of the BFC.

Finally, to make sure that the trimmed inline margins do not contribute
to the intrinsic sizing of its containing block, all we need to do
is check to see if the item is a float and if any of the inline margins
are specified to be trimmed. If so, we do not include those in its
contribution.

* LayoutTests/TestExpectations:
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-end-deeply-nested-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-end-deeply-nested.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-end-up-to-content-block-layout-different-containers-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-end-up-to-content-block-layout-different-containers-vert-lr-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-end-up-to-content-block-layout-different-containers-vert-lr.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-end-up-to-content-block-layout-different-containers.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-end-up-to-content-block-layout-same-container-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-end-up-to-content-block-layout-same-container.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-start-block-layout-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-start-block-layout.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-start-relative-positioned-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-start-relative-positioned.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-start-with-transforms-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-start-with-transforms.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-fit-content-block-layout-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-fit-content-block-layout.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-inline-end-block-layout-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-inline-end-block-layout.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-inline-start-block-layout-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-inline-start-block-layout.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-left-trimmed-margin-allows-float-to-fit-block-layout-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-left-trimmed-margin-allows-float-to-fit-block-layout-vert-lr-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-left-trimmed-margin-allows-float-to-fit-block-layout-vert-lr.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-left-trimmed-margin-allows-float-to-fit-block-layout.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-min-content-with-block-content-block-layout-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-min-content-with-block-content-block-layout.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-right-trimmed-margin-allows-float-to-fit-block-layout-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-right-trimmed-margin-allows-float-to-fit-block-layout.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-overflowing-float-margins-tirmmed-at-final-position-block-layout-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-overflowing-float-margins-tirmmed-at-final-position-block-layout.html: Added.
* Source/WebCore/rendering/FloatingObjects.cpp:
(WebCore::FloatingObject::isLowestPlacedFloatBottomInBlockFormattingContext const):
* Source/WebCore/rendering/FloatingObjects.h:
(WebCore::FloatingObject::setHeight):
* Source/WebCore/rendering/RenderBlock.cpp:
(WebCore::RenderBlock::computeBlockPreferredLogicalWidths const):
* Source/WebCore/rendering/RenderBlock.h:
(WebCore::RenderBlock::logicalMarginBoxHeightForChild const):
(WebCore::RenderBlock::logicalMarginBoxTopForChild const):
* Source/WebCore/rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::layoutBlock):
(WebCore::RenderBlockFlow::trimFloatBlockEndMargins):
(WebCore::RenderBlockFlow::shouldChildInlineMarginContributeToContainerIntrinsicSize const):
(WebCore::RenderBlockFlow::insertFloatingObject):
(WebCore::RenderBlockFlow::trimMarginForFloat):
(WebCore::RenderBlockFlow::computeLogicalLocationForFloat):
(WebCore::RenderBlockFlow::blockFormattingContextInFlowBlockLevelContentHeight const):
* Source/WebCore/rendering/RenderBlockFlow.h:
* Source/WebCore/rendering/RenderBox.cpp:
(WebCore::RenderBox::establishesBlockFormattingContext const):
(WebCore::RenderBox::blockFormattingContextRoot const):
* Source/WebCore/rendering/RenderBox.h:

Canonical link: https://commits.webkit.org/260318@main

fdb75fc

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk1 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
✅ 🛠 🧪 merge ✅ 🛠 watch-sim

@sammygill sammygill force-pushed the eng/margin-trim-Implement-margin-trims-for-floats-within-a-block-container branch from de74673 to 26913a3 Compare January 21, 2023 02:47
@sammygill sammygill self-assigned this Jan 21, 2023
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 21, 2023
@sammygill sammygill added skip-ews Applied to prevent a change from being run on EWS and removed skip-ews Applied to prevent a change from being run on EWS merging-blocked Applied to prevent a change from being merged labels Jan 21, 2023
@sammygill sammygill force-pushed the eng/margin-trim-Implement-margin-trims-for-floats-within-a-block-container branch from 26913a3 to 18be5d8 Compare January 21, 2023 05:10
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 21, 2023
@sammygill sammygill removed the merging-blocked Applied to prevent a change from being merged label Jan 21, 2023
@sammygill sammygill changed the title Implement margin-trim for floats in block container. Fix style invalidation for ancestor documents of fullscreen element Jan 21, 2023
@sammygill sammygill force-pushed the eng/margin-trim-Implement-margin-trims-for-floats-within-a-block-container branch from 18be5d8 to f5f1db7 Compare January 21, 2023 07:12
@sammygill sammygill added the Media Bugs related to the HTML 5 Media elements. label Jan 21, 2023
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 21, 2023
@sammygill sammygill removed request for rniwa and cdumez January 21, 2023 18:29
@sammygill sammygill removed the merging-blocked Applied to prevent a change from being merged label Jan 21, 2023
@sammygill sammygill changed the title Fix style invalidation for ancestor documents of fullscreen element Implement margin-trim for floats (draft). Jan 21, 2023
@sammygill sammygill force-pushed the eng/margin-trim-Implement-margin-trims-for-floats-within-a-block-container branch from f5f1db7 to e46bf26 Compare January 21, 2023 18:31
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 21, 2023
@sammygill sammygill removed the merging-blocked Applied to prevent a change from being merged label Jan 23, 2023
@sammygill sammygill changed the title Implement margin-trim for floats (draft). Implement margin-trim for float (draft) Jan 23, 2023
@sammygill sammygill force-pushed the eng/margin-trim-Implement-margin-trims-for-floats-within-a-block-container branch from e46bf26 to 4a89c14 Compare January 23, 2023 07:23
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 23, 2023
@sammygill sammygill force-pushed the eng/margin-trim-Implement-margin-trims-for-floats-within-a-block-container branch from 66cf0dc to f3e2720 Compare February 13, 2023 20:39
@sammygill sammygill force-pushed the eng/margin-trim-Implement-margin-trims-for-floats-within-a-block-container branch from f3e2720 to 5992334 Compare February 13, 2023 21:13
@sammygill sammygill force-pushed the eng/margin-trim-Implement-margin-trims-for-floats-within-a-block-container branch from 5992334 to 58a94a0 Compare February 13, 2023 21:13
@sammygill sammygill force-pushed the eng/margin-trim-Implement-margin-trims-for-floats-within-a-block-container branch from 58a94a0 to 586d366 Compare February 14, 2023 05:17
@sammygill sammygill force-pushed the eng/margin-trim-Implement-margin-trims-for-floats-within-a-block-container branch from 586d366 to e5b6b0d Compare February 14, 2023 21:08
Copy link
Contributor

@alanbaradlay alanbaradlay left a comment

Choose a reason for hiding this comment

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

this should be good to go (after that &style() adjustment).

@sammygill sammygill force-pushed the eng/margin-trim-Implement-margin-trims-for-floats-within-a-block-container branch 2 times, most recently from 07eed4a to fdb75fc Compare February 15, 2023 06:18
@sammygill sammygill added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels Feb 15, 2023
…y block boxes.

https://bugs.webkit.org/show_bug.cgi?id=249207
rdar://103285803

Reviewed by Alan Baradlay.

Another patch will be added to provide support for block containers that
contain only inline level boxes.

There are 3 different pieces that we need to implement:

1.) Trimming the block start, inline start, and inline end margins
2.) Trimming the block-end margin, which is slightly different from
the others
3.) Making sure the trimmed margins do not contribute to the inrinsic
sizing of the container

When a candidate position for a float is determined in the float
positioning code, we can determine whether it is possible to trim its
margins. For each candidate position, we can determine whether any edges
of the margin box would touch the container and then trim that edge if
it is specified in margin-trim. This can be done for the inline-start,
inline-end, and block-start edges of the container.

Trimming the block-end margin is slightly different because it is not
in the context of the containing block like the other margins but of the
block formatting context. This means that we can only trim these margins
after its block formatting context is done with layout. In
RnderBlockFlow we can check to see if it establishes a block formatting
context and then proceed to trim the block-end margins of its floats
where needed. As we iterate over each float, we check to see if its block
end location is lower than the lowest piece of content in the BFC. If
so, we trim it up to the necesssary amount where it will not extend the
height of the BFC.

Finally, to make sure that the trimmed inline margins do not contribute
to the intrinsic sizing of its containing block, all we need to do
is check to see if the item is a float and if any of the inline margins
are specified to be trimmed. If so, we do not include those in its
contribution.

* LayoutTests/TestExpectations:
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-end-deeply-nested-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-end-deeply-nested.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-end-up-to-content-block-layout-different-containers-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-end-up-to-content-block-layout-different-containers-vert-lr-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-end-up-to-content-block-layout-different-containers-vert-lr.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-end-up-to-content-block-layout-different-containers.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-end-up-to-content-block-layout-same-container-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-end-up-to-content-block-layout-same-container.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-start-block-layout-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-start-block-layout.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-start-relative-positioned-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-start-relative-positioned.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-start-with-transforms-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-block-start-with-transforms.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-fit-content-block-layout-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-fit-content-block-layout.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-inline-end-block-layout-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-inline-end-block-layout.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-inline-start-block-layout-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-inline-start-block-layout.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-left-trimmed-margin-allows-float-to-fit-block-layout-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-left-trimmed-margin-allows-float-to-fit-block-layout-vert-lr-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-left-trimmed-margin-allows-float-to-fit-block-layout-vert-lr.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-left-trimmed-margin-allows-float-to-fit-block-layout.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-min-content-with-block-content-block-layout-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-min-content-with-block-content-block-layout.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-right-trimmed-margin-allows-float-to-fit-block-layout-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-float-right-trimmed-margin-allows-float-to-fit-block-layout.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-overflowing-float-margins-tirmmed-at-final-position-block-layout-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/block-container-overflowing-float-margins-tirmmed-at-final-position-block-layout.html: Added.
* Source/WebCore/rendering/FloatingObjects.cpp:
(WebCore::FloatingObject::isLowestPlacedFloatBottomInBlockFormattingContext const):
* Source/WebCore/rendering/FloatingObjects.h:
(WebCore::FloatingObject::setHeight):
* Source/WebCore/rendering/RenderBlock.cpp:
(WebCore::RenderBlock::computeBlockPreferredLogicalWidths const):
* Source/WebCore/rendering/RenderBlock.h:
(WebCore::RenderBlock::logicalMarginBoxHeightForChild const):
(WebCore::RenderBlock::logicalMarginBoxTopForChild const):
* Source/WebCore/rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::layoutBlock):
(WebCore::RenderBlockFlow::trimFloatBlockEndMargins):
(WebCore::RenderBlockFlow::shouldChildInlineMarginContributeToContainerIntrinsicSize const):
(WebCore::RenderBlockFlow::insertFloatingObject):
(WebCore::RenderBlockFlow::trimMarginForFloat):
(WebCore::RenderBlockFlow::computeLogicalLocationForFloat):
(WebCore::RenderBlockFlow::blockFormattingContextInFlowBlockLevelContentHeight const):
* Source/WebCore/rendering/RenderBlockFlow.h:
* Source/WebCore/rendering/RenderBox.cpp:
(WebCore::RenderBox::establishesBlockFormattingContext const):
(WebCore::RenderBox::blockFormattingContextRoot const):
* Source/WebCore/rendering/RenderBox.h:

Canonical link: https://commits.webkit.org/260318@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/margin-trim-Implement-margin-trims-for-floats-within-a-block-container branch from fdb75fc to f679766 Compare February 15, 2023 18:28
@webkit-commit-queue
Copy link
Collaborator

Committed 260318@main (f679766): https://commits.webkit.org/260318@main

Reviewed commits have been landed. Closing PR #8910 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit f679766 into WebKit:main Feb 15, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Feb 15, 2023
@sammygill sammygill deleted the eng/margin-trim-Implement-margin-trims-for-floats-within-a-block-container branch April 19, 2024 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Layout and Rendering For bugs with layout and rendering of Web pages.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants