Support aspect-ratio on non-replaced elements
Created attachment 413801 [details] Patch
Created attachment 413819 [details] Patch
Created attachment 413853 [details] Patch
Created attachment 414217 [details] Patch
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.
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?
<rdar://problem/71529770>
Created attachment 414440 [details] Patch
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.
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.
Created attachment 414664 [details] Patch
Created attachment 414671 [details] Patch
ping. Anything left for me to do for this bug? I hope I did not misunderstand Darin's comment 6? :)
(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.
(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.
(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.
Committed r270551: <https://trac.webkit.org/changeset/270551> All reviewed patches have been landed. Closing bug and clearing flags on attachment 414440 [details].