Bug 218384

Summary: Remove -webkit-aspect-ratio support
Product: WebKit Reporter: Rob Buis <rbuis>
Component: CSSAssignee: Rob Buis <rbuis>
Status: RESOLVED FIXED    
Severity: Normal CC: changseok, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, joepeck, kondapallykalyan, macpherson, menard, pdr, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=128629
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
ews-feeder: commit-queue-
Patch
none
Patch
none
Patch
none
Patch none

Description Rob Buis 2020-10-30 08:41:03 PDT 
Remove -webkit-aspect-ratio support since it is superseded
by aspect-ratio (https://www.w3.org/TR/css-sizing-4/).
Comment 1 Rob Buis 2020-10-30 08:43:27 PDT 
Created attachment 412747 [details]
Patch
Comment 2 Rob Buis 2020-10-30 12:04:41 PDT 
Created attachment 412784 [details]
Patch
Comment 3 Simon Fraser (smfr) 2020-10-30 13:05:01 PDT 
Comment on attachment 412784 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=412784&action=review

> Source/WebCore/rendering/RenderReplaced.cpp:-121
> -        float aspectRatio = m_intrinsicSize.aspectRatio();
> -        LayoutSize frameSize = size();
> -        float frameAspectRatio = frameSize.aspectRatio();
> -        if (frameAspectRatio < aspectRatio)
> -            setHeight(computeReplacedLogicalHeightRespectingMinMaxHeight(frameSize.height() * frameAspectRatio / aspectRatio));
> -        else if (frameAspectRatio > aspectRatio)
> -            setWidth(computeReplacedLogicalWidthRespectingMinMaxWidth(frameSize.width() * aspectRatio / frameAspectRatio, ComputePreferred));
> -    }

This was added in https://trac.webkit.org/changeset/164265/webkit so removing it is a behavior change, and we suspect that this might be used by (i)Books. I think we'll have to hold off on this change until we can determine that.
Comment 4 Rob Buis 2020-10-30 13:17:01 PDT 
(In reply to Simon Fraser (smfr) from comment #3)
> Comment on attachment 412784 [details]
> Patch
> 
> This was added in https://trac.webkit.org/changeset/164265/webkit so
> removing it is a behavior change, and we suspect that this might be used by
> (i)Books. I think we'll have to hold off on this change until we can
> determine that.

Thanks for the heads up! Would it make sense to restrict -webkit-aspect-ratio support to only parse from-intrinsic/from-dimensions and remove auto/specified as a first step?
Comment 5 Simon Fraser (smfr) 2020-10-30 13:29:34 PDT 
Yes I think that would be OK.
Comment 6 Rob Buis 2020-10-31 02:59:28 PDT 
Created attachment 412837 [details]
Patch
Comment 7 Rob Buis 2020-10-31 10:43:11 PDT 
Created attachment 412847 [details]
Patch
Comment 8 Rob Buis 2020-10-31 12:04:17 PDT 
Created attachment 412850 [details]
Patch
Comment 9 Rob Buis 2020-11-02 01:54:16 PST 
Created attachment 412895 [details]
Patch
Comment 10 Radar WebKit Bug Importer 2020-11-06 07:42:19 PST 
<rdar://problem/71118930>
Comment 11 Rob Buis 2020-11-14 01:38:39 PST 
Created attachment 414128 [details]
Patch
Comment 12 Rob Buis 2020-11-14 02:36:01 PST 
Created attachment 414130 [details]
Patch
Comment 13 EWS 2020-11-14 11:46:39 PST 
Committed r269820: <https://trac.webkit.org/changeset/269820>

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