Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Teaser V2] Invalid HTML if teaser has link + image but no Title #2660

Open
ky940819 opened this issue Jan 18, 2024 · 0 comments
Open

[Teaser V2] Invalid HTML if teaser has link + image but no Title #2660

ky940819 opened this issue Jan 18, 2024 · 0 comments

Comments

@ky940819
Copy link
Contributor

Bug Report

Current Behavior
When a teaser has:

  1. The Link field set
  2. An image set
  3. Blank title

then the resulting HTML is invalid because of nested anchor tags - which is prohibited.

Expected behavior/code
The component should not create HTML that is invalid.

Environment

  • AEM 6.5.19.0
  • Core Components 2.23.4
  • JRE version 11

Possible Solution
The issue stems from the v2/TeaserImpl.java model initImage() method:

    protected void initImage() {
        ...
        if (StringUtils.isNotEmpty(getTitle()) || getTeaserActions().size() > 0) {
            overriddenImageResourceProperties.put(Teaser.PN_IMAGE_LINK_HIDDEN, Boolean.TRUE.toString());
        }
        ...
    }

We see here that the link on the image is suppressed if there is a non-empty title or there are teaser actions.
However, if we look at the component html teaser/v2/teaser/teaser.html we see the following:

    <a class="cmp-teaser__link"
       data-sly-attribute="${teaser.link.htmlAttributes}"
       data-sly-unwrap="${!teaser.link.valid || !teaser.actions.empty}"
       data-cmp-clickable="${teaser.data ? true : false}">
        <div class="cmp-teaser__content">
            <sly data-sly-call="${pretitleTemplate.pretitle @ teaser=teaser}"></sly>
            <sly data-sly-call="${titleTemplate.title @ teaser=teaser}"></sly>
            <sly data-sly-call="${descriptionTemplate.description @ teaser=teaser}"></sly>
            <sly data-sly-call="${actionsTemplate.actions @ teaser=teaser}"></sly>
        </div>
        <sly data-sly-call="${imageTemplate.image @ teaser=teaser}"></sly>
    </a>

Here we see that the outer anchor is unwrapped (removed) if the link is invalid or there are CTA links.

The problem is that the logic for if the image should be linked, and the logic for if the entire Teaser should be linked are not in agreement. This can lead to situations where both the Teaser and the Image are linked - which causes broken HTML because links are not allowed inside of links.

The solution to this is to make sure that the case for when an image is linked and when the teaser is linked are mutually exclusive.
I'm not entirely sure what the blankness of the title has to do with if the image should be linked or not, removing that condition from the V2 model implementation would probably resolve the issue.

A more sustainable approach, that would require a change to the model interface, would be moving the logic for the data-sly-unwrap into a method called something like boolean getLinkTeaser() and then the HTL becomes data-sly-unwrap="${!teaser.linkTeaser}" and then this method can be re-used inside models initImage() method as such:

        if (this.getLinkTeaser()) {
            overriddenImageResourceProperties.put(Teaser.PN_IMAGE_LINK_HIDDEN, Boolean.TRUE.toString());
        }

or, better yet

        overriddenImageResourceProperties.put(Teaser.PN_IMAGE_LINK_HIDDEN, Boolean.valueOf(this.getLinkTeaser()).toString());
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant