Bug #92194
closed
ContentObjectRenderer wraps <img> tags in <p> tags
Added by Reindl Bernd over 3 years ago. Updated 10 months ago.
100%
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 |
Updated by Benni Mack over 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?
Updated by Reindl Bernd over 3 years ago
- File rte_ckeditor_image.zip rte_ckeditor_image.zip added
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 =
}
}
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
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
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?
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
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?
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.
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 ;-)
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
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 :(
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).
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
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
Updated by Oliver Hader 10 months ago
- Related to Bug #39261: parseFunc does not handle nested tags correct added
Updated by Oliver Hader 10 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>,parseFuncis not actually required →HTMLparseris used - however, the example also shows, how to use
parseFunc(it is executed afterstdWrap.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.
Updated by Gerrit Code Review 10 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
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/+/79806
Updated by Oliver Hader 10 months ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset 7415c90089f3f4c4f001b53ec40dc7a2e32ad52c.