Project

General

Profile

Actions

Bug #70957

closed

Story #69617: FormEngine bugs

CE textmedia doesn't show correct palettes in media IRRE

Added by Frans Saris over 8 years ago. Updated over 6 years ago.

Status:
Closed
Priority:
Should have
Category:
-
Target version:
Start date:
2015-10-22
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
7
PHP Version:
Tags:
Complexity:
Is Regression:
No
Sprint Focus:

Description

The media IRRE elements do not show correct palettes when opening the IRRE element after pageload

Actions #1

Updated by Gerrit Code Review over 8 years ago

  • Status changed from New to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/44228

Actions #2

Updated by Christian Kuhn over 8 years ago

background information (slack quote):

@Benni Mack / @franssaris - i need your help & thoughts on the fluid_styled_content tt_content media field configuration. please contact me. here is the situation: the media field is configured as "normal" fal field in ext:frontent for usage within the uploads ctype. for fluid_styled_content ctype textmadia, it also holds the list of connected media elemens as fal relations. problem is, the fal definition must be slightly different, especially you want to have "foreign_selector_fieldTcaOverrides". To not override this for all types (to not harm uploads ctype media field usage), the fluid media field fal configuration is changed as "columnsOverrides" for the textmedia cType only (ext:fluid_styled_content/Configuration/TCA/Overrides/tt_content.php) this foreign_selector_fieldTcaOverride basically changes the TCA configuration of the child TCA of the field the inline foreign_selector points to. So, you are overriding child TCA depending on the type of the parent record by doing that in columnsOverrides. this is unfortunate for two reasons, and leads to the effect that some input fields for textmedia elements are missing if adding or expanding via ajax. everything is ok after safe:
currently, if "adding" a new child to an inline parent via ajax, the parent's record is not fetched at all (this is much quicker), so the columnsOverride of the parent type can not kick in and the inline configuration is the "base" config. i could actually fix this by fetching the parent record within the ajax call again and let it determine its type to then apply columnsOverride and end up with the overriden inline configuration for the child calculation, but:
if adding a new parent tt_content record that is not yet persisted, then there is no record yet and the type determination can not happen if a new media element should be added via ajax. so again missing fields if adding a new media child on a new (!) tt_content parent. one way to fix this is to have "textmedia" set as default type, this is actually defined as such at the moment and if fetching the parent record as outlined above, it would solve the issue. but, the solution is inflexible and hacky, it wouldn't work with columnsOverrides on two different types for instance. Another solution is to transfer the given parent record type via JS to the ajax handler and do the columnsOverrides stuff based on that. This solution however doesn't feel clean, either.
I would rather like to get rid of this problem elsewhere. Having child TCA depending on the record type of a parent has a complexity level of "nightmare" anyway and I'd love to not have this dependency. I'd like to better have some statement like "never override inline stuff within columnsOverrides". one possible solution for the fluid/tt_content/textmedia/media would be to not re-use the media field for both textmedia ctype and uploads ctype, but to have two separate fields. what was the reason for this re-use in the first place? could this maybe be changed again?
what do you think on all this?

Actions #3

Updated by Christian Kuhn over 8 years ago

frans and me aligned on the following:

we came to the conclusion that splitting the field into two fields would probably be better: easier, no columnsOverrides at all anymore, easier to maintain, no risk for side effects. and: if we change this ​*now*​, since all this stuff was implemented so recently, we don't even need an upgrade wizard. i guess existing relations would actually survive and only the reference counter would suffer?

TASK:
ext_tables.sql, some tca simplification and a breaking.rst showing some sql magic. and thats it. what do you think?

  • find name and add new field to tt_content in ext_tables.sql of ext:fluid_styled_content (edited)
  • use this field in text+media element in ext:fluid_styled_content (edited)
  • get rid of columnsOverrides of media field in ext:fluid_styled_content (edited)
  • adjust typo script of ce textmedia
  • hack up tiny upgrade wizard that searches all rows in sys_file_reference that point to a text+media ext:fluid_styled_content content element and that have "media" set in field "fieldname" of sys_file_reference. change this field entry for affected rows to the "new" name, OR alternatively: Write an "breaking" .rst file that points out what upgraders from 7.5 to LTS need to do. (edited)

Christian Kuhn [5:59 PM]
imho, an important .rst would be ok for me, since the group of people running current 7.5 WITH fluid_styled_content installed is probably rather smallish. to be nice, we may add some example sql to at least find affected rows? what do you think? would that be ok for you? (edited)

Frans Saris [6:25 PM]
Sounds correct, big challenge is the name for the new field. :wink:
Upgrade wizard is easy, just copy/adjust the textmedia migration wizard
Could you add this to the ticket?

Christian Kuhn [6:31 PM]
yeah, will do.

Actions #4

Updated by Christian Kuhn over 8 years ago

  • Parent task set to #69617
Actions #5

Updated by Christian Kuhn over 8 years ago

seems as if morton can have a look at this :)

Actions #6

Updated by Morton Jonuschat over 8 years ago

  • Assignee set to Morton Jonuschat
Actions #7

Updated by Gerrit Code Review over 8 years ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/44228

Actions #8

Updated by Gerrit Code Review over 8 years ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/44228

Actions #9

Updated by Gerrit Code Review over 8 years ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/44228

Actions #10

Updated by Gerrit Code Review over 8 years ago

Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/44228

Actions #11

Updated by Morton Jonuschat over 8 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
Actions #12

Updated by Riccardo De Contardi over 6 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF