Bug #69957
closedEpic #69955: Optimize new Extbase/ Fluid based rewrite of EXT:form
EXT:form - Fix some problems with Container elements
Added by Björn Jacob about 9 years ago. Updated almost 7 years ago.
100%
Description
There are some findings on RADIOGROUP and OPTIONGROUP.
- values are not visualized on the confirmAction view
- after the validation prohibited to go from showAction to confirmAction, all options of the option-group are checked/activated, for the radio-group, the selected value is not updated, it uses the default value defined in TypoScript
enctype = multipart/form-data method = post prefix = tx_form confirmation = 1 postProcessor { 1 = mail 1 { layout { label = <b><em><labelvalue /></em></b> } senderEmail = foo@test.com recipientEmail = foo@test.com } } 10 = HEADER 10 { class = content-header headingSize = h1 content = Some form... } 20 = FIELDSET 20 { legend { value = Yay! } 10 = TEXTLINE 10 { name = firstName placeholder = First name label { value = First Name } } 20 = TEXTLINE 20 { name = lastName placeholder = Last name label { value = Last Name } } 30 = TEXTLINE 30 { name = email placeholder = oliver@hader.name label { value = E-Mail } } 40 = CHECKBOXGROUP 40 { class = fieldset-subgroup legend { value = Preference } name = preference 10 = CHECKBOX 10 { label { value = Option 1 } } 20 = CHECKBOX 20 { label { value = Option 2 } } 30 = CHECKBOX 30 { label { value = Option 3 } } } 50 = RADIOGROUP 50 { class = fieldset-subgroup legend { value = Choice } name = choice 10 = RADIO 10 { label { value = Option 1 } } 20 = RADIO 20 { label { value = Option 2 } } 30 = RADIO 30 { checked = checked label { value = Option 3 } } } } 30 = SUBMIT 30 { name = submit value = Submit } rules { 1 = required 1 { breakOnError = 0 showMessage = 1 message = Required error = This field is required element = email } 2 = email 2 { breakOnError = 0 showMessage = 1 message = (john.doe@domain.com) error = This is not a valid email address element = email } }
Updated by Björn Jacob about 9 years ago
The problem has already been analyzed and we're working on it.
It's a combination of 3 problems. The checkboxes have no value property which is not a valid configuration. All of the 3 checkboxes are generated with no values and this produces an error. We have to handle incorrect configurations. In addition the attribute overlay stuff in the element builder needs some optimization.
Updated by Gerrit Code Review almost 9 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 http://review.typo3.org/43452
Updated by Ralf Zimmermann almost 9 years ago
- Status changed from Under Review to New
The patchset fix the following things:
There was 4 rendering functions, which tries to render a message as cObject
if allowed.
TYPO3\CMS\Form\Controller\FrontendController::renderConfirmationMessage TYPO3\CMS\Form\Domain\Builder\ElementBuilder::renderAttributeValue TYPO3\CMS\Form\Domain\Validator\AbstractValidator::renderMessage TYPO3\CMS\Form\PostProcess\MailPostProcessor::renderMessage
But each of them a little bit different.
I unified this 4 functions and put a centralized content rendering method
in the
TYPO3\CMS\Form\Utility\FormUtility::renderContentObject.
They handle each possible data input variant.
The attribute overlay methods in
TYPO3\CMS\Form\Domain\Builder\ElementBuilder::overlayUserdefinedHtmlAttributeValues
and
TYPO3\CMS\Form\Domain\Builder\ElementBuilder::moveAllOtherUserdefinedPropertiesToAdditionalArguments
was not complete.
This is the new program flow:
- set a attribute based on the typoscript model settings
- if exist this attribute in the user configured typoscript
- yes
- if exist a renaming for this attribute
- yes
- if exist already the renamed attribute in the user configured typoscript
- yes
- render the new named attribute value from the user configured typoscript and set it with the new name
- no
- render the original named attribute value from the user configured typoscript and set it with the new name
- yes
- if exist already the renamed attribute in the user configured typoscript
- no
- render the attribute value from the user configured typoscript
- yes
- if exist a renaming for this attribute
- no
- render the model setting based value
- yes
When programming on Friday night a few careless mistakes happened to me.
There some bugs in the partials which will be fixed with the patchset.
Without this fixed it will result in empty html ond / or plaintext parts of the mail,
depending on the comptibility mode.
Updated by Ralf Zimmermann almost 9 years ago
- Status changed from New to Under Review
- Assignee deleted (
Ralf Zimmermann)
Updated by Ralf Zimmermann almost 9 years ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset a81bd7977d41c5f7d2007b6ceaf89fe02f69bf2a.
Updated by Björn Jacob almost 9 years ago
- Status changed from Resolved to Under Review
- % Done changed from 100 to 0
This is not true. The ticket is not solved and nothing was applied to master IMHO. Still open and under review.
Updated by Gerrit Code Review almost 9 years ago
Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43470
Updated by Gerrit Code Review almost 9 years ago
Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43471
Updated by Gerrit Code Review almost 9 years ago
Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43471
Updated by Gerrit Code Review almost 9 years ago
Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43471
Updated by Ralf Zimmermann almost 9 years ago
- Subject changed from Fix some problems with Container elements to EXT:form - Fix some problems with Container elements
Updated by Gerrit Code Review almost 9 years ago
Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43471
Updated by Gerrit Code Review almost 9 years ago
Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43471
Updated by Gerrit Code Review almost 9 years ago
Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43471
Updated by Ralf Zimmermann almost 9 years ago
- Status changed from Under Review to In Progress
Yes i know, is a ugly big patchset. But the most changes have depencies to others. I want to describe the parts and how to test it.
Updated by Gerrit Code Review almost 9 years ago
- Status changed from In Progress to Under Review
Patch set 7 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43471
Updated by Ralf Zimmermann almost 9 years ago
- Priority changed from Should have to Must have
The patchset fix the following things:
1.
There was 4 rendering functions, which tries to render a message or attribute as cObject
if allowed.
TYPO3\CMS\Form\Controller\FrontendController::renderConfirmationMessage TYPO3\CMS\Form\Domain\Builder\ElementBuilder::renderAttributeValue TYPO3\CMS\Form\Domain\Validator\AbstractValidator::renderMessage TYPO3\CMS\Form\PostProcess\MailPostProcessor::renderMessage
But each of them a little bit different.
I unified this 4 functions and put a centralized item rendering method
in the
TYPO3\CMS\Form\Utility\FormUtility::renderItem.
They handle each possible data input variant.
Affected Parts:
- TYPO3\CMS\Form\Controller\FrontendController
- TYPO3\CMS\Form\Domain\Builder\ElementBuilder
- TYPO3\CMS\Form\Domain\Validator\AbstractValidator
- TYPO3\CMS\Form\PostProcess\MailPostProcessor
- TYPO3\CMS\Form\Utility\FormUtility::renderItem
Test:
If the form is rendered by typoscript you can make each setting in a form element to a cObject.
A content element whithin a form typoscript must be displayed (if allowed)
#########
2.
https://forge.typo3.org/issues/69369
A function rename attributes for future use.
But the attribute overlay methods in
TYPO3\CMS\Form\Domain\Builder\ElementBuilder::overlayUserdefinedHtmlAttributeValues
and
TYPO3\CMS\Form\Domain\Builder\ElementBuilder::moveAllOtherUserdefinedPropertiesToAdditionalArguments
was not complete.
This is the new program flow:
- set a attribute based on the typoscript model settings
- if exist this attribute in the user configured typoscript
- yes
- if exist a renaming for this attribute
- yes
- if exist already the renamed attribute in the user configured typoscript
- yes
- render the new named attribute value from the user configured typoscript and set it with the new name
- no
- render the original named attribute value from the user configured typoscript and set it with the new name
- yes
- if exist already the renamed attribute in the user configured typoscript
- no
- render the attribute value from the user configured typoscript
- yes
- if exist a renaming for this attribute
- no
- render the model setting based value
- yes
Affected Parts:
- TYPO3\CMS\Form\Domain\Builder\ElementBuilder
- the changed parts in the methods overlayUserdefinedHtmlAttributeValues and moveAllOtherUserdefinedPropertiesToAdditionalArguments
- TYPO3\CMS\Form\Utility\CompatibilityLayerUtility
- the method getNewAttributeName
- TYPO3\CMS\Form\Hooks\HandleIncomingFormValues
- the TEXTAREA part from line 129
- form/Resources/Private/Partials/Compatibility/Confirmation/FlatElements/Textarea.html
- form/Resources/Private/Partials/Compatibility/PostProcessor/Mail/Html/FlatElements/Textarea.html
- form/Resources/Private/Partials/Compatibility/PostProcessor/Mail/Plain/FlatElements/Textarea.html
- form/Resources/Private/Partials/Compatibility/Show/FlatElements/Textarea.html
- form/Resources/Private/Partials/Compatibility/Show/FlatElements/Textblock.html
- form/Resources/Private/Partials/Default/Confirmation/FlatElements/Textarea.html
- form/Resources/Private/Partials/Default/PostProcessor/Mail/Html/FlatElements/Textarea.html
- form/Resources/Private/Partials/Default/PostProcessor/Mail/Plain/FlatElements/Textarea.html
- form/Resources/Private/Partials/Default/Show/FlatElements/Textarea.html
- form/Resources/Private/Partials/Default/Show/FlatElements/Textblock.html
Test:
- compatibility mode = on
- define OPTION.text
- text will be displayed as option label
- define OPTION.data
- data will be displayed as option label
- define both
- text will be displayed as option label
- define OPTION.text
- compatibility mode = off
- define OPTION.text
- text will be displayed as option label
- define OPTION.data
- nothing will be displayed as label
- define both
- text will be displayed as option label
- define OPTION.text
Try the same for TEXTBLOCK (content) and TEXTAREA (data)
Updated by Gerrit Code Review almost 9 years ago
Patch set 8 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43471
Updated by Gerrit Code Review almost 9 years ago
Patch set 9 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43471
Updated by Gerrit Code Review almost 9 years ago
Patch set 10 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43471
Updated by Gerrit Code Review almost 9 years ago
Patch set 11 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43471
Updated by Benni Mack almost 9 years ago
- Sprint Focus set to Stabilization Sprint
Updated by Gerrit Code Review almost 9 years ago
Patch set 12 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43471
Updated by Gerrit Code Review almost 9 years ago
Patch set 13 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43471
Updated by Gerrit Code Review almost 9 years ago
Patch set 14 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43471
Updated by Gerrit Code Review almost 9 years ago
Patch set 15 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43471
Updated by Gerrit Code Review almost 9 years ago
Patch set 16 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43471
Updated by Gerrit Code Review almost 9 years ago
Patch set 17 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43471
Updated by Ralf Zimmermann almost 9 years ago
Here is some typoscript the test this patchset:
lib.standardForm = COA lib.standardForm { 10 = FORM 10 { confirmation = 1 prefix = standardForm compatibilityMode = 1 postProcessor { 1 = mail 1 { senderEmailField = email senderNameField = surname subjectField = subject recipientEmail = x@domain.de ccEmail = y@domain.de priority = 1 organization = TRITUM } } 10 = COA 10 { 10 = TEXT 10 { value = show me wrap = <b>|</b> } } 20 = TEXTLINE 20 { name = email label = TEXT label { value = show me again wrap = <b>|</b> } } 30 = SELECT 30 { label = Select multiple = 1 name = selectbox 10 = OPTION 10 { data = Option 1 value = value 1 text = value 1 new } 20 = OPTION 20 { data = Option 2 value = value 2 text = value 2 new } } 40 = TEXTAREA 40 { data = testcontent text = testcontent new } 50 = TEXTBLOCK 50 { content = testcontent text = testcontent new } 100 = SUBMIT 100 { name = submit value = Submit } } }
Tests:
Without patch (rendered from typoscript):
- the cObject at "10." are visible (already ok)
- the cObject label from "20." are visible (already ok)
- the labels from the selectbox are not visible (bug)
- the the content from the "data" property from "40." is visible (ok, but we want to rename data to text)
- the the content from the "content" property from "50." is not visible (bug)
Without patch (rendered from backend - wizard):
- the cObject at "10." are not visible (already ok)
- the "value" property from the label from "20." are visible, but not rendered as cObject (already ok)
- the labels from the selectbox are not visible (bug)
- the the content from the "data" property from "40." is visible (ok, but we want to rename data to text)
- the the content from the "content" property from "50." is not visible (bug)
With the patch (rendered from typoscript)
- the cObject at "10." are visible
- the cObject label from "20." are visible
- the labels from the selectbox are visible
- the the content from the "data" property from "40." is visible - if "text" is not set
- the the content from the "text" property from "40." is visible - no matter if "data" is set
- the the content from the "data" property from "50." is visible - if "text" is not set
- the the content from the "text" property from "50." is visible - no matter if "content" is set
With the patch (rendered from backend - wizard):
- the cObject at "10." are not visible
- the "value" property from the label from "20." are visible, but not rendered as cObject
- the labels from the selectbox are visible
- the the content from the "data" property from "40." is visible - if "text" is not set
- the the content from the "text" property from "40." is visible - no matter if "data" is set
- the the content from the "data" property from "50." is visible - if "text" is not set
- the the content from the "text" property from "50." is visible - no matter if "content" is set
If compatibility mode is off, the "data" property from OPTION and TEXTAREA and the "content" property from
TEXTBLOCK are ignored and all 3 elements use the "text" property.
Please note also comment #69957#note-18
Updated by Gerrit Code Review almost 9 years ago
Patch set 18 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43471
Updated by Gerrit Code Review almost 9 years ago
Patch set 19 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43471
Updated by Gerrit Code Review almost 9 years ago
Patch set 20 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43471
Updated by Gerrit Code Review almost 9 years ago
Patch set 21 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43471
Updated by Gerrit Code Review almost 9 years ago
Patch set 22 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43471
Updated by Gerrit Code Review almost 9 years ago
Patch set 23 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43471
Updated by Ralf Zimmermann almost 9 years ago
- Status changed from Under Review to Resolved
Applied in changeset 281bce23c0d6d3ce914d945ee89811dfdb51af30.
Updated by Riccardo De Contardi almost 7 years ago
- Status changed from Resolved to Closed