JFormFieldList has a bug handling the multiple attribute.

[subtext]

The JFormFieldList class has a bug on line 54. It is currently:

$attr .= $this->multiple ? ' multiple="multiple"' : '';

It is calling 'multiple' as being a direct property of the class, but it should be:

$attr .= $this->element['multiple'] ? ' multiple="multiple"' : '';

[pasamio]

Open up a pull request and fix it :)

[subtext]

Honestly, it's just that I'm a little intimidated about making a Pull request. I'm not well versed in the process and I didn't want to make any mistakes. Sam, can you point me in the direction of some documentation on the proper procedure for submitting a pull request to the platform? Thanks

[pasamio]

So there are these documents:
http://docs.joomla.org/Working_with_git_and_github
http://docs.joomla.org/Working_with_git_and_github/My_first_pull_request

In this particular case I think going to https://github.com/joomla/joomla-platform/ and click on "Fork" to create a personal fork. Then navigate to the file and click "edit" on it (make sure you're in the staging branch). You should be able to edit the file and have GitHub create a commit for you. Then you should be able to click on the "Pull Request" button and it'll give you a form to create a pull request. If you already have a fork you may need to update it which is going to require some more effort however if you are getting started this is probably the quickest way.

Some more notes here:
https://help.github.com/articles/using-pull-requests

[dongilbert]

If you are properly using the JForm class, then $this->multiple is the correct check to make. Check the setup method of JFormField here - https://github.com/joomla/joomla-platform/blob/staging/libraries/joomla/form/field.php#L306

The multiple element attribute must either equal "true" or "multiple" in the form XML, which gets processed by the JForm class, which then calls the setup method. The setup method then populates "some important attributes", one of which is the $multipleclass property, based on the $element['mulitple'] array var. If it does, then the class property of $multiple gets properly set, and can be used by the field element child class to set multiple="multiple"

Now, the real question is, why is $multiple a property of the abstract JFormField class? Not every field class that extends JFormField requires (or even allows for that matter) the multiple attribute to be set. After grepping the jform source tree, I found only 5 fields types that utilize the multiple attribute. JFormFieldList, JFormFieldGroupedlist, JFormFieldAccesslevel, JFormFieldUsergroup, and JFormFieldCheckboxes. Out of those, #'s 2 & 3 both extend JFormFieldList, and the last 2 are strange animals. Namely, JFormFieldUsergroup SHOULD extend JFormFieldlist, but doesn't; and JFormFieldCheckboxes creates a whole new property called $forceMultiple, which can override the $multiple class property, but is only used in this single special situation.

With all this in mind, it really looks like a refactor is needed rather than a simple fix as you suggested. But then you run into problems with developers. But since this is platform and not cms, I think we could implement without breaking backwards compatibility, and really nail down the best practices for developers to implement when utilizing the JFormField classes.

[elinw]

Checkboxes are mandated to be multiple according to the w3c so $forcemultiple essentially enforces this even if you don't write multiple="multiple" because doing so is just redundant with declaring type = "checkboxes" ... although it lets you break the rules too.

[dongilbert]

True, but it could be accomplished another way. I think essentially, the $multiple property should be moved to the field classes that use it, rather than in the abstract parent class. Then, the multiple check in JFormField could be changed to something like this:

$this->multiple = (bool) ($this->multiple || ($multiple === 'true' || $multiple === 'multiple'));

With that implementation, the child classes (JFormFieldCheckboxes) could set a property of protected $multiple = true which would replace the function of the $forceMultiple property. Then, we can go through the deprecation process with $forceMultiple, and remove the access code. Thoughts? If you think it's a good way to go, then I can start coding and send in a pull request along these lines.

[elinw]

We actually had a debate about that on another issue #1188 you might want to look at the argument there.

[subtext]

I vote for it to stay in the JFormField class. Though, I would rather see:

$this->multiple = ( ! ( (bool) $this->element['multiple'] ) || $this->element['multiple'] == "false" );

[elinw]

Could this be closed in light of #1684?

[subtext]

Yes, I think that's an appropriate action, Elin.

[LouisLandry]

Thanks, closing.