Project

General

Profile

Actions

Bug #92194

closed

ContentObjectRenderer wraps <img> tags in <p> tags

Added by Reindl Bernd over 3 years ago. Updated 9 months ago.

Status:
Resolved
Priority:
Must have
Assignee:
-
Category:
-
Target version:
-
Start date:
2020-09-04
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
10
PHP Version:
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

$this->_parseFunc($data, $tagConfig) in ContentObjectRenderer::_parseFunc() where the tags are parsed returns a string wrapped in <p></p>.
E.G. $tag is img $data = $this->_parseFunc($data, $conf); returns <p><img .....></p> but should return <img ....>.

I Think this happens because $data = $this->_parseFunc($data, $conf); uses $conf als param. But i think it should use $tagConfig instead.

if (strpos($data, '<') !== false && isset($conf['tags.']) && is_array($conf['tags.'])) {
    foreach ($conf['tags.'] as $tag => $tagConfig) {
                            // only match tag `a` in `<a href"...">` but not in `<abbr>`
                            if (preg_match('#<' . $tag . '[\s/>]#', $data)) {
                                $data = $this->_parseFunc($data, $conf);
                                break;
                            }
                        }
                    }
if (strpos($data, '<') !== false && isset($conf['tags.']) && is_array($conf['tags.'])) {
                        foreach ($conf['tags.'] as $tag => $tagConfig) {
                            // only match tag `a` in `<a href"...">` but not in `<abbr>`
                            if (preg_match('#<' . $tag . '[\s/>]#', $data)) {
                                $data = $this->_parseFunc($data, $tagConfig);
                                break;
                            }
                        }
                    }

Files

rte_ckeditor_image.zip (644 KB) rte_ckeditor_image.zip Reindl Bernd, 2021-01-22 06:52

Related issues 1 (0 open1 closed)

Related to TYPO3 Core - Bug #39261: parseFunc does not handle nested tags correctClosed2012-07-25

Actions
Actions #1

Updated by Reindl Bernd over 3 years ago

Still in 10.4.9

Actions #2

Updated by Reindl Bernd over 3 years ago

Idle since 3 months :(

Actions #3

Updated by Benni Mack about 3 years ago

  • Subject changed from ContentObjectRenderer to ContentObjectRenderer wraps <img> tags in <p> tags
  • Status changed from New to Needs Feedback

Thanks for your report.

Can you send me your:
  • lib.parseFunc / lib.parseFunc_RTE
  • example content which is handed into the method?
Actions #4

Updated by Reindl Bernd about 3 years ago

Benni Mack wrote in #note-3:

Thanks for your report.

Can you send me your:
  • lib.parseFunc / lib.parseFunc_RTE
  • example content which is handed into the method?

TYPO3 10.4.12 "Fluid Content Elements (fluid_styled_content)

# Creates persistent ParseFunc setup for non-HTML content.
lib.parseFunc {
    makelinks = 1
    makelinks {
        http {
            keep = {$styles.content.links.keep}
            extTarget = {$styles.content.links.extTarget}
        }
        mailto {
            keep = path
        }
    }
    tags {
        a = TEXT
        a {
            current = 1
            typolink {
                parameter.data = parameters:href
                title.data = parameters:title
                ATagParams.data = parameters:allParams
                # the target attribute takes precedence over config.intTarget
                target.ifEmpty.data = parameters:target
                # the target attribute takes precedence over the constant (styles.content.links.extTarget)
                # which takes precedence over config.extTarget
                # do not pass extTarget as reference, as it might not be set resulting in the string being
                # written to the target attribute
                extTarget.ifEmpty < config.extTarget
                extTarget.ifEmpty.override = {$styles.content.links.extTarget}
                extTarget.override.data = parameters:target
            }
        }
    }
    allowTags = {$styles.content.allowTags}
    denyTags = *
    sword = <span class="ce-sword">|</span>
    constants = 1
    nonTypoTagStdWrap {
        HTMLparser = 1
        HTMLparser {
            keepNonMatchedTags = 1
            htmlSpecialChars = 2
        }
    }
}

# Creates persistent ParseFunc setup for RTE content (which is mainly HTML) based on the "default" transformation.
lib.parseFunc_RTE < lib.parseFunc
lib.parseFunc_RTE {
    # Processing <ol>, <ul> and <table> blocks separately
    externalBlocks = article, aside, blockquote, div, dd, dl, footer, header, nav, ol, section, table, ul, pre
    externalBlocks {
        ol {
            stripNL = 1
            stdWrap.parseFunc = < lib.parseFunc
        }
        ul {
            stripNL = 1
            stdWrap.parseFunc = < lib.parseFunc
        }
        pre {
            stdWrap.parseFunc < lib.parseFunc
        }
        table {
            stripNL = 1
            stdWrap {
                HTMLparser = 1
                HTMLparser {
                    tags.table.fixAttrib.class {
                        default = contenttable
                        always = 1
                        list = contenttable
                    }
                    keepNonMatchedTags = 1
                }
            }
            HTMLtableCells = 1
            HTMLtableCells {
                # Recursive call to self but without wrapping non-wrapped cell content
                default.stdWrap {
                    parseFunc = < lib.parseFunc_RTE
                    parseFunc.nonTypoTagStdWrap.encapsLines {
                        nonWrappedTag =
                        innerStdWrap_all.ifBlank =
                    }
                }
                addChr10BetweenParagraphs = 1
            }
        }
        div {
            stripNL = 1
            callRecursive = 1
        }
        article < .div
        aside < .div
        blockquote < .div
        footer < .div
        header < .div
        nav < .div
        section < .div
        dl < .div
        dd < .div
    }
    nonTypoTagStdWrap {
        encapsLines {
            encapsTagList = p,pre,h1,h2,h3,h4,h5,h6,hr,dt
            remapTag.DIV = P
            nonWrappedTag = P
            innerStdWrap_all.ifBlank = &nbsp;
        }
    }
    nonTypoTagStdWrap {
        HTMLparser = 1
        HTMLparser {
            keepNonMatchedTags = 1
            htmlSpecialChars = 2
        }
    }
}

TS added by "rte_ckeditor_image"

## TS setup for TYPO3 image rendering

#******************************************************
# Including library for processing of magic images and file abstraction attributes on img tag
#******************************************************

lib.parseFunc {
    tags.img = TEXT
    tags.img {
        current = 1
        preUserFunc = Netresearch\RteCKEditorImage\Controller\ImageRenderingController->renderImageAttributes
    }
    tags.a = TEXT
    tags.a {
        current = 1
        preUserFunc = Netresearch\RteCKEditorImage\Controller\ImageLinkRenderingController->renderImages
    }
    nonTypoTagStdWrap.HTMLparser.tags.img.fixAttrib {
        allparams.unset = 1
        data-htmlarea-file-uid.unset = 1
        data-htmlarea-file-table.unset = 1
        data-htmlarea-zoom.unset = 1
        data-htmlarea-clickenlarge.unset = 1
    }
}

Content:
<p class="text-center"><a href="t3://page?uid=122"><img alt="10 well-named Pump Manuacturers belong to the SPA" data-htmlarea-file-table="sys_file" data-htmlarea-file-uid="1561" height="300" src="https://www.starpumpalliance.com/fileadmin/user_upload/bb03e3aebd.jpg" title="10 well-named Pump Manuacturers belong to the SPA" width="300" /></a>          <a href="t3://page?uid=118"><img alt="13 Types of Pumps from different Pump Technologies" data-htmlarea-file-table="sys_file" data-htmlarea-file-uid="1562" height="300" src="https://www.starpumpalliance.com/fileadmin/user_upload/7a7eefc40a.jpg" title="13 Types of Pumps from different Pump Technologies" width="300" /></a>          <a href="/about-spa/#c3712"><img alt="The SPA Members are present in more than 110 Countries" data-htmlarea-file-table="sys_file" data-htmlarea-file-uid="1563" height="300" src="https://www.starpumpalliance.com/fileadmin/user_upload/1acdb5e37a.jpg" title="The SPA Members are present in more than 110 Countries" width="300" /></a></p>

Best regards
Bernd

Actions #5

Updated by Riccardo De Contardi about 3 years ago

I am not totally sure (or I could possibly be totally wrong) wether this could come some CKEditor manipulation...

something like an inline element (img) must be wrapped in a block element

Actions #6

Updated by Reindl Bernd about 3 years ago

Riccardo De Contardi wrote in #note-5:

I am not totally sure (or I could possibly be totally wrong) wether this could come some CKEditor manipulation...

something like an inline element (img) must be wrapped in a block element

But with $conf instead of $tagConf ....


<p>
<img ...>
<img ...>
<img ...>
</p>

results in


<p><img ...></p>
<p><img ...></p>
<p><img ...></p>

because every image is wrapped no matter if it already is wrapped.

And $tagConf contains the settings for current tag. If you don't us it. What is it for?

Actions #7

Updated by Riccardo De Contardi about 3 years ago

I found an issue that could be related to this one: https://forge.typo3.org/issues/92301

Actions #8

Updated by Reindl Bernd over 2 years ago

  • Priority changed from Should have to Must have

Still in TYPO3 10.4.20.

So easy to solve. So why is this ticket ignored?

Actions #9

Updated by Mike Rucinski over 2 years ago

We run into the same problem after migrating from Typo3 v8 to v10.
As Bernd pointed out, it is obvious that the forEach loop passes the wrong parameter to the _parsFunc call: (ContentObjectRenderer.php -> Line 3993)

foreach ($conf['tags.'] as $tag => $tagConfig) {
// only match tag `a` in `<a href"...">` but not in `<abbr>`
if (preg_match('#<' . $tag . '[\s/>]#', $data)) {
$data = $this->_parseFunc($data, $conf);  // <-- this is the culprit
break;
}
}

You can see that $tagConfig is being set in the loop definition but it is never used.
Changing the _parseFunc call's second parameter to $tagConfig, fixes the problem for us.

foreach ($conf['tags.'] as $tag => $tagConfig) {
// only match tag `a` in `<a href"...">` but not in `<abbr>`
if (preg_match('#<' . $tag . '[\s/>]#', $data)) {
$data = $this->_parseFunc($data, $tagConfig); // <-- this fixes the issue
break;
}
}

Please fix this in the main branch so we don't have to apply a hot fix.

Actions #10

Updated by Ralf Hübner over 2 years ago

I´ve use 10.4.20 and I wondered why the a-Tag is´nt shown in frontend.

In ck-Editor I have this:
<p><a href="..."><img ...></a></p>

But the frontend renders:
<p><img ...></p>

I tested the fix above and the a-Tag is noew rendered ;-)

Actions #11

Updated by Riccardo De Contardi over 1 year ago

As far as I can see the lines

                        foreach ($conf['tags.'] as $tag => $tagConfig) {
                            // only match tag `a` in `<a href"...">` but not in `<abbr>`
                            if (preg_match('#<' . $tag . '[\s/>]#', $data)) {
                                $data = $this->_parseFunc($data, $conf);
                                break;
                            }
                        }

Are still present in TYPO 12.0.0-dev inside /typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php /lines 3446 and following

Actions #12

Updated by Reindl Bernd about 1 year ago

Riccardo De Contardi wrote in #note-11:

As far as I can see the lines

[...]

Are still present in TYPO 12.0.0-dev inside /typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php /lines 3446 and following

Yes. Nobody cares :(

Actions #13

Updated by Sybille Peters about 1 year ago

  • Status changed from Needs Feedback to New

Yes. Nobody cares :(

I disagree. The handful of people here who are very active, just don't have the time to solve everything. TYPO3 is a community driven project. Anyone can contribute to tracking down problems, finding solutions and creating a patch (as far as possible).

I understand that it can be frustrating to wait for patches, so in principal you have my sympathy. It just does not help to complain ;-)

Changing "Needs feedback" to "open" because feedback was asked for and was given.

You might want to fix the formatting of the code snippet though. It is quite hard to read this way. (comments and desciptions can be edited again by clicking the pencil and using formatting. Just be aware all watchers will get a notification every time someone does this).

Contribution Guide

Actions #14

Updated by Gerrit Code Review about 1 year ago

  • Status changed from New 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/+/78112

Actions #15

Updated by JAKOTA Design Group GmbH about 1 year ago

Hi,

I was trying to write a test that shows this issue in action but had no luck with that.

But I noticed something else by doing so.

Changing $conf to $tagConfig might mitigate your specific issue, but only in a roundabout way.
This nullifies the config for the next step and that is why no p tags are added.
The down side of doing so is that nested tags are also not handled anymore.

I've pushed this to Gerrit so you can see the failing tests yourself.
https://git.typo3.org/typo3/CI/cms/-/jobs/2063943

Actions #16

Updated by Oliver Hader 9 months ago

  • Related to Bug #39261: parseFunc does not handle nested tags correct added
Actions #17

Updated by Oliver Hader 9 months ago

  • Description updated (diff)
Actions #18

Updated by Oliver Hader 9 months ago

  • Status changed from Under Review to Rejected

Reindl Bernd wrote in #note-8:

Still in TYPO3 10.4.20.

So easy to solve. So why is this ticket ignored?

Actually it's not that easy - simple because the suggested "fix" might work for your specific problem, but breaks the working scenario for anybody else. The test cases in comment 15 (https://forge.typo3.org/issues/92194#note-15) actually prove how many things would be broken after that.

However, let's go back to the initial problem - and I hope I understood it correctly, the description was fuzzy & unclear. In the following example I'm using <img> tags nested in an outer <p> which shall be stripped.

# Default PAGE object:
page = PAGE
page.10 = TEXT
page.10.value (<p class="outer">
    <a href="t3://page?uid=2">
      <img src="whatever-a.png" data-htmlarea-file-uid="123" class="class" alt="name">
    </a>
    <img src="whatever-b.png" class="class">
  </p>
)
page.10 {
  # note: `HTMLparser` just used for removing `<p>`, to have `<img>` standalone
  # (using `stdWrap.HTMLparser` here, this way it is execute BEFORE `parseFunc` would be executed)
  stdWrap.HTMLparser = 1
  stdWrap.HTMLparser {
    allowTags = img,a
    removeTags = p
    tags {
      img {
        allowedAttribs = src,alt,class
      }
    }
  }

  # note: `parseFunc` is used for resolving `t3://` URIs to real 
  parseFunc {
    allowTags = img,a
    tags {
      a = TEXT
      a {
        current = 1
        typolink {
          parameter.data = parameters:href
        }
      }      
    }
    nonTypoTagStdWrap {
      HTMLparser = 1
      HTMLparser {
          keepNonMatchedTags = 1
          htmlSpecialChars = 2
      }
    }
    htmlSanitize = 1
  }
}
  • for this particular case of "extracting" <img>, parseFunc is not actually required → HTMLparser is used
  • however, the example also shows, how to use parseFunc (it is executed after stdWrap.HTMLparser, see the comments in the example)

Input markup

<p class="outer">
    <a href="t3://page?uid=2">
      <img src="whatever-a.png" data-htmlarea-file-uid="123" class="class" alt="name">
    </a>
    <img src="whatever-b.png" class="class">
  </p>
</p>

Output markup

    <a href="/en/">
      <img src="whatever-a.png" alt="name" class="class">
    </a>
    <img src="whatever-b.png" class="class">

Conclusion

Not a bug, no change required. I'm closing this issue.

Actions #19

Updated by Gerrit Code Review 9 months ago

  • Status changed from Rejected 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/+/79804

Actions #20

Updated by Gerrit Code Review 9 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/+/79806

Actions #21

Updated by Oliver Hader 9 months ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
Actions

Also available in: Atom PDF