Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUGFIX] Fix for colPos handling in ColumnPositionItems.php #2236

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

rtfirst
Copy link

@rtfirst rtfirst commented Feb 26, 2025

Description
This pull request fixes a bug in the colPosListItemProcFunc method of the ColumnPositionItems class. The issue occurred when the $parameters['row']['colPos'] value was not set, which led to incorrect or unexpected behavior when calculating the parent record UID. By ensuring that the colPos value is checked and cast to an integer (defaulting to 0 when not set), we prevent potential runtime errors and ensure consistent behavior.

Motivation
The bug was reported in scenarios where the colPos parameter was missing from the $parameters array. Since this value is critical for determining the parent record UID via the ColumnNumberUtility::calculateParentUid() method, missing or invalid data could lead to issues in rendering the backend layout columns. This fix enhances the robustness of the code by providing a reliable fallback and avoiding errors.

Changes Made
Added a check for the existence of $parameters['row']['colPos'].
Cast the value to an integer, defaulting to 0 if not set.
Updated the calculation of parentRecordUid to use the safe, validated colPos value.

Testing
Manually verified that the patch applies correctly.
Confirmed that the fix resolves the issue when the colPos parameter is missing.
All existing tests pass without modification.

Backwards Compatibility
This is a non-breaking bugfix. The change introduces a safe fallback for missing data without altering the intended behavior when the colPos is provided.

Related Issues
No related issue has been linked to this PR yet.

@NamelessCoder
Copy link
Member

@rtfirst Thanks for the patch - but I need to understand exactly how it happens that a record passed to this function does not contain a colPos column. It simply should not be possible, as this method will never be called with anything but a tt_content record which would always have the column.

Another aspect is that it's probably better to simply early-return from the method if the colPos is any value type other than an integer with a value above 100 (the minimum value required to classify the record as actually being a child of a parent). If it isn't a >100 integer then the parent will never be resolved and there's no need to even attempt to fetch a record.

...but I would still like to understand the precise context. If this happens with a record that actually is a child of a parent then assuming colPos=0 could cause undesirable side effects.

@rtfirst
Copy link
Author

rtfirst commented Feb 27, 2025

In short, the issue occurred in the backend when I attempted to access the page via "history/undo" for a page that utilizes a grid layout with multiple content-elements. During this process, I encountered the following error message in backend:

(1/1) TypeError
FluidTYPO3\Flux\Utility\ColumnNumberUtility::calculateParentUid(): Argument #1 ($colPos) must be of type int, null given, called in /app/vendor/fluidtypo3/flux/Classes/Integration/HookSubscribers/ColumnPositionItems.php on line 30

I wasn't entirely certain about the problem since the page still contains elements, and they are visible on the history page. I have not observed any other side effects so far.

I hope I have been able to help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants