1917056 - Move the special logic for table frame out of IntrinsicForAxis()

Status: RESOLVED FIXED

(Core :: Layout, task)

Description

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

We have a special logic for table frame in IntrinsicForAxis() to establish its min intrinsic inline size as the lower bound. I'd like the move it into nsTableWrapperFrame::IntrinsicISize(), and simplify the code around it.

Comment 1

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

Attachment: Bug 1917056 Part 1 - Remove DEBUG_INTRINSIC_WIDTH. r?#layout

These debug information can be easily reintroduced if needed, or inspected via
rr or pernosco. I find them to be a distraction while working on the code.

Comment 2

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

Attachment: Bug 1917056 Part 2 - Move the special logic for table frame out of IntrinsicForAxis(). r?#layout

In the old logic, the table's content inline size is stored in the min
variable, and passed into AddIntrinsicSizeOffset() to account for margin,
border, and padding.

We don't seem to have test coverage for this behavior, so I added some WPTs in
the next part.

This patch shouldn't change behavior.

Comment 3

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

Attachment: Bug 1917056 Part 3 - Add WPTs for table intrinsic size. r?#layout

table-intrinsic-size-001.html and table-intrinsic-size-002.html test
inline-size and max-inline-size on the table elements, respectively. They
currently pass on Nightly, but will fail if the special logic in
IntrinsicForAxis() is removed. Since we lacked test coverage for this scenario,
I added these tests.

table-intrinsic-size-003.html and table-intrinsic-size-004.html tests the
intrinsic size when table has orthogonal writing mode. They fail due to
bug 1310551.

Comment 4

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

Attachment: Bug 1917056 Part 4 - Remove aContentMinSize parameter for AddIntrinsicSizeOffset(). r?#layout

In AddIntrinsicSizeOffset(), aContentMinSizeis used only to accept the table's min intrinsic size fromIntrinsicForAxis(). After removing the logic from IntrinsicForAxis() in part 2, we no longer need the aContentMinSize`
parameter.

Also, this patch improves the readability of the code that initializes result,
contentBoxToBoxSizingDiff, and boxSizingToMarginDiff (renamed from
coordOutsideSize).

This patch doesn't change behavior.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/09c40ef62a2b
Part 1 - Remove DEBUG_INTRINSIC_WIDTH. r=layout-reviewers,emilio
https://hg.mozilla.org/integration/autoland/rev/ec7ee6605016
Part 2 - Move the special logic for table frame out of IntrinsicForAxis(). r=layout-reviewers,emilio
https://hg.mozilla.org/integration/autoland/rev/45b1d3abcc09
Part 3 - Add WPTs for table intrinsic size. r=layout-reviewers,emilio
https://hg.mozilla.org/integration/autoland/rev/7a9f35f44a91
Part 4 - Remove aContentMinSize parameter for AddIntrinsicSizeOffset(). r=layout-reviewers,emilio

Comment 6

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/48006 for changes under testing/web-platform/tests

Comment 7

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/09c40ef62a2b
https://hg.mozilla.org/mozilla-central/rev/ec7ee6605016
https://hg.mozilla.org/mozilla-central/rev/45b1d3abcc09
https://hg.mozilla.org/mozilla-central/rev/7a9f35f44a91

Comment 8

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 9

[Ian Kilpatrick [:iank]]

:TYLin - The WPT:
http://wpt.live/css/css-tables/table-intrinsic-size-003.html

Contins an error, it should be "height: 80px" instead of "100px".

The height of the table is "border + content" which is "10px + 10px + 100px" = "120px"
I'll submit a fix.

Comment 10

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

Hi Ian,

I meant to use "block-size:100px" instead of "height: 100px". I can also submit a fix in wpt upstream if you haven't got to it.

David also leaves a message about this.
https://github.com/web-platform-tests/wpt/commit/ff8ec78f3504a09f25b9a83fdc42b0db94332605#diff-a1fe52c68868afc60d9cdb78a1e9ec810daa4b3580132ab4c6b89f41e9173169R23

Comment 11

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

I posted a fix in https://github.com/web-platform-tests/wpt/pull/48029