Skip to content

layout: Refactor box size computation #49748

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

Closed

Conversation

servo-wpt-sync
Copy link
Collaborator

#32853 has been adding quite a bit of boilerplate that keeps repeating in each layout logic, in order to correctly resolve sizing keywords.

This patch adds a new Sizes struct which holds the preferred, min and max sizing values for one axis, and unifies the logic to resolve the final size into there.

Reviewed in servo/servo#34671

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

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

The review process for this patch is being conducted in the Servo project.

Consider:
```html
<div style="position: relative; width: 50px; height: 50px; border: solid; margin: 5px">
  <div style="position: absolute; top: 0; bottom: 0; height: max-content">
    <canvas width="25" height="25" style="background: cyan; height: 100%"></canvas>
  </div>
</div>
```

In order to determine the inline min/max-content sizes, we need a
tentative block size as the input, which only takes extrinsic values
into account.

In this case `height: max-content` is intrinsic, so we were treating it
as `height: initial`, which would behave as a definite `height: stretch`.
Therefore, the canvas was able to resolve its percentage.

However, it seems weird to treat an explicitly intrinsic keyword in an
extrinsic way, and Blink doesn't do it. So now we treat the tentative
block size as indefinite, therefore the percentage behaves as auto.

This adds a new test, we were previously failing 6 subtests, now only 3.

Signed-off-by: Oriol Brufau <[email protected]>
@servo-wpt-sync
Copy link
Collaborator Author

👋 Downstream pull request (servo/servo#34671) no longer contains any upstreamable changes. Closing pull request without merging.

@servo-wpt-sync servo-wpt-sync deleted the servo_export_34671 branch December 18, 2024 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants