adobe / aem-core-wcm-components

Standardized components to build websites with AEM.
https://docs.adobe.com/content/help/en/experience-manager-core-components/using/introduction.html
Apache License 2.0
742 stars 749 forks source link

Content Language for Breadcrumb links referring to custom/unknown page property? #2815

Open HitmanInWis opened 3 months ago

HitmanInWis commented 3 months ago

Bug present as of version: 2.25.5-SNAPSHOT

https://github.com/adobe/aem-core-wcm-components/pull/2725/files added a language attribute to Breadcrumb links. However, as best I can tell the contentLang property being referred to in breadcrumb.html is not a standard page property.

<sly data-sly-set.lang="${navItem.page.properties.contentLang}"/>

Also, if it is important to set language on Breadcrumb links, shouldn't we be doing the same for all links?

ky940819 commented 2 months ago

I agree with this. The contentLang page property doesn't appear anywhere.

The pull request says when a custom language is defined in advanced property of the page. However, when a language is selected in the page properties dialog it is stored under the property name jcr:language.

Additional Issues:

I think the proper solution to this issue would be to get the language information in the model (or directly in the LinkBuilder) using either the LanguageManager or Page.getLanguage() and then expose that information as part of the Link and/or it's HTML attributes.

Adding this functionality directly to the LinkBuilder would make it global for any page-based link. One concern there is that the hreflang can reasonably be set on any link (because it refers to the language of the target page content), but the lang attribute should only be set when the text of the link is in a different language - this would likely be true in any situation where the link text is derived from the target page.

vladbailescu commented 2 months ago

Pinging @LSantha into the conversation as he reviewed and merged the PR from @renow-luxembourg