[webkit-reviews] review granted: [Bug 209983] [css-flexbox] WebKit doesn't preserve aspect ratio when computing cross size of flexed images in auto-height flex container : [Attachment 414685] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Nov 20 14:01:22 PST 2020


Darin Adler <darin at apple.com> has granted Sergio Villar Senin
<svillar at igalia.com>'s request for review:
Bug 209983: [css-flexbox] WebKit doesn't preserve aspect ratio when computing
cross size of flexed images in auto-height flex container
https://bugs.webkit.org/show_bug.cgi?id=209983

Attachment 414685: Patch

https://bugs.webkit.org/attachment.cgi?id=414685&action=review




--- Comment #7 from Darin Adler <darin at apple.com> ---
Comment on attachment 414685
  --> https://bugs.webkit.org/attachment.cgi?id=414685
Patch

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

> Source/WebCore/ChangeLog:9
> +	   Aspect ratio was not preserved in the cross axis because WebKit was
stretching the items (as they're auto sized) whitout considering

Spelling error here in the word "without".

> Source/WebCore/rendering/RenderFlexibleBox.cpp:1555
> +    // Aspect ratio is properly handled by RenderReplaced during layout.
> +    if (child.isRenderReplaced() && child.hasAspectRatio())
> +	   return false;

This is frustratingly specific; I worry about a line of code like this that
affects only a single regression test. Are there other things we should be
testing?

> LayoutTests/ChangeLog:9
> +	   * css3/flexbox/flexitem.html: Updated expectations. Aspect ratio
must be preserved.

Do we need to keep this test around?

When our tests are duplicates of tests in Web Platform Tests, it would be good
for the WebKit project to delete the older WebKit tests, unless they still have
separate value. I presume in many cases the WPT test is a descendant of the
test that began as a separate WebKit test. It’s better for long term
maintainability to cut down the duplication when we can as we notice it.


More information about the webkit-reviews mailing list