WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 218794
Support aspect-ratio on non-replaced elements
https://bugs.webkit.org/show_bug.cgi?id=218794
Summary
Support aspect-ratio on non-replaced elements
Rob Buis
Reported
2020-11-11 01:56:02 PST
Support aspect-ratio on non-replaced elements
Attachments
Patch
(19.07 KB, patch)
2020-11-11 02:02 PST
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(19.28 KB, patch)
2020-11-11 07:03 PST
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(19.28 KB, patch)
2020-11-11 12:22 PST
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(19.32 KB, patch)
2020-11-16 05:20 PST
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(19.27 KB, patch)
2020-11-18 05:12 PST
,
Rob Buis
no flags
Details
Formatted Diff
Diff
Patch
(11.15 KB, patch)
2020-11-20 02:34 PST
,
Rob Buis
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Patch
(19.31 KB, patch)
2020-11-20 06:03 PST
,
Rob Buis
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Show Obsolete
(6)
View All
Add attachment
proposed patch, testcase, etc.
Rob Buis
Comment 1
2020-11-11 02:02:06 PST
Created
attachment 413801
[details]
Patch
Rob Buis
Comment 2
2020-11-11 07:03:10 PST
Created
attachment 413819
[details]
Patch
Rob Buis
Comment 3
2020-11-11 12:22:51 PST
Created
attachment 413853
[details]
Patch
Rob Buis
Comment 4
2020-11-16 05:20:09 PST
Created
attachment 414217
[details]
Patch
Simon Fraser (smfr)
Comment 5
2020-11-17 14:06:16 PST
Comment on
attachment 414217
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=414217&action=review
> Source/WebCore/rendering/RenderBox.cpp:2421 > +static LayoutUnit inlineSizeFromAspectRatio(LayoutUnit borderPaddingInlineSum, LayoutUnit borderPaddingBlockSum, LayoutUnit aspectRatio, BoxSizing boxSizing, LayoutUnit blockSize)
I think aspectRatio should be floating point.
> Source/WebCore/rendering/style/RenderStyle.h:507 > + LayoutUnit logicalAspectRatio() const
This return a floating point value, not a LayoutUnit.
Darin Adler
Comment 6
2020-11-17 15:25:16 PST
Comment on
attachment 414217
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=414217&action=review
> Source/WebCore/ChangeLog:11 > + Compute logical width/height for non-replaced elements > + taking aspect ratio into account. This is very basic > + support and does not consider things like transferred min/max > + block size.
Is it a problem to ship very basic support in web browsers? If so, do we need this behind a feature flag so we can turn it on when it’s more complete?
Radar WebKit Bug Importer
Comment 7
2020-11-18 01:56:17 PST
<
rdar://problem/71529770
>
Rob Buis
Comment 8
2020-11-18 05:12:16 PST
Created
attachment 414440
[details]
Patch
Rob Buis
Comment 9
2020-11-18 06:28:26 PST
Comment on
attachment 414217
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=414217&action=review
>> Source/WebCore/rendering/RenderBox.cpp:2421 >> +static LayoutUnit inlineSizeFromAspectRatio(LayoutUnit borderPaddingInlineSum, LayoutUnit borderPaddingBlockSum, LayoutUnit aspectRatio, BoxSizing boxSizing, LayoutUnit blockSize) > > I think aspectRatio should be floating point.
Done.
>> Source/WebCore/rendering/style/RenderStyle.h:507 >> + LayoutUnit logicalAspectRatio() const > > This return a floating point value, not a LayoutUnit.
Done.
Rob Buis
Comment 10
2020-11-18 07:44:53 PST
Comment on
attachment 414217
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=414217&action=review
>> Source/WebCore/ChangeLog:11 >> + block size. > > Is it a problem to ship very basic support in web browsers? If so, do we need this behind a feature flag so we can turn it on when it’s more complete?
I feel it is safer to let this mature a bit, at least the majority of aspect ratio tests should pass I think. Maybe it can be turned on in a few weeks if people feel it is safe to do so and there is demand for it.
Rob Buis
Comment 11
2020-11-20 02:34:18 PST
Created
attachment 414664
[details]
Patch
Rob Buis
Comment 12
2020-11-20 06:03:24 PST
Created
attachment 414671
[details]
Patch
Rob Buis
Comment 13
2020-12-07 14:17:58 PST
ping. Anything left for me to do for this bug? I hope I did not misunderstand Darin's
comment 6
? :)
Simon Fraser (smfr)
Comment 14
2020-12-08 10:26:27 PST
(In reply to Rob Buis from
comment #13
)
> ping. Anything left for me to do for this bug? I hope I did not > misunderstand Darin's
comment 6
? :)
He was asking whether it's OK to have this shipping in an incomplete form. If not, we should add a feature flag for it.
Rob Buis
Comment 15
2020-12-08 11:04:03 PST
(In reply to Simon Fraser (smfr) from
comment #14
)
> (In reply to Rob Buis from
comment #13
) > > ping. Anything left for me to do for this bug? I hope I did not > > misunderstand Darin's
comment 6
? :) > > He was asking whether it's OK to have this shipping in an incomplete form. > If not, we should add a feature flag for it.
Thanks! Just to be clear, at the moment aspect-ratio is an experimental preference. If I understand correctly, that does not equal to actually shipping? The feature is defined here: Source/WTF/Scripts/Preferences/WebPreferencesExperimental.yaml And parsing and handling aspect-ratio depends on that in CSSPropertyParser::parseSingleValue.
Darin Adler
Comment 16
2020-12-08 11:06:18 PST
(In reply to Rob Buis from
comment #15
)
> Thanks! Just to be clear, at the moment aspect-ratio is an experimental > preference. If I understand correctly, that does not equal to actually > shipping?
Great, that answers my question.
EWS
Comment 17
2020-12-08 12:30:22 PST
Committed
r270551
: <
https://trac.webkit.org/changeset/270551
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 414440
[details]
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug