Bug 225015

Summary: Block sizing is miscalculated for elements with a preferred aspect-ratio when padding is applied, and content can overflow.
Product: WebKit Reporter: Jen Simmons <jensimmons>
Component: Layout and RenderingAssignee: Rob Buis <rbuis>
Status: RESOLVED FIXED    
Severity: Blocker CC: bfulgham, changseok, clopez, darin, dino, esprehn+autocc, ews-watchlist, glenn, graouts, graouts, kondapallykalyan, pdr, rbuis, simon.fraser, webkit-bug-importer, youennf, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 47738    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Jen Simmons 2021-04-24 08:15:49 PDT 
When `aspect-ratio` is applied to a box with content, the box should prevent overflow by growing to be tall enough to fit the content, plus space for whatever margins or padding are involved. 

From the spec: 
>In order to avoid unintentional overflow, the automatic minimum size in the ratio-dependent axis of a box with a preferred aspect ratio that is neither a replaced element nor a scroll container is its min-content size clamped from above by its maximum size.
https://www.w3.org/TR/css-sizing-4/#aspect-ratio-minimum

This works in the WebKit implementation, unless there is padding applied to the box. It looks like the padding is being subtracted from the height, instead of added, or something similarly weird.

Example: https://labs.jensimmons.com/2021/aspect-ratio/04-bug.html
Comment 1 Rob Buis 2021-04-24 10:54:51 PDT 
Created attachment 426984 [details]
Patch
Comment 2 Rob Buis 2021-04-25 02:11:24 PDT 
Created attachment 426997 [details]
Patch
Comment 3 Rob Buis 2021-04-25 03:43:16 PDT 
Created attachment 427000 [details]
Patch
Comment 4 Rob Buis 2021-04-25 11:05:06 PDT 
Created attachment 427006 [details]
Patch
Comment 5 EWS Watchlist 2021-04-25 11:06:16 PDT 
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Comment 6 Rob Buis 2021-04-28 10:03:54 PDT 
Created attachment 427273 [details]
Patch
Comment 7 Radar WebKit Bug Importer 2021-04-28 11:42:20 PDT 
<rdar://problem/77276415>
Comment 8 Rob Buis 2021-04-28 11:53:35 PDT 
To be clear, the first patches had a WPT test included that I developed. Today I discovered that there is already a very similar WPT test available, so the latest patch imports that one.
Comment 9 Darin Adler 2021-04-28 12:05:36 PDT 
Comment on attachment 427273 [details]
Patch

Looks OK.

But there are multiple code changes and only one test case. Seems like we need more test cases. When I see branches in code I assume each needs a different test, so this seems to require at *least* 3 tests, but maybe more if we want to cover not changing the cases we don’t want to.
Comment 10 EWS 2021-04-28 18:00:35 PDT 
Committed r276745 (237145@main): <https://commits.webkit.org/237145@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 427273 [details].