[BUGFIX] keep child order on copy and move to other language - #761
Open
DenisMir wants to merge 1 commit into
Open
[BUGFIX] keep child order on copy and move to other language#761DenisMir wants to merge 1 commit into
DenisMir wants to merge 1 commit into
Conversation
copyOrMoveChildren reverses the children per colPos, which is only correct as long as every child is pasted on the same target, resolved to the top of the container column. When the language changes, the target switches to the previously handled child from the second child on, so the reversed order is no longer compensated and the children end up in reverse order. Fixes: b13#760
|
PRAIS Thank you. I'll test your patch on monday next week. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #760
copyOrMoveChildren()reverses the children per colPos. That is correct as long as every child is pasted on the same positive target, whichrewriteCommandMapTargetForTopAtContainer()resolves to "top of the container column" — inserting a reversed list at the top repeatedly restores the original order.As soon as the paste
updatecarries asys_language_uidthat differs from the child's own language, the target switches to-$previousUidat the end of the first iteration. From the second child on the semantics change from insert at top to insert after the previous one, while the list is still reversed, so the children end up in reverse order. This applies to thecopyand themovebranch alike.The fix keeps the reversal for the top-insert case and iterates in natural sorting order when the loop is going to chain.
Tests
Three cases in
Tests/Functional/Datahandler/Localization/FreeMode/ContainerTest.php:copyContainerKeepsChildOrdercopyContainerToOtherLanguageKeepsChildOrdermoveContainerToOtherLanguageKeepsChildOrderThe existing cross-language fixtures only have a single child per colPos, where
array_reverse()is a no-op — that is why this went unnoticed. The new fixtures use three children in colPos 200 and two in colPos 201.Without the fix both cross-language tests fail with the children swapped; the same-language test stays green. With the fix the full suite passes (524 functional, 23 unit), CGL and PHPStan v14 are clean.
Note
A few lines below the change,
BackendUtility::getRecord('tt_content', abs($newId), 'pid')returns?arraybut$previousRecord['pid']is accessed unconditionally. Unrelated to this bug, so I left it out — happy to add a guard here or in a separate PR, whichever you prefer.