ComponentFeatureImageResolver: Switch to image from page by default i…

…f no image reference is given in image/teaser resource.
wcm-io · Sep 4, 2023 · 02d3cce · 02d3cce
1 parent 7f6e265
commit 02d3cce
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 30 deletions.
diff --git a/bundles/core/src/main/java/io/wcm/wcm/core/components/impl/models/v2/ImageV2Impl.java b/bundles/core/src/main/java/io/wcm/wcm/core/components/impl/models/v2/ImageV2Impl.java
@@ -68,7 +68,7 @@ public class ImageV2Impl extends ImageV3Impl implements LinkMixin {
   public static final String RESOURCE_TYPE = "wcm-io/wcm/core/components/image/v2/image";
 
   @Override
-  protected Media buildMedia(boolean altFromAsset, boolean imageFromPageImage) {
+  protected Media buildMedia(boolean altFromAsset) {
     return HandlerUnwrapper.get(mediaHandler, resource)
         // disable dynamic media support as it is not compatible with the "src-pattern" concept
         .dynamicMediaDisabled(true)

diff --git a/bundles/core/src/main/java/io/wcm/wcm/core/components/impl/models/v3/ImageV3Impl.java b/bundles/core/src/main/java/io/wcm/wcm/core/components/impl/models/v3/ImageV3Impl.java
@@ -136,7 +136,6 @@ private void activate() {
     lazyThreshold = currentStyle.get(PN_DESIGN_LAZY_THRESHOLD, 0);
     isDecorative = properties.get(PN_IS_DECORATIVE, currentStyle.get(PN_IS_DECORATIVE, false));
     boolean altFromAsset = properties.get(PN_ALT_VALUE_FROM_DAM, currentStyle.get(PN_ALT_VALUE_FROM_DAM, true));
-    boolean imageFromPageImage = properties.get(PN_IMAGE_FROM_PAGE_IMAGE, true);
 
     // resolve link - decorative images have no link and no alt text by definition
     if (isDecorative) {
@@ -147,7 +146,7 @@ private void activate() {
     }
 
     // resolve media and properties from DAM asset
-    media = buildMedia(altFromAsset, imageFromPageImage);
+    media = buildMedia(altFromAsset);
 
     if (media.isValid() && !media.getRendition().isImage()) {
       // no image asset selected (cannot be rendered) - set to invalid
@@ -162,11 +161,10 @@ private void activate() {
 
   }
 
-  protected Media buildMedia(boolean altFromAsset, boolean imageFromPageImage) {
+  protected Media buildMedia(boolean altFromAsset) {
     ComponentFeatureImageResolver imageResolver = new ComponentFeatureImageResolver(resource, getCurrentPage(), currentStyle, mediaHandler)
         .targetPage(getCurrentPage())
         .altValueFromDam(altFromAsset)
-        .imageFromPageImage(imageFromPageImage)
         .mediaHandlerProperty(PROP_CSS_CLASS, "cmp-image__image")
         .mediaHandlerProperty("itemprop", "contentUrl");
     String imageTitle = title;

diff --git a/...ore/src/main/java/io/wcm/wcm/core/components/impl/util/ComponentFeatureImageResolver.java b/...ore/src/main/java/io/wcm/wcm/core/components/impl/util/ComponentFeatureImageResolver.java
@@ -42,6 +42,7 @@
 import io.wcm.handler.media.Media;
 import io.wcm.handler.media.MediaBuilder;
 import io.wcm.handler.media.MediaHandler;
+import io.wcm.handler.media.MediaInvalidReason;
 
 /**
  * Resolves images and alt. texts for components either from the component resource,
@@ -128,7 +129,16 @@ public ComponentFeatureImageResolver altValueFromDam(boolean value) {
    * @return Media
    */
   public @NotNull Media buildMedia() {
-    Media media;
+    Media media = mediaHandler.invalid();
+
+    if (!imageFromPageImage) {
+      // image from resource properties
+      media = buildMedia(componentResource);
+      if (!media.isValid() && media.getMediaInvalidReason() == MediaInvalidReason.MEDIA_REFERENCE_MISSING) {
+        // fallback to image from page if no reference was given
+        imageFromPageImage = true;
+      }
+    }
 
     if (imageFromPageImage) {
       // try to get feature image from target page
@@ -145,10 +155,6 @@ public ComponentFeatureImageResolver altValueFromDam(boolean value) {
         media = buildMedia(wrapFeatureImageResource(featuredImageResource));
       }
     }
-    else {
-      // image from teaser resource
-      media = buildMedia(componentResource);
-    }
 
     return media;
   }

diff --git a/bundles/core/src/test/java/io/wcm/wcm/core/components/impl/models/v3/ImageV3ImplTest.java b/bundles/core/src/test/java/io/wcm/wcm/core/components/impl/models/v3/ImageV3ImplTest.java
@@ -23,7 +23,6 @@
 import static com.adobe.cq.wcm.core.components.models.Image.PN_DESIGN_LAZY_LOADING_ENABLED;
 import static com.adobe.cq.wcm.core.components.models.Image.PN_DESIGN_LAZY_THRESHOLD;
 import static com.adobe.cq.wcm.core.components.models.Image.PN_DISPLAY_POPUP_TITLE;
-import static com.adobe.cq.wcm.core.components.models.Image.PN_IMAGE_FROM_PAGE_IMAGE;
 import static com.adobe.cq.wcm.core.components.models.Image.PN_IS_DECORATIVE;
 import static com.adobe.cq.wcm.core.components.models.Image.PN_MAP;
 import static com.adobe.cq.wcm.core.components.models.Image.PN_TITLE_VALUE_FROM_DAM;
@@ -159,7 +158,6 @@ void testInvalidAssetReference() {
   void testWithAssetImage() {
     context.currentResource(context.create().resource(page, "image",
         PROPERTY_RESOURCE_TYPE, RESOURCE_TYPE,
-        PN_IMAGE_FROM_PAGE_IMAGE, false,
         PN_MEDIA_REF_STANDARD, asset.getPath(),
         JCR_TITLE, "Resource Title",
         PN_ALT, "Resource Alt"));
@@ -190,7 +188,6 @@ void testWithAssetImage_SVG() {
 
     context.currentResource(context.create().resource(page, "image",
         PROPERTY_RESOURCE_TYPE, RESOURCE_TYPE,
-        PN_IMAGE_FROM_PAGE_IMAGE, false,
         PN_MEDIA_REF_STANDARD, svgAsset.getPath()));
 
     Image underTest = AdaptTo.notNull(context.request(), Image.class);
@@ -238,7 +235,6 @@ void testWithAssetImageFromPage() {
   void testWithUploadedImage() {
     Resource imageResource = context.create().resource(page, "image",
         PROPERTY_RESOURCE_TYPE, RESOURCE_TYPE,
-        PN_IMAGE_FROM_PAGE_IMAGE, false,
         NN_MEDIA_INLINE_STANDARD + "Name", "file1.png");
     context.load().binaryFile("/files/test.png", imageResource.getPath() + "/" + NN_MEDIA_INLINE_STANDARD, ContentType.PNG);
     context.currentResource(imageResource);
@@ -270,7 +266,6 @@ void testWithImageAndLink() {
 
     context.currentResource(context.create().resource(page, "image",
         PROPERTY_RESOURCE_TYPE, RESOURCE_TYPE,
-        PN_IMAGE_FROM_PAGE_IMAGE, false,
         PN_MEDIA_REF_STANDARD, asset.getPath(),
         PN_LINK_TITLE, ExternalLinkType.ID,
         PN_LINK_EXTERNAL_REF, "http://myhost"));
@@ -299,7 +294,6 @@ void testWithImageAndLink() {
   void testWithImageAndLink_Decorative() {
     context.currentResource(context.create().resource(page, "image",
         PROPERTY_RESOURCE_TYPE, RESOURCE_TYPE,
-        PN_IMAGE_FROM_PAGE_IMAGE, false,
         PN_MEDIA_REF_STANDARD, asset.getPath(),
         PN_LINK_TITLE, ExternalLinkType.ID,
         PN_LINK_EXTERNAL_REF, "http://myhost",
@@ -324,7 +318,6 @@ void testWithImageAndLink_Decorative_ContentPolicy() {
 
     context.currentResource(context.create().resource(page, "image",
         PROPERTY_RESOURCE_TYPE, RESOURCE_TYPE,
-        PN_IMAGE_FROM_PAGE_IMAGE, false,
         PN_MEDIA_REF_STANDARD, asset.getPath(),
         PN_LINK_TITLE, ExternalLinkType.ID,
         PN_LINK_EXTERNAL_REF, "http://myhost"));
@@ -343,7 +336,6 @@ void testWithImageAndLink_Decorative_ContentPolicy() {
   void testWithImage_TitleAltNotFormAsset() {
     context.currentResource(context.create().resource(page, "image",
         PROPERTY_RESOURCE_TYPE, RESOURCE_TYPE,
-        PN_IMAGE_FROM_PAGE_IMAGE, false,
         PN_MEDIA_REF_STANDARD, asset.getPath(),
         JCR_TITLE, "Resource Title",
         PN_ALT, "Resource Alt",
@@ -364,7 +356,6 @@ void testWithImage_TitleAltNotFormAsset_ContentPolicy() {
 
     context.currentResource(context.create().resource(page, "image",
         PROPERTY_RESOURCE_TYPE, RESOURCE_TYPE,
-        PN_IMAGE_FROM_PAGE_IMAGE, false,
         PN_MEDIA_REF_STANDARD, asset.getPath(),
         JCR_TITLE, "Resource Title",
         PN_ALT, "Resource Alt"));
@@ -381,7 +372,6 @@ void testWithNoImageAsset() {
 
     context.currentResource(context.create().resource(page, "image",
         PROPERTY_RESOURCE_TYPE, RESOURCE_TYPE,
-        PN_IMAGE_FROM_PAGE_IMAGE, false,
         PN_MEDIA_REF_STANDARD, pdfAsset.getPath()));
 
     Image underTest = AdaptTo.notNull(context.request(), Image.class);
@@ -393,7 +383,6 @@ void testWithNoImageAsset() {
   void testDisplayPopupTitle() {
     context.currentResource(context.create().resource(page, "image",
         PROPERTY_RESOURCE_TYPE, RESOURCE_TYPE,
-        PN_IMAGE_FROM_PAGE_IMAGE, false,
         PN_MEDIA_REF_STANDARD, asset.getPath(),
         PN_DISPLAY_POPUP_TITLE, false));
 
@@ -409,7 +398,6 @@ void testDisplayPopupTitle_ContentPolicy() {
 
     context.currentResource(context.create().resource(page, "image",
         PROPERTY_RESOURCE_TYPE, RESOURCE_TYPE,
-        PN_IMAGE_FROM_PAGE_IMAGE, false,
         PN_MEDIA_REF_STANDARD, asset.getPath()));
 
     Image underTest = AdaptTo.notNull(context.request(), Image.class);
@@ -424,7 +412,6 @@ void testLazyEnabled_ContentPolicy() {
 
     context.currentResource(context.create().resource(page, "image",
         PROPERTY_RESOURCE_TYPE, RESOURCE_TYPE,
-        PN_IMAGE_FROM_PAGE_IMAGE, false,
         PN_MEDIA_REF_STANDARD, asset.getPath()));
 
     Image underTest = AdaptTo.notNull(context.request(), Image.class);
@@ -440,7 +427,6 @@ void testUUID() {
 
     context.currentResource(context.create().resource(page, "image",
         PROPERTY_RESOURCE_TYPE, RESOURCE_TYPE,
-        PN_IMAGE_FROM_PAGE_IMAGE, false,
         PN_MEDIA_REF_STANDARD, asset.getPath()));
 
     Image underTest = AdaptTo.notNull(context.request(), Image.class);
@@ -459,7 +445,6 @@ void testUUID_Disabled() {
 
     context.currentResource(context.create().resource(page, "image",
         PROPERTY_RESOURCE_TYPE, RESOURCE_TYPE,
-        PN_IMAGE_FROM_PAGE_IMAGE, false,
         PN_MEDIA_REF_STANDARD, asset.getPath()));
 
     Image underTest = AdaptTo.notNull(context.request(), Image.class);
@@ -477,7 +462,6 @@ void testWidths() {
 
     context.currentResource(context.create().resource(page, "image",
         PROPERTY_RESOURCE_TYPE, RESOURCE_TYPE,
-        PN_IMAGE_FROM_PAGE_IMAGE, false,
         PN_MEDIA_REF_STANDARD, asset.getPath()));
 
     Image underTest = AdaptTo.notNull(context.request(), Image.class);
@@ -494,7 +478,6 @@ void testWidths() {
   void testAreas() {
     context.currentResource(context.create().resource(page, "image",
         PROPERTY_RESOURCE_TYPE, RESOURCE_TYPE,
-        PN_IMAGE_FROM_PAGE_IMAGE, false,
         PN_MEDIA_REF_STANDARD, asset.getPath(),
         PN_MAP, ImageAreaTestData.MAP_STRING));
 
@@ -511,7 +494,6 @@ void testWithImageAutoCropping_ContentPolicy() {
 
     context.currentResource(context.create().resource(page, "image",
         PROPERTY_RESOURCE_TYPE, RESOURCE_TYPE,
-        PN_IMAGE_FROM_PAGE_IMAGE, false,
         PN_MEDIA_REF_STANDARD, asset.getPath()));
 
     Image underTest = AdaptTo.notNull(context.request(), Image.class);
@@ -533,7 +515,6 @@ void testWithImageAutoCropping_ContentPolicy_WrappedResource() {
 
     Resource resource = context.create().resource(page, "image",
         PROPERTY_RESOURCE_TYPE, DELEGATE_RESOURCE_TYPE,
-        PN_IMAGE_FROM_PAGE_IMAGE, false,
         PN_MEDIA_REF_STANDARD, asset.getPath());
 
     // set context resource to wrapped resource

diff --git a/changes.xml b/changes.xml
@@ -27,9 +27,12 @@
       <action type="add" dev="sseifert" issue="13">
         Add Image (v3).
       </action>
-      <action type="update" dev="sseifert" issue="11">
+      <action type="update" dev="sseifert" issue="14">
         Update to AEM WCM Core Components 2.23.2.
       </action>
+      <action type="fix" dev="sseifert">
+        ComponentFeatureImageResolver: Switch to image from page by default if no image reference is given in image/teaser resource.
+      </action>
     </release>
 
     <release version="1.13.2-2.22.6" date="2023-08-21">