Skip to content
Permalink
Browse files
Not computing image aspect ratios from width and height attributes fo…
…r lazy loaded images

https://bugs.webkit.org/show_bug.cgi?id=224197

Reviewed by Darin Adler.

LayoutTests/imported/w3c:

The test cases for error images and images without src in img-aspect-ratio.html are passed. This patch
doesn't change the behavior of the original aspect ratio case, so it's failed like before.

* web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/img-aspect-ratio-expected.txt:

Source/WebCore:

This patch supports error images and lazy loaded images (without src attribute) to compute
implicit aspect ratios from width and height attributes. Refactor the code a bit. Added
intrinsicAspectRatioFromWidthHeight() to compute aspect ratio from width and height attributes when
the object is allowed to which is decided by canMapWidthHeightToAspectRatio().
Remove `!downcast<RenderImage>(*this).cachedImage()` constraint, so that images without src attributes
is allowed. As to error images, compute the aspect ratio when the image shouldDisplayBrokenImageIcon().

* rendering/RenderImage.cpp:
(WebCore::RenderImage::canMapWidthHeightToAspectRatio const): To indicate that the object is allowed
to compute aspect ratio from width and height attributes.
(WebCore::RenderImage::computeIntrinsicRatioInformation const): When shouldDisplayBrokenImageIcon(),
try to compute the aspect ratio from attributes width and height.
* rendering/RenderImage.h:
* rendering/RenderReplaced.cpp:
(WebCore::RenderReplaced::intrinsicAspectRatioFromWidthHeight const): Compute the aspect ratio from attributes width and height.
(WebCore::RenderReplaced::computeIntrinsicRatioInformation const):
* rendering/RenderReplaced.h:
(WebCore::RenderReplaced::canMapWidthHeightToAspectRatio const): Ditto.


Canonical link: https://commits.webkit.org/236975@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@276521 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
cathiechen committed Apr 23, 2021
1 parent 30ba388 commit df1d439a74f773520a7ae327a2b87b917f2e5748
Showing 7 changed files with 74 additions and 18 deletions.
@@ -1,3 +1,15 @@
2021-04-23 Cathie Chen <cathiechen@igalia.com>

Not computing image aspect ratios from width and height attributes for lazy loaded images
https://bugs.webkit.org/show_bug.cgi?id=224197

Reviewed by Darin Adler.

The test cases for error images and images without src in img-aspect-ratio.html are passed. This patch
doesn't change the behavior of the original aspect ratio case, so it's failed like before.

* web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/img-aspect-ratio-expected.txt:

2021-04-23 Cathie Chen <cathiechen@igalia.com>

Import the update to "mapping attribute width and height to aspect ratio" tests from WPT
@@ -36,8 +36,8 @@ PASS Computed style test: input with {"type":"image","width":"10%","height":"20"
PASS Computed style test: input with {"type":"submit","width":"10%","height":"20"}
PASS Loaded images test: <img> without width height attributes
PASS Loaded images test: <img> with width and height attributes, but conflicting to the original aspect ratio
FAIL Loaded images test: <img> with width, height and empty src attributes assert_approx_equals: aspect-ratio should override intrinsic size of images that don't have any src. expected 0.8 +/- 0.001 but got Infinity
FAIL Loaded images test: Error image with width and height attributes assert_approx_equals: aspect-ratio should affect the size of error images. expected 0.8 +/- 0.001 but got 1
PASS Loaded images test: <img> with width, height and empty src attributes
PASS Loaded images test: Error image with width and height attributes
PASS Loaded images test: Error image with width, height and alt attributes
FAIL Loaded images test: <img> with width and height attributes, but not equal to the original aspect ratio assert_approx_equals: The original aspect ratio of blue.png expected 1.2547169811320755 +/- 0.001 but got 1.25

@@ -1,3 +1,29 @@
2021-04-23 Cathie Chen <cathiechen@igalia.com>

Not computing image aspect ratios from width and height attributes for lazy loaded images
https://bugs.webkit.org/show_bug.cgi?id=224197

Reviewed by Darin Adler.

This patch supports error images and lazy loaded images (without src attribute) to compute
implicit aspect ratios from width and height attributes. Refactor the code a bit. Added
intrinsicAspectRatioFromWidthHeight() to compute aspect ratio from width and height attributes when
the object is allowed to which is decided by canMapWidthHeightToAspectRatio().
Remove `!downcast<RenderImage>(*this).cachedImage()` constraint, so that images without src attributes
is allowed. As to error images, compute the aspect ratio when the image shouldDisplayBrokenImageIcon().

* rendering/RenderImage.cpp:
(WebCore::RenderImage::canMapWidthHeightToAspectRatio const): To indicate that the object is allowed
to compute aspect ratio from width and height attributes.
(WebCore::RenderImage::computeIntrinsicRatioInformation const): When shouldDisplayBrokenImageIcon(),
try to compute the aspect ratio from attributes width and height.
* rendering/RenderImage.h:
* rendering/RenderReplaced.cpp:
(WebCore::RenderReplaced::intrinsicAspectRatioFromWidthHeight const): Compute the aspect ratio from attributes width and height.
(WebCore::RenderReplaced::computeIntrinsicRatioInformation const):
* rendering/RenderReplaced.h:
(WebCore::RenderReplaced::canMapWidthHeightToAspectRatio const): Ditto.

2021-04-23 Michael Catanzaro <mcatanzaro@igalia.com>

Remove virtual function calls in GraphicsLayer destructors
@@ -842,6 +842,12 @@ void RenderImage::layoutShadowContent(const LayoutSize& oldSize)
clearChildNeedsLayout();
}

bool RenderImage::canMapWidthHeightToAspectRatio() const
{
// The aspectRatioOfImgFromWidthAndHeight only applies to <img>.
return is<HTMLImageElement>(element()) && !isShowingAltText();
}

void RenderImage::computeIntrinsicRatioInformation(FloatSize& intrinsicSize, double& intrinsicRatio) const
{
RenderReplaced::computeIntrinsicRatioInformation(intrinsicSize, intrinsicRatio);
@@ -858,6 +864,10 @@ void RenderImage::computeIntrinsicRatioInformation(FloatSize& intrinsicSize, dou

// Don't compute an intrinsic ratio to preserve historical WebKit behavior if we're painting alt text and/or a broken image.
if (shouldDisplayBrokenImageIcon()) {
if (!style().hasAspectRatio()) {
intrinsicRatio = intrinsicAspectRatioFromWidthHeight().valueOr(1);
return;
}
intrinsicRatio = 1;
return;
}
@@ -103,6 +103,8 @@ class RenderImage : public RenderReplaced {
imageChanged(imageResource().imagePtr());
}

bool canMapWidthHeightToAspectRatio() const override;

private:
const char* renderName() const override { return "RenderImage"; }

@@ -474,6 +474,23 @@ RoundedRect RenderReplaced::roundedContentBoxRect() const
borderLeft() + paddingLeft(), borderRight() + paddingRight());
}

Optional<double> RenderReplaced::intrinsicAspectRatioFromWidthHeight() const
{
if (!settings().aspectRatioOfImgFromWidthAndHeightEnabled())
return Optional<double>();

if (!canMapWidthHeightToAspectRatio())
return Optional<double>();

ASSERT(element());
double attributeWidth = parseValidHTMLFloatingPointNumber(element()->getAttribute(HTMLNames::widthAttr)).valueOr(0);
double attributeHeight = parseValidHTMLFloatingPointNumber(element()->getAttribute(HTMLNames::heightAttr)).valueOr(0);
if (attributeWidth > 0 && attributeHeight > 0)
return attributeWidth / attributeHeight;

return Optional<double>();
}

void RenderReplaced::computeIntrinsicRatioInformation(FloatSize& intrinsicSize, double& intrinsicRatio) const
{
// If there's an embeddedContentBox() of a remote, referenced document available, this code-path should never be used.
@@ -490,22 +507,7 @@ void RenderReplaced::computeIntrinsicRatioInformation(FloatSize& intrinsicSize,
return;

if (intrinsicSize.isEmpty()) {
if (!settings().aspectRatioOfImgFromWidthAndHeightEnabled())
return;

auto* node = element();
// The aspectRatioOfImgFromWidthAndHeight only applies to <img>.
if (!node || !is<HTMLImageElement>(*node) || !node->hasAttribute(HTMLNames::widthAttr) || !node->hasAttribute(HTMLNames::heightAttr))
return;

// We shouldn't override the aspect-ratio when the <img> element has an empty src attribute.
if (!is<RenderImage>(*this) || !downcast<RenderImage>(*this).cachedImage())
return;

double attributeWidth = parseValidHTMLFloatingPointNumber(node->getAttribute(HTMLNames::widthAttr)).valueOr(0);
double attributeHeight = parseValidHTMLFloatingPointNumber(node->getAttribute(HTMLNames::heightAttr)).valueOr(0);
if (attributeWidth > 0 && attributeHeight > 0)
intrinsicRatio = attributeWidth / attributeHeight;
intrinsicRatio = intrinsicAspectRatioFromWidthHeight().valueOr(0);
return;
}

@@ -58,6 +58,10 @@ class RenderReplaced : public RenderBox {

void computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidth, LayoutUnit& maxLogicalWidth) const final;

// This function determines if the object is allowed to compute aspect ratio from attributes width and height.
virtual bool canMapWidthHeightToAspectRatio() const { return false; }
Optional<double> intrinsicAspectRatioFromWidthHeight() const;

virtual LayoutUnit minimumReplacedHeight() const { return 0_lu; }

bool isSelected() const;

0 comments on commit df1d439

Please sign in to comment.