Project

General

Profile

Actions

Bug #73240

closed

substituteMarkerArrayCached completely broken in TYPO3 6.2.17

Added by Franz Holzinger about 8 years ago. Updated about 8 years ago.

Status:
Closed
Priority:
Must have
Assignee:
-
Category:
Frontend
Target version:
-
Start date:
2016-02-11
Due date:
% Done:

70%

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

Description

The issue #72252 introduces big problems in the front end content rendering.
The generated front end is completely messy. Some markers are not recognized and others are randomly replaced by the contents of other markers.


Files


Related issues 1 (0 open1 closed)

Related to TYPO3 Core - Bug #72252: substituteMarkerArrayCached not substitute $subpartContentArrayClosedMarkus Klein2015-12-15

Actions
Actions #1

Updated by Markus Klein about 8 years ago

How do your markers look like?
Can you please, please, please post examples.
(We need them also to add them to the unit tests.)

Actions #2

Updated by Markus Klein about 8 years ago

Your current solution kills 2 unit tests:

            'one marker with lots of chars' => array(
                'dummy content with ###RE.:##-=_()LACED### marker',
                array(
                    '###RE.:##-=_()LACED###' => '_replaced_'
                ),
                array(),
                array(),
                'dummy content with _replaced_ marker'
            ),
            'markers which are special' => array(
                'dummy ###aa##.#######A### ######',
                array(
                    '###aa##.###' => 'content ',
                    '###A###' => 'is',
                    '######' => '-is not considered-'
                ),
                array(),
                array(),
                'dummy content #is ######'
            ),

There were 2 failures:

1) TYPO3\CMS\Frontend\Tests\Unit\ContentObject\ContentObjectRendererTest::substituteMarkerArrayCachedReturnsExpectedContent with data set "one marker with lots of chars" ('dummy content with ###RE.:##-...marker', array('_replaced_'), array(), array(), 'dummy content with _replaced_ marker')
storeHash call count mismatch
Failed asserting that 0 is identical to 1.

/mnt/hgfs/typo3src/TYPO3_6-2/typo3/sysext/frontend/Tests/Unit/ContentObject/ContentObjectRendererTest.php:3524
/mnt/hgfs/typo3src/TYPO3_6-2/Packages/Libraries/phpunit/phpunit/phpunit:36

2) TYPO3\CMS\Frontend\Tests\Unit\ContentObject\ContentObjectRendererTest::substituteMarkerArrayCachedReturnsExpectedContent with data set "markers which are special" ('dummy ###aa##.#######A### ######', array('content ', 'is', '-is not considered-'), array(), array(), 'dummy content #is ######')
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-dummy content #is ######
+dummy ###aa##.#######A### ######

/mnt/hgfs/typo3src/TYPO3_6-2/typo3/sysext/frontend/Tests/Unit/ContentObject/ContentObjectRendererTest.php:3525
/mnt/hgfs/typo3src/TYPO3_6-2/Packages/Libraries/phpunit/phpunit/phpunit:36

Updated by Franz Holzinger about 8 years ago

I wonder if it is really supported to use '###' inside of marker names. Who will ever need this?

I have added 2 files with debug information.

Actions #4

Updated by Markus Klein about 8 years ago

Thanks, will take a look.
By the way: '###' is of course NOT supported, but '##' is.

The unit tests were created with possible options how people might have used those markers.

Actions #5

Updated by Franz Holzinger about 8 years ago

Then I still wonder why '##' is supported. I propose to forbid any character '#' inside of a marker name.

Actions #6

Updated by Markus Klein about 8 years ago

Well, I would too, but it hasn't been in the past, so we can't change that requirement in a released version.

First finding: You use invalid markers. e.g.:

<!--###LINK_ITEM##-->

It's missing an #.

Actions #7

Updated by Markus Klein about 8 years ago

  • Status changed from New to Needs Feedback

And your code generates wrong HTML, because you only use ###G1B###, but never ###G1E###.

Actions #8

Updated by Franz Holzinger about 8 years ago

The usage of ###GW1B### without a ###GW1E### should however be no problem, because all ###GW1B### are inside of HTML comments. And ###GW1B### is only a single marker.
It could produce wrong HTML, but the wrong parts are always commented out.

###GW1E###    </span>
###GW1B###    <span class="tx-ttproducts-pi1-wrap1">
        <!--
        <td valign=top>###GW1B### ###PRICE_TAX### </td>
        -->
Actions #9

Updated by Markus Klein about 8 years ago

The usage of ###GW1B### without a ###GW1E### should however be no problem

The marker usage is no problem of course, this was just a side note.

Did the remaining problem resolve by fixing the LINK_ITEM marker?

Actions #10

Updated by Franz Holzinger about 8 years ago

It also works after fixing only the subpart marker ###LINK_ITEM###.

    <div class="ttprod_single_title">
        <!--###LINK_ITEM###-->
        <h2 class="head">###PRODUCT_TITLE###</h2>

        <h3 class="subhead">###PRODUCT_SUBTITLE###</h3>
        <!--###LINK_ITEM###-->
    </div>

Then also the rest of the HTML of the page is put together correctly.

Actions #11

Updated by Markus Klein about 8 years ago

  • Status changed from Needs Feedback to Closed
  • Target version deleted (6.2.18)

Alright, then everything is fine.

Actions #12

Updated by Franz Holzinger about 8 years ago

No, nothing is fine with this severe change in the behaviour. You get a weird output in the front end if you miss one or two '#'. This makes it much more difficult to use TYPO3.

Actions #13

Updated by Andreas Kienast about 8 years ago

You could also argue that PHP is "hard" to write if you miss a semicolon at the end of a line or add a opening bracket too much. Please stick to the correct syntax, it previously worked by accident.

Actions #14

Updated by Franz Holzinger about 8 years ago

@Andreas Otto † Fernandez: No, your comparison with PHP does never fit the case with a missing '#' in the HTML file, because you get a clear error message in PHP which gives the file name and the line number. However in TYPO3 you get no error message and weird output. You have no clue at all where to search for this error. So in TYPO3 you have very often to search and investigate for hours.

Actions #15

Updated by Wouter Wolters about 8 years ago

`A marker is a word wrapped in three hash tags (###) on either side.`
`A marker is a word in your HTML template, which is wrapped by "###" on both sides.`

From the official documentation. It states that it needs to have "3" hash tags!

https://wiki.typo3.org/Templating_Tutorial_-_Basics#Short_information_about_markers_and_subparts
https://docs.typo3.org/typo3cms/TemplatingTutorial/Tasks/WorkingHTML/HtmlTemplate/Index.html

Actions #16

Updated by Markus Klein about 8 years ago

Well... It's a matter of principle how this marker stuff works that no error messages can be generated...
If you want structured templates use Fluid. At least that's what we do, exactly because of this and because your controllers simply loose 50% of their code. ;-)

Actions #17

Updated by Franz Holzinger about 8 years ago

@Wouters: Thanks for the info. But we all did know this already.

@Markus Klein: But what is the adavantage of your last patch in comparison to my patch?

  • You support the '#' as a part of a marker name. Nobody will ever use this. Or do you know a person. And it breaks the former behaviour of TYPO3.
  • My patch supports to have a normal output even if a marker has not been closed correctly by '###'. This is how TYPO3 has been working from version 3.0 until 6.2.16.
    Without this most users will have to invest many hours to discover the reason for the wrong front end output. I already know several persons who ran into this bug. A simple '#' as a marker name does not work any more. This should never have been supported.
Actions #18

Updated by Markus Klein about 8 years ago

It all started out with doing you a faviour by fixing your bug report back from 4.7 with #44270.
This fix did contain a regression unfortunately, I'm sorry for that, but we got that straight now with the following patches.
We now also have the benefit to have this function really well unit-tested.

It is a matter of fact that each bugfix is breaking change in terms of "it does not work as before", but usually people want the intended functionality even if it means to change a workaround.

Regarding your patches: You still keep attaching patches to forge here, albeit it is common knowledge that patches are handled in Gerrit for a long time now, and anybody can push those there.

I don't know how your projects look like, but in our current projects the extensions we use all use Fluid (except felogin), and I don't know a single person except you, who ran into this issue.

I'm really angry now about having wasted lots of hours thinking around that very function, just because users of your extension sent me mails, telling me that you suggested them that there is a Core bug. In the end it turns out that the code is perfectly fine now and the extension is the culprit.

So I will not spend any more time on this topic.
If you feel like changing something again, take your time and push a patch to Gerrit and gather reviews via Slack.
I'll not be in your way.

Regards, Markus

Actions #19

Updated by Michael Sollmann about 8 years ago

After updating to 6.2.18 my frontend crashed - no markers were replaced anymore. After hours of searching I found the problem. It was this comment:

Since the property "markerWrap" is set to "###|###" the above comment must not crash the frontend! It has to be fixed, my opinion.
(Setting "nonCachedSubst" to "1" fixes the problem too, but I prefer to use a clean solution.)

Regards, Michael

Actions #20

Updated by Markus Klein about 8 years ago

@Sollmann: Please create a new ticket and add me as watcher.
We sort that out then.

Actions

Also available in: Atom PDF