? Error

User tests: Successful: Unsuccessful:

avatar mhehm
mhehm
10 Dec 2013

...e

if list field have both (readonly & disabled) attrs, hidden input must be disabled too

avatar mhehm mhehm - open - 10 Dec 2013
avatar betweenbrain
betweenbrain - comment - 10 Dec 2013

Based on a quick code review, this makes sense.

avatar Achal-Aggarwal
Achal-Aggarwal - comment - 7 Jan 2014

If we are going to disable hidden input field then it is better to remove it from output at all. The whole logic can be simplified more, I mean less code.

avatar mhehm
mhehm - comment - 8 Jan 2014

I agree with Achal-Aggarwal. if you too, i apply it to my pr.

avatar Bakual
Bakual - comment - 8 Jan 2014

As far as I understand it, the hidden field is only there because the <select> doesn't support the readonly attribute.
It's a cheap way to achieve sort of a readonly behavior.
However since the select supports the disabled attribute, there is not really a need for a hidden field in this case to begin with.
My main question here would be why you would add the disabled and the readonly attribute. Imho disabled implies readonly anyway, but not the other way around (like we have it in code...).

Thus imho we should change the check for readonly to also check if disabled is false, and only add the hidden element if only readonly is set but not disabled.

avatar mhehm mhehm - change - 8 Jan 2014
Labels Added: ? ?
avatar mhehm
mhehm - comment - 8 Jan 2014

hmmm. yes your question is correct. why i added disabled and readonly both?!!
I think i do not added both of them to my component's form's xml. in my component i needed to disable or readonly some forms fields based on some fields values in load form function. then some fields may have both of readonly and disabled values together.
now when user save form, in save function i run some function based on fields that user submitted.
i can do very things to fix that input do not have both attributes. but this is more simple.

now your solution is better. i search web and understood that disabled attribute will not work with hidden inputs. (http://www.w3schools.com/tags/att_input_disabled.asp)
sorry for my mistake about this.

now i fixed my pr.

avatar Bakual
Bakual - comment - 8 Jan 2014

This will always return true: (string) $this->disabled != '1' || (string) $this->disabled != 'true')
You need: (string) $this->disabled != '1' && (string) $this->disabled != 'true') :smile:

search web and understood that disabled attribute will not work with hidden inputs.

Ah didn't know that myself but makes perfectly sense if you think about it.

avatar brianteeman brianteeman - change - 21 Aug 2014
Status New Pending
avatar nicksavov nicksavov - change - 21 Aug 2014
Labels Removed: ?
avatar zero-24
zero-24 - comment - 6 Feb 2015

Hi @mhehm

is this issue still valid? As the last comment here was on 8 Jan 2014? Also pull requests to the master branch of this repo are not longer accepted. Can you close this pull request and submit a new one against the staging branch, if this issue still exists?
If there is no Response in a month we will close the PR here.

Thanks for understanding!

avatar brianteeman
brianteeman - comment - 23 Mar 2015

closing as per the comment above from @zero-24


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/2659.
avatar brianteeman brianteeman - change - 23 Mar 2015
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2015-03-23 17:55:17
Closed_By brianteeman
avatar brianteeman brianteeman - close - 23 Mar 2015

Add a Comment

Login with GitHub to post a comment