Bug #92194
closedContentObjectRenderer wraps <img> tags in <p> tags
Added by Reindl Bernd about 4 years ago. Updated 5 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 almost 4 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 almost 4 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 almost 4 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 almost 4 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 almost 4 years ago
I found an issue that could be related to this one: https://forge.typo3.org/issues/92301
Updated by Reindl Bernd over 3 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 about 3 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 about 3 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 about 2 years 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 almost 2 years 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 over 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 over 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 over 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 over 1 year ago
- Related to Bug #39261: parseFunc does not handle nested tags correct added
Updated by Oliver Hader over 1 year 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 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 over 1 year 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 over 1 year 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 over 1 year ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset 7415c90089f3f4c4f001b53ec40dc7a2e32ad52c.