Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

standardize fieldnames #8403

Closed
timgatzky opened this issue Jul 11, 2016 · 35 comments
Closed

standardize fieldnames #8403

timgatzky opened this issue Jul 11, 2016 · 35 comments
Assignees
Labels
Milestone

Comments

@timgatzky
Copy link

Hi,
field names should be standardized on save by default to avoid spaces or any letter that might cause trouble later on.

@discordier
Copy link
Contributor

I have no idea what you mean. Can you post a real world code example?

@ADoebeling
Copy link

Maybe @timgatzky was talking about "file names", uploaded by the file manager?

@timgatzky
Copy link
Author

timgatzky commented Jul 12, 2016

regular field names. Example might be over the top. But I always get requests why forms won't send and at the end it's special characters. all contao does is replacing spaces with underscore.
I would suggest we run a standardize(...) in the save_callback. Just like generating alias for acticles etc.

Screenshot

@timgatzky
Copy link
Author

I give a +1 to file names also!

@ADoebeling
Copy link

Ahh, you're talking about the form generator. Yes, that seems to be a bug, even if I didn't tried to reproduce.

@Aybee
Copy link
Contributor

Aybee commented Jul 12, 2016

👍 fieldnames and filenames
But I think we should accept the underscore.

@fritzmg
Copy link
Contributor

fritzmg commented Jul 14, 2016

See also #8400

@timgatzky
Copy link
Author

jup. refers to the same problem. standardize field names on save and all be fine :)

@leofeyer leofeyer added this to the 3.5.15 milestone Jul 14, 2016
@leofeyer
Copy link
Member

I guess we also need [] for array fields, so I think it should be:

A-Za-z0-9[]_-

@contao/developers Which characters should we allow?

@aschempp
Copy link
Member

do we really support manually creating array of fields? Does that currently work?

@Toflar
Copy link
Member

Toflar commented Jul 14, 2016

Just for the record: Full ASCII (so spaces too) are perfectly valid in a "name" attribute. Here's an excerpt of the HTML specification:

The "get" method restricts form data set values to ASCII characters. Only the "post" method (with enctype="multipart/form-data") is specified to cover the entire [ISO10646] character set.

But I'm fine with restricting it to alphanumerics plus _ and -.

@leofeyer
Copy link
Member

@aschempp Yes, and AFAIR you have added this feature. 😄

@leofeyer
Copy link
Member

@leofeyer
Copy link
Member

@Toflar You are right, but some browsers replace special characters such as dot or white space with _ and +.

@timgatzky
Copy link
Author

timgatzky commented Jul 14, 2016

Problems might not occur in the name attribute of the html element but in the POST/GET variables.
Since the field name will be the key selector for e.g. the POST variable ($_POST[...myfieldname...]) we should avoid any unregular chars including white spaces and brackets (maybe except multiple values in GET parameters).

@leofeyer leofeyer modified the milestones: 3.5.15, 3.5.16 Jul 15, 2016
@leofeyer
Copy link
Member

leofeyer commented Jul 21, 2016

As discussed in Mumble on July 21st, we want to add a new method Validator::isFieldName(), which allows A-Za-z0-9[]_-. Also, Widget::validator() needs to be extended with 'rgxp' => 'fieldname'.

@timgatzky
Copy link
Author

timgatzky commented Jul 22, 2016

[] should not be valid!

Incase you have 2 form fields:
In addition: Field names should be unique inside the form!

Screenshot

Frontend:
Screenshot

The second one will overwrite the value of the first one.

prints in $_POST
Screenshot

In POST variables the brackets will be ignored anyways. So leave that out. Brackets can only be valid for multiple GET parameters.

@ausi
Copy link
Member

ausi commented Jul 22, 2016

In POST variables the brackets will be ignored anyways. So leave that out. Brackets can only be valid for multiple GET parameters.

I don’t think that is true. I just tested it with this sample page:

<pre><?php var_export($_POST); ?></pre>
<form action="" method="post">
    <input name="foo[]" value="bar">
    <input name="foo[]" value="baz">
    <button type="submit">submit</button>
</form>

@timgatzky
Copy link
Author

timgatzky commented Jul 22, 2016

Sorry, wrong screenshot of the backend. Run your test with fields name like:

  1. myField[]
  2. myField

Anyways. Just run a standardize over the field name value in a save_callback for that field and all is good. I don't even see a need for a validation.

@leofeyer
Copy link
Member

Sorry, wrong screenshot of the backend. Run your test with a fields name like:

  1. myField[]
  2. myField

That's a configuration error IMHO.

@timgatzky
Copy link
Author

timgatzky commented Jul 22, 2016

From Contaos point that is valid. See this as an example. I think field names should not contain brackets and should be tested unique against other field names in the same form.

@leofeyer
Copy link
Member

-1

Because the following is a valid configuration:

  1. myField[0]
  2. myField[1]

And there is no reason not to support this.

@timgatzky
Copy link
Author

timgatzky commented Jul 22, 2016

myField[0]
myField[1]

Yes. that will work. It kinda scares me to see array keys like this. But true, that is a configuration thing

@discordier
Copy link
Contributor

We must keep the brackets, otherwise we will break every wizard field.

@aschempp
Copy link
Member

aschempp commented Jul 24, 2016 via email

@aschempp
Copy link
Member

aschempp commented Jul 24, 2016 via email

@Toflar
Copy link
Member

Toflar commented Jul 24, 2016 via email

@discordier
Copy link
Contributor

Should be fine for me, will have to wait for the final implementation, maybe I have other concerns then but currently it looks like it should work out fine.

@leofeyer
Copy link
Member

Fixed in 3222512.

jsonn pushed a commit to jsonn/pkgsrc that referenced this issue Sep 8, 2016
### 4.2.3 (2016-09-06)

 * Do not double URL encode the content syndication links.
 * Use CSS3 transforms instead of transitions to animate the off-canvas navigation.
 * Improve the exception handling when using the resource locator (see #557).
 * Correctly reset the filter menu in parent view.
 * Support all characters but =!<> and whitespace in simple tokens (see contao/core#8436).
 * Check the user's permission when generating links in the picker (see contao/core#8407).
 * Handle forward pages without target in the navigation modules (see contao/core#8377).
 * Provide the same template variables for downloads and enclosures (see contao/core#8392).
 * Handle %n when parsing date formats (see contao/core#8411).
 * Fix the module wizard's accessibility (see contao/core#8391).
 * Correctly initialize TinyMCE in sub-palettes in Firefox (see contao/core#3673).
 * Validate form field names more accurately (see contao/core#8403).
 * Correctly show the ctime, mtime and atime of a folder (see contao/core#8408).
 * Correctly index changed pages (see contao/core#8439).
@havutcuoglu
Copy link

I think it's better to change the name of the form field
from:
Please enter only the following characters: A-Z0-9[]_-
to:
Please enter only the following characters: A-Z0-9[]_- Not allowed are Ö,ö,Ü,ü,Ä,ä,ß and spaces

The user doesn't understand the meaning of A-Z because in german you have between A-Z also the Ä,Ö,Ü etc. or we need to but standardize and change all character from 'ä' to 'ae' like space with '_'

bildschirmfoto 2018-01-24 um 13 29 56

@ausi
Copy link
Member

ausi commented Jan 24, 2018

The user doesn't understand the meaning of A-Z because in german you have between A-Z also the Ä,Ö,Ü etc.

I don’t think that the Umlauts are between A and Z in the german alphabet: https://de.wikipedia.org/wiki/Deutsches_Alphabet

@havutcuoglu
Copy link

You are very funny. But you must understand that not all people are smart like you and me. 😃
However, I think different.

@Toflar
Copy link
Member

Toflar commented Jan 24, 2018

However, I think different.

If you do so, feel free to adjust the language files. I'm with @ausi: no change required here.

@Aybee
Copy link
Contributor

Aybee commented Jan 24, 2018

@ausi LOL that was what I liked to answer.

I think we should use
abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789[]_-
No, not seriously 😉 😆 😊

@havutcuoglu
Copy link

havutcuoglu commented Jan 26, 2018

Hey guys, ok forget the (Umlaut) letters but here existing a other problem.

If you duplicate a form field add Contao automatically the (Kopie) on field name, but it's only A-Z0-9[]_- allowed.
I think here is something wrong.
Testet on Contao v3.5.31

And also in Contao v4.4.7 by duplicating a form field stay the field name unchanged but here add Contao as label the (Kopie) with parentheses. More than one textfield with the same name attribute is strange.

Contao v3.5.31
bildschirmfoto 2018-01-24 um 13 29 56

Contao v4.4.7
bildschirmfoto 2018-01-26 um 15 18 24

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

10 participants