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

[LayoutNG] Mild cleanup and bugfixing of end margin strut handling. #9430

Merged
merged 1 commit into from Feb 8, 2018

Conversation

Projects
None yet
4 participants
@chromium-wpt-export-bot
Copy link
Collaborator

chromium-wpt-export-bot commented Feb 7, 2018

Also some more documentation.

Tried to make the various situations a bit clearer, with documentation
and code. And shared code for layout abortion.

The code used to call MaybeUpdateFragmentBfcOffset() quite
unconditionally based on an end offset inside the block. I found it
confusing to use an end offset as a BFC start offset. The code was
correct, though, since MaybeUpdateFragmentBfcOffset() wouldn't do
anything if the BFC offset was already known (which would be the case
if had children with actual size, for instance, making for a strange
BFC offset in that case). We'll now only call
MaybeUpdateFragmentBfcOffset() if BFC offset is unknown.
That's the only time end_bfc_block_offset can actually be used as a BFC
start offset.

Fixed one bug, though: A block with explicit height:0 ate the input
margin, rather than letting it collapse through and propagate to
subsequent layout input nodes. The intention of the code was just to get
rid of the last child margin, since height was non-auto. Now we check if
we have a BFC offset before doing so. If we have BFC offset, it means
that no input margins should collapse through. And if we DON'T have a
BFC offset, leave the margins alone for subsequent layout input nodes.
Otherwise we'd just lose them.

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: Iec8cea316015c6d36ef6bab9acbe826513222b1d
Reviewed-on: https://chromium-review.googlesource.com/907549
Commit-Queue: Morten Stenshorne mstensho@chromium.org
Commit-Queue: Emil A Eklund eae@chromium.org
Reviewed-by: Emil A Eklund eae@chromium.org
Reviewed-by: Ian Kilpatrick ikilpatrick@chromium.org
Cr-Commit-Position: refs/heads/master@{#535273}

@wpt-pr-bot
Copy link
Collaborator

wpt-pr-bot left a comment

Already reviewed downstream.

@w3c-bots

This comment has been minimized.

Copy link

w3c-bots commented Feb 7, 2018

Build PASSED

Started: 2018-02-08 03:08:06
Finished: 2018-02-08 03:12:19

View more information about this build on:

@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-907549 branch from 50651bb to b1ba629 Feb 7, 2018

[LayoutNG] Mild cleanup and bugfixing of end margin strut handling.
Also some more documentation.

Tried to make the various situations a bit clearer, with documentation
and code. And shared code for layout abortion.

The code used to call MaybeUpdateFragmentBfcOffset() quite
unconditionally based on an *end* offset inside the block. I found it
confusing to use an end offset as a BFC start offset. The code was
correct, though, since MaybeUpdateFragmentBfcOffset() wouldn't do
anything if the BFC offset was already known (which would be the case
if had children with actual size, for instance, making for a strange
BFC offset in that case). We'll now only call
MaybeUpdateFragmentBfcOffset() if BFC offset is unknown.
That's the only time end_bfc_block_offset can actually be used as a BFC
start offset.

Fixed one bug, though: A block with explicit height:0 ate the input
margin, rather than letting it collapse through and propagate to
subsequent layout input nodes. The intention of the code was just to get
rid of the last child margin, since height was non-auto. Now we check if
we have a BFC offset before doing so. If we have BFC offset, it means
that no input margins should collapse through. And if we DON'T have a
BFC offset, leave the margins alone for subsequent layout input nodes.
Otherwise we'd just lose them.

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: Iec8cea316015c6d36ef6bab9acbe826513222b1d
Reviewed-on: https://chromium-review.googlesource.com/907549
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535273}

@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-907549 branch from b1ba629 to c243c07 Feb 8, 2018

@chromium-wpt-export-bot chromium-wpt-export-bot merged commit 19b995f into master Feb 8, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@chromium-wpt-export-bot chromium-wpt-export-bot deleted the chromium-export-cl-907549 branch Feb 8, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.