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] #42: exception when editing a tag or author #43

Merged
merged 2 commits into from
Jun 25, 2019

Conversation

dmitryd
Copy link
Contributor

@dmitryd dmitryd commented Mar 18, 2019

Fixes #42 by removing wrong displayCond.

@benjaminkott
Copy link
Member

benjaminkott commented Apr 29, 2019

@NeoBlack I need your opinion on this one. I could reproduce the issue if I add a storage folder outside the root and then add authors etc to it. Opening the record again will produce the error @dmitryd described. I am not sure if we should handle this at all, because I am unaware of possible side-effects, with duplicate slug entries for example.

This does not happen if you add categories to the root itself, but slug fields are not generated correctly.

@NeoBlack
Copy link
Member

As described in the issue, the display condition is for pages only, so for me, it was a mistake to use this display condition for tags and categories.
I guess we should accept this PR and if other errors occur we have to think about a solution, e.g. with an own display condition class.

@benjaminkott
Copy link
Member

benjaminkott commented Apr 29, 2019

@NeoBlack I´ve may found an issue in the PseudoSiteTcaDisplayCondition it fails for all not translatable records (without l10n_parent) that are out of a site. I´ve marked the point where it fails. https://github.com/TYPO3/TYPO3.CMS/blob/master/typo3/sysext/core/Classes/Compatibility/PseudoSiteTcaDisplayCondition.php#L61-L66

@dmitryd
Copy link
Contributor Author

dmitryd commented May 5, 2019

@benjaminkott

I am not sure if we should handle this at all, because I am unaware of possible side-effects, with duplicate slug entries for example.

You should because:

  • displayCond is for pages only
  • having a blog folder outside of the root is a use case for several sites sharing the same blog entries

@benjaminkott benjaminkott merged commit 886cf1e into TYPO3GmbH:v9.1 Jun 25, 2019
@benjaminkott
Copy link
Member

I´ve thought a lot about this issue, tested a lot and talked a lot about with different persons. Removing the condition seems to be the best option we have after all. It does not make it worse or solved the initial intend. But it´s at least avoiding the exception. We will reconsider it when we have a better solution.

benjaminkott pushed a commit that referenced this pull request Jun 25, 2019
benjaminkott pushed a commit that referenced this pull request Jun 25, 2019
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.

3 participants