fix: Always set image alt text the same way (#15757)

Fixes #15756
vaadin · Jan 26, 2023 · c622a15 · c622a15
1 parent 5734f85
commit c622a15
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 17 deletions.
diff --git a/flow-html-components/src/main/java/com/vaadin/flow/component/html/Image.java b/flow-html-components/src/main/java/com/vaadin/flow/component/html/Image.java
@@ -36,12 +36,10 @@
 public class Image extends HtmlContainer
         implements ClickNotifier<Image>, HasAriaLabel {
 
+    private static final String ALT_ATTRIBUTE = "alt";
     private static final PropertyDescriptor<String, String> srcDescriptor = PropertyDescriptors
             .attributeWithDefault("src", "");
 
-    private static final PropertyDescriptor<String, Optional<String>> altDescriptor = PropertyDescriptors
-            .optionalAttributeWithDefault("alt", "");
-
     /**
      * Creates a new empty image.
      */
@@ -65,7 +63,7 @@ public Image() {
      */
     public Image(String src, String alt) {
         setSrc(src);
-        getElement().setProperty("alt", alt);
+        setAlt(alt);
     }
 
     /**
@@ -84,7 +82,7 @@ public Image(String src, String alt) {
      */
     public Image(AbstractStreamResource src, String alt) {
         setSrc(src);
-        getElement().setProperty("alt", alt);
+        setAlt(alt);
     }
 
     /**
@@ -123,7 +121,12 @@ public void setSrc(AbstractStreamResource src) {
      *            the alternate text
      */
     public void setAlt(String alt) {
-        set(altDescriptor, alt);
+        if (alt == null) {
+            getElement().removeAttribute(ALT_ATTRIBUTE);
+        } else {
+            // Also an empty string should be set as alt
+            getElement().setAttribute(ALT_ATTRIBUTE, alt);
+        }
     }
 
     /**
@@ -133,6 +136,6 @@ public void setAlt(String alt) {
      *         text has been set
      */
     public Optional<String> getAlt() {
-        return get(altDescriptor);
+        return Optional.ofNullable(getElement().getAttribute(ALT_ATTRIBUTE));
     }
 }
diff --git a/flow-html-components/src/test/java/com/vaadin/flow/component/html/ImageTest.java b/flow-html-components/src/test/java/com/vaadin/flow/component/html/ImageTest.java
@@ -15,6 +15,8 @@
  */
 package com.vaadin.flow.component.html;
 
+import java.util.Optional;
+
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -24,7 +26,6 @@ public class ImageTest extends ComponentTest {
 
     @Override
     protected void addProperties() {
-        addOptionalStringProperty("alt");
         addStringProperty("src", "");
     }
 
@@ -35,15 +36,12 @@ public void testHasAriaLabelIsImplemented() {
     }
 
     @Test
-    public void setEmptyAltInConstructor_altPropertExists() {
+    public void emptyAltKeepsAttribute() {
         Image img = new Image("test.png", "");
-        Assert.assertTrue(
-                "'alt' property should have been retained with constructor",
-                img.getElement().hasProperty("alt"));
-
-        img.setAlt("");
-
-        Assert.assertTrue("'alt' property should have been cleared with setAlt",
-                img.getElement().hasProperty("alt"));
+        Assert.assertEquals("", img.getAlt().get());
+        Assert.assertTrue(img.getElement().hasAttribute("alt"));
+        img.setAlt(null);
+        Assert.assertEquals(Optional.empty(), img.getAlt());
+        Assert.assertFalse(img.getElement().hasAttribute("alt"));
     }
 }