Exception with "Hidden" sys_file_references when using levelmedia - RootlineUtility::enrichWithRelationFields ignores disable field
Test scenario in TYPO3 6.2:
A Page (e.g. ID=15) has 2 sys_file_reference relations (pages.media)
Frontend output is via levelmedia, for instance
import.data = levelfield:1, media
treatIdAsReference = 1
import.listNum = 0
In 6.2 whichever reference is sorted to the top in the backend is output as expected so far.
(in 6.1 this does not work because sorting_foreign was ignored http://forge.typo3.org/issues/46383#change-204405) but that is fixed for 6.2 with https://review.typo3.org/#/c/26727/
Now the editor uses the "Light bulb" icon to hide the first reference.
The second reference should be shown now.
Exception thrown "#1317178794: No fileusage (sys_file_reference) found for given UID"
levelmediameans the rootline is checked for the references.
- They are put there by the function
- This loads relations by generating a SQL query.
- This query checks that record isn't
deleted=1and in 6.2 also orders by
foreign_sortby(in 6.1 it doesn't)
- However it does not check for
This means that both sys_file_reference entries go into the "enriched" rootline data.
import.listNum = 0 means that the first one is returned.
That one is hidden.
So when we get to
TYPO3\CMS\Core\Resource\ResourceFactory::getFileReferenceObject the exception is thrown because only the hidden reference has arrived.
This is pretty confusing for editors as they are using seemingly harmless, legal operations in TYPO3 ("hiding") and this ends up killing entire page trees in the frontend. As when the rootline is checked of course all the pages below can be affected. A typical application of levelmedia is banners for a chapter and so this situation could "exceptionalize" an entire website chapter just due to hinding a banner.
This is hitting live projects with 6.1
In 6.2 they can solve that by resorting the references but in 6.1 they can't
It would be easy to change
enrichWithRelationFields to respect
$GLOBALS['TCA'][$table]['ctrl']['enablecolumns']['disabled'] in general or for
However the comment in
NOTICE: This function only takes deleted pages into account! So hidden, starttime and endtime restricted pages are included no matter what.
So this seems to be necessary/desired behaviour?
Is it possible to make levelmedia in 6.1/6.2 "hidden-safe" without breaking that behaviour (e.g. by applying the hidden check only for sys_file_reference relations in
Or could at least the exception "No fileusage (sys_file_reference) found for given UID" be replaced with a more graceful failure that allows the page to be rendered? Editors will then just scratch their heads why there is no image instead of going into panic.
This was all not a problem in 4.x because pages.media was just a comma separated list of values, there was no "hiding" of a relation from that, and no foreign sorting either. So checking hidden was indeed irrelevant for levelmedia in 4.x
Updated by Markus Klein almost 7 years ago
- Status changed from New to Accepted
- Priority changed from -- undefined -- to Must have
- Target version set to next-patchlevel
- TYPO3 Version changed from 6.2 to 6.1
thanks for this in-depth report!
Yes, the foreign_sortby was introduced by me, but not backported, as it might be a breaking change.
I fully agree that the exception, which is ok IMO, must be caught and must not break the FE.
The general question, whether getRootLine() should all enable fields has a lot of consequences probably, that's why I've to hand this over to people with more in-depth knowledge about this area in the Core.
Since this is also a problem for 6.1 live sites, as you wrote above, I'm setting the version to 6.1 for this ticket.
Updated by Christian Reiter almost 7 years ago
For live 6.1 sites I've just made a quixk and dirty xclass fix. It just backports the enrichWithRelationFields from current 6.2 (importing the solution for #46383 to 6.1) and adds disableClause.
This delivers correct behaviour for a 6.1 site affected by the bug.
- sorting OK
- All references hidden - no exception, behaviour as if no refs
- one reference hidden, one visible - visible one picked by listNum=0 regardless of sorting
is RootlineUtility only used in the Frontend or also called from the Backend?
If it's also used from the BE I'd check the TYPO3_MODE as "hidden" should not be "invisible" in the backend.
Updated by Christian Reiter almost 7 years ago
TYPO3\CMS\Core\Utility\RootlineUtility::__construct there is a check for the
$GLOBALS['TSFE']->sys_page object for the page context, maybe by checking for this in
enrichWithRelationFields we could avoid side effects when the RootlineUtility is used for something else than rendering a frontend page?
Updated by Christian Reiter over 6 years ago
So then I guess the information that is available within the scope of that function is possibly not sufficient to be sure that everything is safe.
I see two possibilities right now,1.
- add some kind of parameter that says "I explicitly want to enrich a rootline for FE rendering" - although I don't really like the concept of adding such parameters
- perhaps from TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer::rootLineValue which is called to make the rootline for levelmedia
- reason: Comment for this function says "@todo Define visibility"
- so someone seems to be aware that this function returns a rootline with undefined treatment of visibility?
- This function "_rootLineValue_" is used from TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer::getData
- That one again has "@todo Define visibility" in the comment
- getData is basically just a big switch construct which treats the different TS functions like leveltitle,levelmedia,leveluid,levelfield etc...
so it is pretty much limiting things to a Typoscript Frontend context where we would want the change to be effective
- Also since this is basically the cObj class, it should be frontend, or at least simulation of frontend?
- Just change it and test a lot... ;)
I've had no problem with the Xclass solution that adds the visibility/disablefield check so far on a live project but of course that is ony one site.
Updated by Markus Klein over 6 years ago
@todo Define visibility
refers to the protection of the method itself. This was added when all the function got access modified. Back then it was not known which function are protected or not, so all of them got public.
It will take years to finally make those protected, which are actually used only internally and we never know, if such a function has been used by an extension.
You'll find these todos everywhere in Core.
I propose to simply add it for now to have a solution in Gerrit. Then it can be tested and can be part of the next beta version, before backporting it to 6.1.
Can you push a patch?