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

[css-overflow-3] Define how child margin-boxes contributes to scrollable overflow (ISSUE 6) #3653

Closed
MatsPalmgren opened this issue Feb 17, 2019 · 4 comments

Comments

@MatsPalmgren
Copy link

https://drafts.csswg.org/css-overflow-3/#scrollable

The UA may additionally include the margin areas of boxes for which it is the containing block.
The conditions under which such margin areas are included is undefined in this level.
ISSUE 6. This needs further testing and investigation; is therefore deferred in this draft.

Firefox and Chrome handles margins differently (see testcases in bug 1527539). This might lead to web-compat problems.
It looks like Chrome includes the block-end margin of children when calculating the overflow for Grid and Flexbox layout, whereas Firefox doesn't. Both includes it in block layout though, well at least in most cases... (exact details TBD)

(Fwiw, the relevant block layout code in Gecko lives here in case anyone wants to decipher when the margin is included in blockEndEdgeOfChildren there.)

@upsuper
Copy link
Member

upsuper commented Feb 17, 2019

It's worth checking with Chrome team about their behavior of LayoutNG, their next generation layout engine. I have heard that for many cases, they have different behavior than their current engine.

@emilio
Copy link
Collaborator

emilio commented Feb 18, 2019

cc @kojiishi @bfgeek, do you know what does LayoutNG do here?

@fantasai
Copy link
Collaborator

Probably should include margins here, alongside padding. Same principle: authors expect spacing to be there when they request spacing, regardless of which side of the scrollbox the item is on. Agenda+ to discuss.

Previous resolutions related to this topic: #3665

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed margins and scrollable overflow, and agreed to the following:

  • RESOLVED: for flex and grid, margins contribute to overflow
The full IRC log of that discussion <fantasai> Topic: margins and scrollable overflow
<fantasai> github: https://github.com//issues/3653
<fantasai> fantasai: ....
<fantasai> florian: For all things where we don't have a legacy constraint, I'm in favor of including margins, and this is one of them
<florian> fantasai: for blocks we cannot just include the margins, because compat, but for grid and flex we don't have this problem
<florian> fantasai: so when you put overflow on things, and have margins, authors expect them to show up
<fantasai> [fantasai was explaining why spacing specified by the author should be included in scrollable overflow -- it's used for spacing, and needs to provide that spacing between item and bottom/right edges of scroll container]
<fantasai> [ but have compat issues with block containers, would create too many unexpected scrollbars; so adapting for grid flexbox only ]
<florian> proposed resolution: for flex and grid, margins contribute to overflow
<florian> RESOLVED: for flex and grid, margins contribute to overflow
<fantasai> end topic
<fantasai> github: https://github.com//issues/3653
<fantasai> end topic

@fantasai fantasai added css-overflow-3 Current Work and removed Needs Edits labels Jun 7, 2019
@fantasai fantasai closed this as completed Jun 7, 2019
@fantasai fantasai added this to the css-grid-1 CR 2017-12-14+ milestone Jul 22, 2020
webkit-commit-queue pushed a commit to WebKit/WebKit that referenced this issue Sep 15, 2021
https://bugs.webkit.org/show_bug.cgi?id=226981

Reviewed by Javier Fernandez.

When computing the overflow area in the grid container, we need to obtain the result of ClientLogicalBottom
function. This value should be retrieved after computing the actual logical height rather than before.

For grid, width/height of the overflow rect should be the width/height of the grid area of the items rather than
the width/height of the container block. As per w3c/csswg-drafts#3653, child's margins
along with padding should contribute to the scrollable overflow area.

Test: imported/w3c/web-platform-tests/css/css-grid/alignment/grid-content-alignment-overflow-002.html
* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::clientLogicalBottomAfterRepositioning):
(WebCore::RenderBlock::computeOverflow):
* rendering/RenderBlock.h:
(WebCore::RenderBlock::logicalLeftForChild const):
* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::clientLogicalBottomAfterRepositioning): Deleted.
* rendering/RenderFlexibleBox.h:
* rendering/RenderGrid.cpp:
(WebCore::RenderGrid::layoutBlock):


Canonical link: https://commits.webkit.org/241711@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@282463 268f45cc-cd09-0410-ab3c-d52691b4dbfc
bertogg pushed a commit to Igalia/webkit that referenced this issue Sep 17, 2021
https://bugs.webkit.org/show_bug.cgi?id=226981

Reviewed by Javier Fernandez.

When computing the overflow area in the grid container, we need to obtain the result of ClientLogicalBottom
function. This value should be retrieved after computing the actual logical height rather than before.

For grid, width/height of the overflow rect should be the width/height of the grid area of the items rather than
the width/height of the container block. As per w3c/csswg-drafts#3653, child's margins
along with padding should contribute to the scrollable overflow area.

Test: imported/w3c/web-platform-tests/css/css-grid/alignment/grid-content-alignment-overflow-002.html
* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::clientLogicalBottomAfterRepositioning):
(WebCore::RenderBlock::computeOverflow):
* rendering/RenderBlock.h:
(WebCore::RenderBlock::logicalLeftForChild const):
* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::clientLogicalBottomAfterRepositioning): Deleted.
* rendering/RenderFlexibleBox.h:
* rendering/RenderGrid.cpp:
(WebCore::RenderGrid::layoutBlock):


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@282463 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants