1909761 - Pass percentage basis when computing intrinsic sizes

Status: RESOLVED FIXED

(Core :: Layout, task)

Description

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

This tracks my work to refactor GetMinISize() and GetPrefISize() to accept a parameter of current frame's size so that it can be used as a percentage basis when computing child frame's intrinsic contribution.

I think this bug can at least fixes bug 1800991 and some tests on wpt.

Comment 1

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

The main entry point for computing the intrinsic inline size of a frame is nsIFrame::IntrinsicISize(), which is called when computing the computed inline size of ReflowInput.

nsLayoutUtils::IntrinsicForAxis() is an underlying helper method used for calculating children's intrinsic inline size contributions. GetPercentBSize() is a helper used in nsLayoutUtils::IntrinsicForAxis() to compute the percentage basis when there is a percentage size in the block axis. GetPercentBSize() traverses up the containing block chain to find the first ancestor containing block with a definite block size. However, it does not find a definite block size if the containing block's definite block size is established via inline size and aspect-ratio (see bug 1800991).

The main idea of the patch is to pass the frame's definite block size (with min block size and max block size constraints considered) into IntrinsicISize(). This bug also enables potential future improvements allowing flex or grid items to use stretched block size as a percentage basis for the content inside the items (e.g. bug 1742042).

The patch stack doesn't handle all the edge cases where IntrinsicISize(), GetPrefISize, and GetMinISize() need a percentage basis. For example, AddIntrinsicSizeOffset() requires more refactor to construct percentage basis easily. After that, we can fix bug 1231059 by removing GetPercentBSize().

Comment 2

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

Attachment: Bug 1909761 Part 1 - Improve lambda expressions in nsLayoutUtils::IntrinsicForAxis(). r?dholbert,#layout

Capitalize the name of lambda expressions to make them look like functions
rather than variables.

Rename and improve GetContentEdgeToBoxSizing since I feel the indentation
makes it difficult to read. Also, rename the local Maybe<LogicalSize> storing
the result of the lambda.

This patch doesn't change behavior.

Comment 3

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

Attachment: Bug 1909761 Part 2 - Create a helper struct IntrinsicSizeInput to aggregate needed data when computing intrinsic inline size. r?dholbert,#layout

This patch changes the signature to GetMinISize(), GetPrefISize(),
IntrinsicISize by adding a helper struct as a preparation. Then we can just
add more data such as a percentage basis to the struct without altering the
signature in the future.

When passing IntrinsicSizeInput struct down to another helper method, we
generally just pass the original one if the method is computing the intrinsic
size of our own or our anonymous children. If the method is computing our
children's intrinsic contribution, we'll need to create a brand new
IntrinsicSizeInput for our children.

Comment 4

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

Attachment: Bug 1909761 Part 3 - Use IntrinsicSizeInput for AddInlineMinISize() and AddInlinePrefISize(). r?dholbert,#layout

Another preparation patch in order to pass down a percentage basis when
computing intrinsic size.

Comment 5

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

Attachment: Bug 1909761 Part 4 - Construct a percentage basis when computing children's intrinsic inline size contributions. r?dholbert,#layout

A percentage basis is needed to resolve percentage block size when computing
children's intrinsic inline size contributions. This is necessary for a child or
descendants with a preferred aspect-ratio so that the block size can transfer
through the aspect-ratio to become an intrinsic inline size.

The change in nsFlexContainerFrame::ComputeIntrinsicISize() is necessary to
keep us passing
testing/web-platform/tests/css/css-flexbox/image-nested-within-definite-column-flexbox.html.

The change in nsPlaceholderFrame::AddFloatToIntrinsicISizeData() is necessary
to keep us passing
testing/web-platform/tests/css/css-sizing/intrinsic-percent-replaced-dynamic-010.html.

GetISizeInfo() in BasicTableLayoutStrategy.cpp is modified to pass table cell
frame's bsize as percentage basis. Otherwise,
layout/reftests/bugs/522632-1.html fails. This is our current behavior, but it
is bug 1542098.

Comment 6

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

Attachment: Bug 1909761 Part 5 - Add web-platform reftests for percentage resolution during intrinsic size computation. r?dholbert,#layout

All the tests pass in Google Chrome. They fail in the current Firefox Nightly
but will pass with the patches in this bug.

intrinsic-percent-replaced-{012,013,014}.html are adapted from the second
testcase in bug 1231059 comment 0.

intrinsic-percent-replaced-{015,016}.html are adapted from the testcase in bug
1800991 comment 2.

intrinsic-percent-replaced-017.html is adapted from bug 1910290 comment 7.

Comment 7

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

Attachment: Bug 1909761 Part 6 - Delete or improve obsolete comments in layout. r?dholbert,#layout

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4e7f22f43bb4
Part 1 - Improve lambda expressions in nsLayoutUtils::IntrinsicForAxis(). r=dholbert
https://hg.mozilla.org/integration/autoland/rev/8de556490289
Part 2 - Create a helper struct IntrinsicSizeInput to aggregate needed data when computing intrinsic inline size. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/38d1e5eda09d
Part 3 - Use IntrinsicSizeInput for AddInlineMinISize() and AddInlinePrefISize(). r=dholbert
https://hg.mozilla.org/integration/autoland/rev/3f48c106bff9
Part 4 - Construct a percentage basis when computing children's intrinsic inline size contributions. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/d2dcf159185b
Part 5 - Add web-platform reftests for percentage resolution during intrinsic size computation. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/1e9f47784a54
Part 6 - Delete or improve obsolete comments in layout. r=layout-reviewers,emilio

Comment 9

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

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

Comment 10

[Atila Butkovits]

Backed out for causing failures at test_contextmenu_rtl.xhtml.

Backout link: https://hg.mozilla.org/integration/autoland/rev/cf474d30f1b7f477897a8d5beeea49d748b546d0

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=1e9f47784a540079253157e46f81f0159d628448&selectedTaskRun=ZklC5NLzQKq9WG4YUnifog.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=472042803&repo=autoland&lineNumber=7673

Comment 11

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

Upstream PR was closed without merging

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d0f7ac5c6dc9
Part 1 - Improve lambda expressions in nsLayoutUtils::IntrinsicForAxis(). r=dholbert
https://hg.mozilla.org/integration/autoland/rev/9c9289d5bffc
Part 2 - Create a helper struct IntrinsicSizeInput to aggregate needed data when computing intrinsic inline size. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/f531438e13a4
Part 3 - Use IntrinsicSizeInput for AddInlineMinISize() and AddInlinePrefISize(). r=dholbert
https://hg.mozilla.org/integration/autoland/rev/b509d1a77c27
Part 4 - Construct a percentage basis when computing children's intrinsic inline size contributions. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/4646bfebde8f
Part 5 - Add web-platform reftests for percentage resolution during intrinsic size computation. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/d0336fee53a3
Part 6 - Delete or improve obsolete comments in layout. r=layout-reviewers,emilio

Comment 13

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/d0f7ac5c6dc9
https://hg.mozilla.org/mozilla-central/rev/9c9289d5bffc
https://hg.mozilla.org/mozilla-central/rev/f531438e13a4
https://hg.mozilla.org/mozilla-central/rev/b509d1a77c27
https://hg.mozilla.org/mozilla-central/rev/4646bfebde8f
https://hg.mozilla.org/mozilla-central/rev/d0336fee53a3

Comment 14

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

Upstream PR merged by moz-wptsync-bot