Project

General

Profile

Actions
Copy link

Bug #69957

closed

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

EXT:form - Fix some problems with Container elements

Added by Björn Jacob over 8 years ago. Updated over 6 years ago.

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

100%

Estimated time:
(Total: 0.00 h)
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 2 (0 open — 2 closed)

Bug #70091: EXT:form - Typos in the partialsClosed2015-09-24

Actions
Bug #70095: EXT:form - Wrong Checkbox and Radiobutton handlingClosed2015-09-24

Actions

Related issues 2 (0 open — 2 closed)

Related to TYPO3 Core - Bug #69971: EXT:form Error at Mail sending / ObjectStorage could not be converted to stringClosedBjörn Jacob2015-09-19

Actions
Related to TYPO3 Core - Task #69369: EXT:form - Use property value instead of data for TEXTAREA, TEXTBLOCK, OPTIONClosed2015-08-27

Actions
Actions
Copy link
 #1

Updated by Björn Jacob over 8 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.

Actions
Copy link
 #2

Updated by Björn Jacob over 8 years ago

  • Target version set to 7.5
Actions
Copy link
 #3

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 http://review.typo3.org/43452

Actions
Copy link
 #4

Updated by Ralf Zimmermann over 8 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.

Actions
Copy link
 #5

Updated by Ralf Zimmermann over 8 years ago

  • Status changed from New to Under Review
  • Assignee deleted (Ralf Zimmermann)
Actions
Copy link
 #6

Updated by Ralf Zimmermann over 8 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
Actions
Copy link
 #7

Updated by Björn Jacob over 8 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.

Actions
Copy link
 #8

Updated by Gerrit Code Review over 8 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

Actions
Copy link
 #9

Updated by Gerrit Code Review over 8 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

Actions
Copy link
 #10

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 http://review.typo3.org/43471

Actions
Copy link
 #11

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 http://review.typo3.org/43471

Actions
Copy link
 #12

Updated by Ralf Zimmermann over 8 years ago

  • Subject changed from Fix some problems with Container elements to EXT:form - Fix some problems with Container elements
Actions
Copy link
 #13

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 http://review.typo3.org/43471

Actions
Copy link
 #14

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 http://review.typo3.org/43471

Actions
Copy link
 #15

Updated by Gerrit Code Review over 8 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

Actions
Copy link
 #16

Updated by Ralf Zimmermann over 8 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.

Actions
Copy link
 #17

Updated by Gerrit Code Review over 8 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

Actions
Copy link
 #18

Updated by Ralf Zimmermann over 8 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)

Actions
Copy link
 #19

Updated by Gerrit Code Review over 8 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

Actions
Copy link
 #20

Updated by Gerrit Code Review over 8 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

Actions
Copy link
 #21

Updated by Gerrit Code Review over 8 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

Actions
Copy link
 #22

Updated by Gerrit Code Review over 8 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

Actions
Copy link
 #23

Updated by Benni Mack over 8 years ago

  • Sprint Focus set to Stabilization Sprint
Actions
Copy link
 #24

Updated by Gerrit Code Review over 8 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

Actions
Copy link
 #25

Updated by Gerrit Code Review over 8 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

Actions
Copy link
 #26

Updated by Gerrit Code Review over 8 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

Actions
Copy link
 #27

Updated by Gerrit Code Review over 8 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

Actions
Copy link
 #28

Updated by Gerrit Code Review over 8 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

Actions
Copy link
 #29

Updated by Gerrit Code Review over 8 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

Actions
Copy link
 #30

Updated by Ralf Zimmermann over 8 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

Actions
Copy link
 #31

Updated by Gerrit Code Review over 8 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

Actions
Copy link
 #32

Updated by Gerrit Code Review over 8 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

Actions
Copy link
 #33

Updated by Gerrit Code Review over 8 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

Actions
Copy link
 #34

Updated by Gerrit Code Review over 8 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

Actions
Copy link
 #35

Updated by Gerrit Code Review over 8 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

Actions
Copy link
 #36

Updated by Gerrit Code Review over 8 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

Actions
Copy link
 #37

Updated by Ralf Zimmermann over 8 years ago

  • Status changed from Under Review to Resolved
Actions
Copy link
 #38

Updated by Riccardo De Contardi over 6 years ago

  • Status changed from Resolved to Closed
Actions
Copy link

Also available in: Atom PDF