[BUGFIX] Fix paste handling in CmdMap part

[opi99]

We now support negative uids on the target while handling paste
commands.

Resolves: #76

[IchHabRecht]

Hi @opi99,

The handling of a negative page id is handeled already in the DatamapDataHandlerHook, where it's belonging as you copy/paste an content element. Does the mentioned issue also happen with any core element and FAL references or does it apply to container elements only?

[opi99]

It's a content element build with mask, so no b13/container_element ... will check with Text-Image from core

[opi99]

Now, tried it with the "image" element with one image, now the source record have 2 references to same image.

[opi99]

In the issue I added to the description, that this shouldn't be the first position on page.
On the first position an other issue exists, it allows to fill in every element also the ones which are not allowed (together with b13/container extension).

[IchHabRecht]

I was wondering why this is broken as there are tests to cover the copy & paste handling. I found out that the datamap changed from TYPO3 version 7.6 (where content_defender comes from) to TYPO3 8.7. I adjusted the tests and the handling. Would you mind to test the branch https://github.com/IchHabRecht/content_defender/tree/fix-copy-paste and see if this fixes your problem/problems.

[opi99]

This leads to allowing all content to be added on target also the wrong ones ... but this sounds a bit stupid, as you didn't changed anything else. Need to check again and look deeper.

[IchHabRecht]

As tests are running, I would think there needs to be some other problem. Have you enabled content_defender at all?

[opi99]

So digging deeper ... more or less your patch does the same as mine for calculating. So without b13/container it should work.
The only difference is the call to $backendLayoutConfiguration->getConfigurationByColPos, you fill it with the $id of the source I fill it with 'NEW_'.

And here we come to the point, that this can't work. The function getConfigurationByColPos calls the $hookObject->manipulateConfiguration which uses the given $recordUid to find the parent but this will be the parent of the source and not the parent of the target. And that the source location worked is known couse the source resides there. This function only gets a target $colPos but knows nothing about the $targetElement. If the given $recordUid is (from my patch) 'NEW_' it gives the configuration back as is, which works on my system with my test but should also fail on other combinations like insert into another element and not on plain page.
Inside processDatamap_beforeStart, which worked before your patch, we have now the same $pageId to load BackendLayout from (like in Cmdmap) and the same $colPos, which now leads to load the same wrong configuration from getConfigurationByColPos as $recordUid is ignored in the runtime cache.

So, all in all, it seams the handling is wrong on both sides content_defender and b13/container so maybe it needs patches on both sides of the API to get this resolved. Maybe we could write/call in slack to look forward.

Edit: Maybe the source UID is needed to know what will be tried to insert but IMHO it shouldn't be needed to calculate the configuration array. But maybe another tool which likes to hook into could get the possibility to do so.

[IchHabRecht]

To be honest, I don't fully understand your explanation. Inside the call $backendLayoutConfiguration->getConfigurationByColPos($colPos, $id); (that is content_defender's own api) the given id (aka recordUid) is used only for the implemented hook api. So nothing in the content_defender api is influenced by the given record/id.

So from content_defenders point of view the paste of a content element on another page like described in your ticket should be fixed.

[opi99]

You have a hook inside your API

content_defender/Classes/BackendLayout/BackendLayoutConfiguration.php

Line 86 in 97a5662

foreach ($GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['content_defender']['ColumnConfigurationManipulationHook'] ?? [] as $className) {

which is used by b13/container the only point the $recordUid is used.

And here

content_defender/Classes/BackendLayout/BackendLayoutConfiguration.php

Line 66 in 97a5662

if (isset(self::$columnConfiguration[$configurationIdentifier][$colPos])) {

is your RuntimeCache which do not respect all function parameters.

[IchHabRecht]

This is intended and because there is not need for content_defender to build the configurationIdentifier depending on the record uid.

[opi99]

Yes, no need for content_defender but maybe for extensions which use the hook.
620e36a seams one part of fixing the described issue.

[IchHabRecht]

Fixed with #78