Closed Bug 1690422 Opened 4 years ago Closed 4 years ago

Incorrect calculation of the inline min/max content contribution in grid tracking algorithm when using aspect-ratio property in grid item

Categories

(Core :: Layout: Grid, defect, P2)

defect

Tracking

()

RESOLVED FIXED
87 Branch
Tracking Status
firefox87 --- fixed

People

(Reporter: boris, Assigned: boris)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

When using aspect-ratio on grid items, we calculate the size of this grid correctly, but its grid tracking size doesn't take this aspect-ratio property into account.
e.g.

.grid-container {
  display: grid;
  grid-template-columns: auto;
  width: max-content;
  height: 100px;
  gap: 1rem;
}
.grid-item {
  aspect-ratio: 1/1;
  height: 100px;
  width: auto;  // the final width should be 100px (= 100px * 1 / 1).
  min-width: 0px;
}

<div class="grid-container">
  <div class="grid-item">
    <div style="width:50px; height:50px;"></div>
  </div>
</div>

The max content contribution of this grid item should be 100px. However, the actual result is 50px because IntrinsicForAxis() and AddIntrinsicSizeOffset() don't use aspect-ratio to replace the min-content and max-content values. This is a follow-up bug of Bug 1646100 for Grid layout.

Besides, the usage of AspectRatio in IntrinsicForAxis() should take box-sizing into account. We always use content-box for now, in all cases. This is not correct. This is a follow-up bug for Bug 1683424.

Status: NEW → ASSIGNED
Depends on: 1667501
Attachment #9201565 - Attachment description: Bug 1690422 - Take box-sizing into account when computing the intrinsic size. → Bug 1690422 - Take box-sizing into account when computing the intrinsic size with the preferred aspect ratio.

This is simliar with the previous patch, but we use
min-content/max-content for the inner div (i.e. child frame).

Perhaps we could remove the simliar implementation from
ComputeISizeValue(). However, I still keep it because it can avoid some
extra work when resolving {min|max}-content with aspect-ratio when
reflowing.

Blocks: 1693200
Attachment #9201932 - Attachment description: Bug 1690422 - Resolve {min|max}-content contrinbution with the preferred aspect ratio and the definie block size. → Bug 1690422 - Resolve {min|max}-content contribution with the preferred aspect ratio and the definite block size.
Pushed by bchiou@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/373fe3a0806a Take box-sizing into account when computing the intrinsic size with the preferred aspect ratio. r=TYLin,mats https://hg.mozilla.org/integration/autoland/rev/d5975841c326 Resolve {min|max}-content contribution with the preferred aspect ratio and the definite block size. r=TYLin
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/27655 for changes under testing/web-platform/tests
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 87 Branch
Upstream PR merged by moz-wptsync-bot
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: