Project

General

Profile

Actions
Copy link

Bug #102303

closed

Empty radio elements are saved as "on"

Added by Robert Vock 6 months ago. Updated 5 months ago.

Status:
Resolved
Priority:
Should have
Assignee:
-
Category:
FormEngine aka TCEforms
Target version:
-
Start date:
2023-11-02
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
12
PHP Version:
Tags:
Complexity:
Is Regression:
Yes
Sprint Focus:

Description

When the radio (or select) entry contains an empty value, this value will be saved as "on" in TYPO3 v11. In TYPO3 v10 it was the empty string.

Example Flexform-Configuration:

<settings.size>
    <TCEforms>
        <label>Größe</label>
        <config>
            <type>radio</type>
            <items>
                <numIndex index="0">
                    <numIndex index="0">Groß</numIndex>
                    <numIndex index="1"></numIndex>
                </numIndex>
                <numIndex index="1">
                    <numIndex index="0">Mittel</numIndex>
                    <numIndex index="1">medium</numIndex>
                </numIndex>
                <numIndex index="2">
                    <numIndex index="0">Klein</numIndex>
                    <numIndex index="1">small</numIndex>
                </numIndex>
            </items>
        </config>
    </TCEforms>
</settings.size>

If the user chooses the first option (Groß), then "on" is saved and when the form is presented again, no option is selected.
When a class or template checks, if settings.size is empty, it would return true in TYPO3 v10 and returns false in TYPO3 v11, because "on" is saved.

The error was introduced in #91787
https://github.com/TYPO3/typo3/commit/757f82d1240497500165435c1db7bdf9e15a48a7#diff-d0d979ea8e3e22f10cbeb8385f2c3812d775ca558f4de30a4b22bf6ce7e0e017

Instead of building the attributes itself, the method now uses GeneralUtility::implodeAttributes, but without the keepBlankAttributes=true parameter. This leads to empty value-attributes being dropped and the browser saves radio-buttons without a value attribute as "on".

The fix would be easy: Add the keepBlankAttributes=true parameter.

Actions
Copy link
 #1

Updated by Garvin Hicking 6 months ago

Good catch. It actually seems to occur in other formEngine fields too. I'm on it preparing a patch.

Actions
Copy link
 #2

Updated by Gerrit Code Review 6 months ago

  • Status changed from New to Under Review

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

Actions
Copy link
 #3

Updated by Gerrit Code Review 6 months ago

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

Actions
Copy link
 #4

Updated by Garvin Hicking 6 months ago

The patch in https://review.typo3.org/c/Packages/TYPO3.CMS/+/81645 contains tests based on the styleguide basic TCA elemements.

  • Create an element of that type ("Form engine elements - input, text, checkbox, radio, none, passthrough, user")
  • Go to tab "in flex" then "radio" and try to set the empty value for "radio (empty)"
  • Go to tab "radio" and for radio_4 try to set the element with empty value.
Without patch (only apply the patches to styleguide to test this):
  • Setting radio_4 to "nofoo" is not saved. Remains on previous value.
  • Setting flex_1.radio_2 is not saved. No radio buttons remain checked.
With patch:
  • Both "empty" values get properly saved.
Actions
Copy link
 #5

Updated by Gerrit Code Review 6 months ago

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

Actions
Copy link
 #6

Updated by Gerrit Code Review 6 months ago

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

Actions
Copy link
 #7

Updated by Gerrit Code Review 6 months ago

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

Actions
Copy link
 #8

Updated by Gerrit Code Review 6 months ago

Patch set 6 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/81645

Actions
Copy link
 #9

Updated by Gerrit Code Review 6 months ago

Patch set 7 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/81645

Actions
Copy link
 #10

Updated by Gerrit Code Review 6 months ago

Patch set 8 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/81645

Actions
Copy link
 #11

Updated by Christian Kuhn 6 months ago

Did this really work in v10? If so, which patch broke it?

Actions
Copy link
 #13

Updated by Garvin Hicking 6 months ago

Thanks Robert for checking this out, one item less on my agenda for today 🥳
Did you by chance try out the patch and does it work for you properly?

Actions
Copy link
 #14

Updated by Robert Vock 6 months ago

@Garvin Hicking The patch is working. (I only tested Checkboxes and Radios)

We applied a similar patch using Composer-Patches, which just sets the third argument to implodeAttributes to true for Checkboxes and Radios and are using it in production.

It's only required for Checkboxes and Radio-Buttons, because those form elements save empty values as "on". But it does not hurt for other form elements and is more consistent, if applied everywhere.

Actions
Copy link
 #15

Updated by Gerrit Code Review 5 months ago

Patch set 9 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/81645

Actions
Copy link
 #16

Updated by Gerrit Code Review 5 months ago

Patch set 10 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/81645

Actions
Copy link
 #17

Updated by Gerrit Code Review 5 months ago

Patch set 11 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/81645

Actions
Copy link
 #18

Updated by Gerrit Code Review 5 months ago

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

Actions
Copy link
 #19

Updated by Gerrit Code Review 5 months ago

Patch set 12 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/81645

Actions
Copy link
 #20

Updated by Garvin Hicking 5 months ago

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

Updated by Gerrit Code Review 5 months ago

  • Status changed from Resolved to Under Review

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

Actions
Copy link
 #22

Updated by Gerrit Code Review 5 months ago

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

Actions
Copy link
 #23

Updated by Garvin Hicking 5 months ago

  • Status changed from Under Review to Resolved
Actions
Copy link

Also available in: Atom PDF