Project

General

Profile

Actions

Bug #100867

closed

Image srcset paths get prefixed with server path in the output

Added by Manuel Glauser 12 months ago. Updated 10 months ago.

Status:
Resolved
Priority:
Must have
Category:
Frontend
Target version:
Start date:
2023-05-12
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
12
PHP Version:
8.2
Tags:
Complexity:
easy
Is Regression:
Yes
Sprint Focus:

Description

When rendering an "IMAGE" content object according to layoutKey 'picture'), paths to the rendered image sources are prefixed with the "Environment:getPublicPath()" value, resulting in corrupt image paths in the frontend.

How to reproduce
TypoScript setup:

lib.renderedImage = IMAGE
lib.renderedImage {
    file = fileadmin/testimage.jpg
    file {
        maxW = 800
        maxH = 800
    }

    layoutKey = picture

    layout {
        picture {
            element = <picture>###SOURCECOLLECTION###<img src="###SRC###" ###PARAMS### ###ALTPARAMS### ###SELFCLOSINGTAGSLASH###></picture>
            source = <source srcset="###SRC###" media="###MEDIAQUERY###" ###SELFCLOSINGTAGSLASH###>
        }
    }

    sourceCollection {
        landscape-large {
            mediaQuery = (min-width: 1800px) and (orientation:landscape)
            width = 2000c
            height = 1000c
        }
        portrait-large {
            mediaQuery = (min-height: 1800px) and (orientation:portrait)
            width = 1000c
            height = 2000c
        }
        landscape-medium {
            mediaQuery = (min-width: 1000px) and (orientation:landscape)
            width = 1200c
            height = 800c
        }
        portrait-medium {
            mediaQuery = (min-height: 1000px) and (orientation:portrait)
            width = 800c
            height = 1200c
        }
    }
}

This results in the following output (note the additional paths prepended to each "srcset" attribute):

<picture>
  <source srcset="/home/app/source/web//fileadmin/_processed_/0/b/csm_testimage_f41a083527.jpg" media="(min-width: 1800px) and (orientation:landscape)">
  <source srcset="/home/app/source/web//fileadmin/_processed_/0/b/csm_testimage_0ca3f7597f.jpg" media="(min-height: 1800px) and (orientation:portrait)">
  <source srcset="/home/app/source/web//fileadmin/_processed_/0/b/csm_testimage_276dc20341.jpg" media="(min-width: 1000px) and (orientation:landscape)">
  <source srcset="/home/app/source/web//fileadmin/_processed_/0/b/csm_testimage_9b57637f4e.jpg" media="(min-height: 1000px) and (orientation:portrait)">
  <img src="/fileadmin/_processed_/0/b/csm_testimage_e3e75227ae.jpg" alt="">
</picture>

"/home/app/source/web/" is the local path to the document root directory in my container.

Analysis/Solution
As far as I can tell, changes applied to solve #95379 did apply "PathUtility::stripPathSitePrefix()" in most cases, which is why the default image's path (above) does not have the extra path prepended. However the change probably missed stripping the path in one location: https://github.com/TYPO3-CMS/frontend/blob/main/Classes/ContentObject/ImageContentObject.php#L224 leading to the corrupted output.
Instead of

$sourceConfiguration['src'] = htmlspecialchars($urlPrefix . $sourceInfo[3]);

this line should probably read
$sourceConfiguration['src'] = htmlspecialchars($urlPrefix . PathUtility::stripPathSitePrefix($sourceInfo[3]));

as $this->cObj->getImgResource()[3] (which the above $sourceInfo3 resolves to) now always contains the document root prefix, according to the current source code and #95379.

Actions #1

Updated by Manuel Glauser 12 months ago

  • Description updated (diff)
Actions #2

Updated by Oliver Hader 12 months ago

  • Status changed from New to Accepted
Actions #3

Updated by Gerrit Code Review 12 months ago

  • Status changed from Accepted to Under Review

Patch set 1 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/79030

Actions #4

Updated by Gerrit Code Review 12 months ago

Patch set 2 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/79030

Actions #5

Updated by Gerrit Code Review 12 months ago

Patch set 3 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/79030

Actions #6

Updated by Benjamin Robinson 12 months ago

I tested "Patch set 3" manually - it did not work. Output before the patch:

<img src="/fileadmin/_processed_/7/d/csm_file_00_392x392%402x_60b99adfc6.jpg" srcset="/home/www/p123456/html/typo3-staging//fileadmin/media/file_00_392x392%402x.jpg 2x" width="392" height="392" loading="lazy" alt="xyz">

Output after:

<img src="/fileadmin/_processed_/7/d/csm_file_00_392x392%402x_60b99adfc6.jpg" srcset=" 2x" width="392" height="392" loading="lazy" alt="xyz">

My TypoScript:


10 = IMAGE
10{
    file{
        import.dataWrap = {file:current:storage}:{file:current:identifier}
        width = 392
    }
    altText{
        field = seo_title // title
        stripHtml = 1
    }
    layout.srcset {
        element = <img src="###SRC###" srcset="###SOURCECOLLECTION###" width="###WIDTH###" height="###HEIGHT###" ###PARAMS######ALTPARAMS######SELFCLOSINGTAGSLASH###>
        source = ###SRC### ###SRCSETCANDIDATE###
    }
    params = loading="lazy" 
    layoutKey = srcset
    sourceCollection {
        10 {
            width = 784
            srcsetCandidate = 2x
        }
    }
}

Actions #7

Updated by Benjamin Robinson 12 months ago

Sorry, I made a mistake while testing. I can now confirm that the patch works. This is my first test of a patch. Should I confirm it somewhere in the commit? If so, how?

Actions #8

Updated by Manuel Glauser 12 months ago

I was just about to write back, as I couldn't reproduce the error. Thank you :)

Actions #9

Updated by Manuel Glauser 12 months ago

If you could officially review it here: https://review.typo3.org/c/Packages/TYPO3.CMS/+/79030 that'd be great.
Documentation on how to review: https://docs.typo3.org/m/typo3/guide-contributionworkflow/main/en-us/HandlingAPatch/Review.html#lifeofapatch-review

If you need help, hit me up on Slack.

Actions #10

Updated by Manuel Glauser 12 months ago

  • Assignee set to Manuel Glauser
  • Priority changed from Should have to Must have
  • Target version changed from 12 LTS to next-patchlevel
  • % Done changed from 0 to 100
Actions #11

Updated by Gerrit Code Review 10 months ago

Patch set 1 for branch 12.4 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/79682

Actions #12

Updated by Manuel Glauser 10 months ago

  • Status changed from Under Review to Resolved
Actions

Also available in: Atom PDF