Bug #69957

Epic #69955: Optimize new Extbase/ Fluid based rewrite of EXT:form

EXT:form - Fix some problems with Container elements

Added by Bjoern Jacob about 4 years ago. Updated about 2 years ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
Form Framework
Target version:
Start date:
2015-09-24
Due date:
% Done:

100%

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

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
    }
}


Subtasks

Bug #70091: EXT:form - Typos in the partialsClosed

Bug #70095: EXT:form - Wrong Checkbox and Radiobutton handlingClosed


Related issues

Related to TYPO3 Core - Bug #69971: EXT:form Error at Mail sending / ObjectStorage could not be converted to string Closed 2015-09-19
Related to TYPO3 Core - Task #69369: EXT:form - Use property value instead of data for TEXTAREA, TEXTBLOCK, OPTION Closed 2015-08-27

Associated revisions

Revision a81bd797 (diff)
Added by Ralf Zimmermann about 4 years ago

[BUGFIX] EXT:form - wrong attribute handling

  • centralize the attribute rendering method.
  • make the attribute overlay methods more complete
  • put the container elements foreach within the f:else branch

Resolves: #69957
Releases: master
Change-Id: I7c1f067d23dbd8a5bab4cfd8afa7d70a3e38585a

Revision 281bce23 (diff)
Added by Ralf Zimmermann about 4 years ago

[BUGFIX] EXT:form - wrong attribute handling

  • centralize the attribute rendering method
  • fix the attribute overlay methods
    • fix: "select" labels remains empty
    • fix: "textblock" remains empty

Resolves: #69957
Releases: master
Change-Id: I60554dbdf1eb8a25b9dfad820a2bc7bd20704199
Reviewed-on: http://review.typo3.org/43471
Reviewed-by: Frans Saris <>
Tested-by: Frans Saris <>
Reviewed-by: Georg Ringer <>
Tested-by: Georg Ringer <>

History

#1 Updated by Bjoern Jacob about 4 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.

#2 Updated by Bjoern Jacob about 4 years ago

  • Target version set to 7.5

#3 Updated by Gerrit Code Review about 4 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

#4 Updated by Ralf Zimmermann about 4 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
        • no
          • render the attribute value from the user configured typoscript
    • no
      • render the model setting based value

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.

#5 Updated by Ralf Zimmermann about 4 years ago

  • Status changed from New to Under Review
  • Assignee deleted (Ralf Zimmermann)

#6 Updated by Ralf Zimmermann about 4 years ago

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

#7 Updated by Bjoern Jacob about 4 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.

#8 Updated by Gerrit Code Review about 4 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

#9 Updated by Gerrit Code Review about 4 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

#10 Updated by Gerrit Code Review about 4 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

#11 Updated by Gerrit Code Review about 4 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

#12 Updated by Ralf Zimmermann about 4 years ago

  • Subject changed from Fix some problems with Container elements to EXT:form - Fix some problems with Container elements

#13 Updated by Gerrit Code Review about 4 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

#14 Updated by Gerrit Code Review about 4 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

#15 Updated by Gerrit Code Review about 4 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

#16 Updated by Ralf Zimmermann about 4 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.

#17 Updated by Gerrit Code Review about 4 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

#18 Updated by Ralf Zimmermann about 4 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
        • no
          • render the attribute value from the user configured typoscript
    • no
      • render the model setting based value

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
  • 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

Try the same for TEXTBLOCK (content) and TEXTAREA (data)

#19 Updated by Gerrit Code Review about 4 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

#20 Updated by Gerrit Code Review about 4 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

#21 Updated by Gerrit Code Review about 4 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

#22 Updated by Gerrit Code Review about 4 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

#23 Updated by Benni Mack about 4 years ago

  • Sprint Focus set to Stabilization Sprint

#24 Updated by Gerrit Code Review about 4 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

#25 Updated by Gerrit Code Review about 4 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

#26 Updated by Gerrit Code Review about 4 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

#27 Updated by Gerrit Code Review about 4 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

#28 Updated by Gerrit Code Review about 4 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

#29 Updated by Gerrit Code Review about 4 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

#30 Updated by Ralf Zimmermann about 4 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

#31 Updated by Gerrit Code Review about 4 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

#32 Updated by Gerrit Code Review about 4 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

#33 Updated by Gerrit Code Review about 4 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

#34 Updated by Gerrit Code Review about 4 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

#35 Updated by Gerrit Code Review about 4 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

#36 Updated by Gerrit Code Review about 4 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

#37 Updated by Ralf Zimmermann about 4 years ago

  • Status changed from Under Review to Resolved

#38 Updated by Riccardo De Contardi about 2 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF