8000 [Form] read_only field option should create a readonly input attribute · Issue #1974 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Form] read_only field option should create a readonly input attribute #1974

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
kmohrf opened this issue Aug 17, 2011 · 18 comments
Closed

[Form] read_only field option should create a readonly input attribute #1974

kmohrf opened this issue Aug 17, 2011 · 18 comments
Labels
Enhancement Form
Milestone
2.1

Comments

@kmohrf
Copy link
Contributor
kmohrf commented Aug 17, 2011

hey,

i stumbled upon this field option and think it's not the right way to solve it

http://symfony.com/doc/current/reference/forms/types/text.html#read-only

the docs say, that the read_only parameter renders the field with a disabled attribute. even if disabled is the favored method of setting the field "not-editable" the parameter should be called disabled because there actually is a readonly attribute since html4 and it allows some kind of interaction that a disabled field does not.

in my case i have a text input that is filled with a value by a colorpicker. the color picker is triggered by the user clicking on that field. in contrast to a readonly input a disabled input is not clickable.

this is not a blocker as the readonly attribute can still be created by adding it to the attr parameter. i guess its still confusing though.

kind regards (and thanks for creating this)

konrad
@helmer
Copy link
Contributor
helmer commented Sep 1, 2011

+1 to changing this (in 2.1? it's likely a BC breaker)

Imho, read_only parameter should render as readonly="readonly" and disabled as disabled="disabled", raised this a while ago (sadly, cannot find reference) and it yielded contradictory opinions. What are the current thoughts on this?

@stloyd
Copy link
Contributor
stloyd commented Oct 25, 2011

@fabpot, @stof, @bschussek: I was thinking about this. We should decided which way we choose... Do we go way to be compilant with W3C specs ?

If so we will need to change behavior. Current Form::setReadOnly() should be renamed to Form::setDisabled() also maybe we would need to change behavior of Form::setData() (for skipping field data if is disabled [not sure yet]). Those changes are required to be compilant with W3C spec. Also we would need to add readonly behavior, which should be normally validated as there is no notes (AFAIK) that field with this attr will be not send through form. Next thing would be limiting to set this option only at fields listed in spec.

If not we should note in docs that we are not compilant with W3C, and lists diffs in behavior.

ps. I listed html4 spec, but AFAIK in html5 is almost identical, maybe just with more details.

@beberlei
Copy link
Contributor

I am +1 for changing this, i stumbled accross this problem several times already.

@kmohrf
Copy link
Contributor Author
kmohrf commented Oct 25, 2011

@stloyd could you be a little more verbose about "that field with this attr will be not send through form".

if i get your point you don't want readonly fields to be processed by the form. i dont think that this would be a good decision and my usecase pretty much sums up for what kind of interaction input elements with readonly attribute are handy for. the w3c spec actually says that "Read-only elements may be successful.". grant them success :)

@stloyd
Copy link
Contributor
stloyd commented Oct 25, 2011

@kmohrf My thought was that, there is no info that field with readonly should skip validation (I'm not talking about html5 pattern etc.). As I understand W3C spec, field with readonly should be send as normal and validated and even bound normally, but only rendered as not editable.

You simply got me wrong (or I was not precise), readonly (html attr) should go as normal (mentioned above), disabled should not be send (browser should do it alone), (AFAIK) given data should be skipped and not validated.

ps. I'm a bit confused now and I think I get myself into loop ^^

@quba

Copy link
quba commented Nov 13, 2011

Is there any option to add readonly parameter to a collection field? Can't achieve that using "attr" in twig nor setAttribute in formbuilder. Disabled="disabled" isn't what i want, becouse it doesn't send post data.
BTW. I was also confused with naming disabled="disabled" as read_only o_O.

+1 for adding disabled and read_only to the formbuilder parameters array for text and email fields.

@alterphp
Copy link

not very original.... +1 for changing this !

@abdeltiflouardi
Copy link

+1 for changing this:

I modified it #3037

@fabpot
Copy link
Member
fabpot commented Jan 22, 2012

@bschussek Can you give us your opinion here? Why it is as it is today?

@webmozart
Copy link
Contributor

The implementation is as is because I wasn't fully aware of the W3C's details concerning "readonly" and "disabled", and because one option for making a field read-only seemed sufficient.

Since this seems to cause confusion, we could should change the implementation to be W3C-compliant. The HTML5 spec allows submission of inputs with a "readonly" attribute, but not of inputs with a "disabled" attribute. Thus, fields having the "read_only" option set should be processed (unlike today), while binding of "disabled" fields should be skipped (like "read_only" today).

The documentation should clearly state that "read_only" fields are processed by the server, to prevent any potential security holes.

This would be a BC break. @fabpot?

@fabpot
Copy link
Member
fabpot commented Jan 24, 2012

@bschussek: Yes, that would break BC but I think this is needed.

@Tobion
Copy link
Contributor
Tobion commented Jan 24, 2012

Maybe make read_only option configurable to clearly distinguish the cases:
true: read_only attribute and server-processed like HTML spec
'WITH_VALIDATION': read_only attribute and ensures that the submitted value is the same as the default

@helmer
Copy link
Contributor
helmer commented Jan 24, 2012

@Tobion Please, no.

@helmer
Copy link
Contributor
helmer commented Jan 24, 2012

@stloyd, @bschussek: I could not quite grasp from the spec, how readonly fields should be treated on the server side, but .. it does not really make sense to me, that the posted values are processed normally. A readonly field makes sense only if it has a (default) value. You can of course make it dynamic (ie full name = first name + last name) with a bit of client side scripting, but this logic should imo be duplicated server side as well.

Let's take a common user. He has no idea, how to edit the value of a readonly field and therefore binding the posted value server side equals to binding the default value. If we are dealing with a smart user though, she has control over the field, and can set it to whatever she prefers, perhaps even to something potentially risky. This might bring up wtf'y moments ..

Based on that I propose readOnly fields to be bound based on default value, not the posted value.

@alterphp
Copy link

As readonly HTML attribute can be set with 'attr' array FormType property, as well as disabled HTML attribute, and given the current process of 'read_only' FormType property is very ambiguous, 'read_only' property could simply be deprecated ?

@kmohrf kmohrf closed this as completed Jan 24, 2012
@kmohrf kmohrf reopened this Jan 24, 2012
@kmohrf
Copy link
Contributor Author
kmohrf commented Jan 24, 2012

sorry. damn keyboard shortcuts.

what i wanted to say:
@helmer: i strongly disagree. every useragent i know of processes and submits readonly fields. and i dont see a reason why readonly fields shouldnt be processed in symfony. i actually provided a usecase above where you dont really want the user to edit the field but allow him to see what is sent. and the colorpicker (as long as type="color" is not implemented) is a quite good example imho.

@helmer
Copy link
Contributor
helmer commented Jan 26, 2012

@kmohrf I tend to agree with you, discard my previous rant.

As a side note, I opened a PR reflecting the comments in this issue, have a look.

@fabpot fabpot closed this as completed in e71d157 Feb 2, 2012
@ataucare
Copy link

It might be a little late the help, but I could solve this way:

In TWIG template => {{ form_widget(form.field, { 'attr': { 'readonly': 'true' } }) }}

Regards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Enhancement Form
Projects
None yet
Milestone
2.1
Development

No branches or pull requests
0