Fixes img without src but with title doesn't respect CSS dimensions.

Prior to this CL, img with CSS height, width and a title attribute is treated as inline text without the CSS dimensions applied. The reason is that title attribute is treated as a backup for alt text [1]. Thus, this prevents the img to be treated as replaced element with intrinsic dimensions [2]:represents-2, but it's treated as non-replaced text [2]:represents-3 where the CSS dimensions are ignored. The fix is instead of checking the HTMLElement::AltText(), check only the presence of alt attribute when determining if the element should be treated as a replaced element. I had considered making the fix in [1] so that AltText() doesn't fallback to title. However, I decided against it to minimized the impact of this change and only fix it when the img doesn't represent an image and defaults to its fallback behavior. [1] https://source.chromium.org/chromium/chromium/src/+/master:third_party/blink/renderer/core/html/html_image_element.cc;l=321 [2] https://html.spec.whatwg.org/multipage/rendering.html#images-3 Bug: 958250 Change-Id: I78b3d84d8237b72505fdc5389702d119d61ae405 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2239473 Commit-Queue: Yu Han <[email protected]> Reviewed-by: Stephen Chenney <[email protected]> Reviewed-by: Fredrik Söderquist <[email protected]> Reviewed-by: Mason Freed <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#779100} Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src Cr-Mirrored-Commit: ac9f839eca574f5195d2282ed3e83d1883a8ddae
mjfroman · Jun 17, 2020 · a440f57 · a440f57
1 parent 542cb6b
commit a440f57
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 1 deletion.
diff --git a/blink/renderer/core/html/html_image_fallback_helper.cc b/blink/renderer/core/html/html_image_fallback_helper.cc
@@ -25,8 +25,15 @@ static bool NoImageSourceSpecified(const Element& element) {
   return element.FastGetAttribute(html_names::kSrcAttr).IsEmpty();
 }
 
+static bool ImageHasNoAltAttribute(const Element& element) {
+  return element.FastGetAttribute(html_names::kAltAttr).IsNull();
+}
+
 static bool ElementRepresentsNothing(const Element& element) {
   const auto& html_element = To<HTMLElement>(element);
+  // We source fallback content/alternative text from more than just the 'alt'
+  // attribute, so consider the element to represent text in those cases as
+  // well.
   bool alt_is_set = !html_element.AltText().IsNull();
   bool alt_is_empty = alt_is_set && html_element.AltText().IsEmpty();
   bool src_is_set = !NoImageSourceSpecified(element);
@@ -170,7 +177,7 @@ void HTMLImageFallbackHelper::CustomStyleForAltText(Element& element,
   bool image_has_intrinsic_dimensions =
       new_style.Width().IsSpecifiedOrIntrinsic() &&
       new_style.Height().IsSpecifiedOrIntrinsic();
-  bool image_has_no_alt_attribute = To<HTMLElement>(element).AltText().IsNull();
+  bool image_has_no_alt_attribute = ImageHasNoAltAttribute(element);
   bool treat_as_replaced =
       image_has_intrinsic_dimensions &&
       (element.GetDocument().InQuirksMode() || image_has_no_alt_attribute);

diff --git a/...replaced-elements/attributes-for-embedded-content-and-images/img-title-only-w-sizing.html b/...replaced-elements/attributes-for-embedded-content-and-images/img-title-only-w-sizing.html
@@ -0,0 +1,25 @@
+<!doctype html>
+<title>Images with only title should be treated as a replaced element</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<link rel="author" href="mailto:[email protected]" title="Yu Han">
+<link rel="help" href="https://crbug.com/958250">
+<link ref="help" href="https://html.spec.whatwg.org/multipage/rendering.html#images-3">
+<style>
+  .title-only {
+    width: 100px;
+    height: 150px;
+  }
+</style>
+<img class="title-only" title="title">
+<img width="100" height="150px" title="title">
+<script>
+async_test(t => {
+  onload = t.step_func_done(function() {
+    for (const img of document.querySelectorAll("img")) {
+      assert_equals(img.offsetWidth, 100, `width: ${img.outerHTML}`);
+      assert_equals(img.offsetHeight, 150, `height: ${img.outerHTML}`);
+    }
+  });
+});
+</script>