Changeset 276521 in webkit

Timestamp:

Apr 23, 2021 3:00:12 PM (15 months ago)

Author:

cathiechen

Message:

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.

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.

Location:

trunk

Files:

• LayoutTests/imported/w3c/ChangeLog (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/img-aspect-ratio-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderImage.cpp (modified) (2 diffs)
• Source/WebCore/rendering/RenderImage.h (modified) (1 diff)
• Source/WebCore/rendering/RenderReplaced.cpp (modified) (2 diffs)
• Source/WebCore/rendering/RenderReplaced.h (modified) (1 diff)

trunk/LayoutTests/imported/w3c/ChangeLog

@@ +1 @@
+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>
 

trunk/LayoutTests/imported/w3c/web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/img-aspect-ratio-expected.txt

@@ -37 +37 @@
 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

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/rendering/RenderImage.cpp

@@ -843 +843 @@
 }
 
+bool RenderImage::canMapWidthHeightToAspectRatio() const
+{
+    // The aspectRatioOfImgFromWidthAndHeight only applies to <img>.
+    return is<HTMLImageElement>(element()) && !isShowingAltText();
+}
+
 void RenderImage::computeIntrinsicRatioInformation(FloatSize& intrinsicSize, double& intrinsicRatio) const
 {
@@ -859 +865 @@
     // 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;

trunk/Source/WebCore/rendering/RenderImage.h

@@ -104 +104 @@
     }
 
+    bool canMapWidthHeightToAspectRatio() const override;
+
 private:
     const char* renderName() const override { return "RenderImage"; }

trunk/Source/WebCore/rendering/RenderReplaced.cpp

@@ -475 +475 @@
 }
 
+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
 {
@@ -491 +508 @@
 
     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;
     }

trunk/Source/WebCore/rendering/RenderReplaced.h

@@ -59 +59 @@
     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; }