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.

Sign up for GitHub

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

Jump to bottom

Sitemap fix error in case of image with null Title or Caption #37462

Open
wants to merge 8 commits into
base: 2.4-develop
Choose a base branch
from
Open

Sitemap fix error in case of image with null Title or Caption #37462

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b09d03f
Fix error in case of image with null Title or Caption
RPrudon May 4, 2023
cabaaaf
Merge branch '2.4-develop' into patch-4
RPrudon May 25, 2023
a367075
Merge branch '2.4-develop' into patch-4
engcom-Hotel Jun 6, 2023
5c14d7a
37462: Fix PHPCS warnings
engcom-Dash Oct 22, 2024
06547ed
37462: remove extra line
engcom-Dash Oct 22, 2024
1d5fd30
Merge branch '2.4-develop' into patch-4
engcom-Dash Oct 22, 2024
dc8e367
37462: Fix static failure related to copyright tag
engcom-Dash Oct 23, 2024
329e7c1
Merge branch '2.4-develop' into patch-4
engcom-Hotel Nov 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions app/code/Magento/Sitemap/Model/Sitemap.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
* Copyright 2011 Adobe
* All Rights Reserved.
*/

namespace Magento\Sitemap\Model;
Expand Down Expand Up @@ -576,9 +576,9 @@ protected function _getSitemapRow($url, $lastmod = null, $changefreq = null, $pr
foreach ($images->getCollection() as $image) {
$row .= '<image:image>';
$row .= '<image:loc>' . $this->_escaper->escapeUrl($image->getUrl()) . '</image:loc>';
$row .= '<image:title>' . $this->escapeXmlText($images->getTitle()) . '</image:title>';
$row .= '<image:title>' . $this->escapeXmlText((string)$images->getTitle()) . '</image:title>';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be $image instead of $images

Suggested change
$row .= '<image:title>' . $this->escapeXmlText((string)$images->getTitle()) . '</image:title>';
$row .= '<image:title>' . $this->escapeXmlText((string)$image->getTitle()) . '</image:title>';

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@engcom-Hotel, I have analyzed and debugged the code and found that the current implementation is correct. The reason is that $image does not contain any title; the title is associated only with $images. This is why, in the current codebase, it is $images instead of $image. Please have a look at below screenshots:
image
image

Additionally, for the caption, we are already checking if it is null, so IMO, we don't need the typecasting, as the error mentioned is related to null values only.

Since the changes is related to typecasting only and we already have automated tests for this class, could you please review the implementation?

Thanks!

cc: @RPrudon

if ($image->getCaption()) {
$row .= '<image:caption>' . $this->escapeXmlText($image->getCaption()) . '</image:caption>';
$row .= '<image:caption>' . $this->escapeXmlText((string)$image->getCaption()) . '</image:caption>';
}
$row .= '</image:image>';
}
Expand Down Expand Up @@ -815,6 +815,7 @@ public function getSitemapUrl($sitemapPath, $sitemapFileName)
* @return bool
* @deprecated 100.1.5 Because the robots.txt file is not generated anymore,
* this method is not needed and will be removed in major release.
* @see Nothing
*/
protected function _isEnabledSubmissionRobots()
{
Expand All @@ -829,6 +830,7 @@ protected function _isEnabledSubmissionRobots()
* @return void
* @deprecated 100.1.5 Because the robots.txt file is not generated anymore,
* this method is not needed and will be removed in major release.
* @see Nothing
*/
protected function _addSitemapToRobotsTxt($sitemapFileName)
{
Expand Down