Opened 10 years ago

Closed 7 years ago

#10544 closed defect (fixed)

[patch][ccla]dojox.form.CheckedMultiSelect does not set name attribute for form submission

Reported by: Kitson Kelly Owned by: Kitson Kelly
Priority: high Milestone: 1.8
Component: DojoX Form Version: 1.4.0
Keywords: Cc:
Blocked By: Blocking:

Description

dojox.form.CheckedMultiSelect? does not set name attribute for form submission. For example when I declare the following CheckedMultiSelect?:

<select dojoType="dojox.form.CheckedMultiSelect" name="processTypeSelect" id="processTypeSelect" multiple="true" style="height:166px;" store="processTypeList"></select>

I get the following in Firebug for the hidden select field where the value of the selection is set:

<select id="processTypeSelect" class="dojoxMultiSelectSelect" dojoattachpoint="containerNode,focusNode" multiple="true" tabindex="0" style="-moz-user-select: none;"/>

Because the name attribute is not being set, form submission functions do not work properly, with the field being ignored. I am using 1.4.0 final.

Attachments (2)

CheckedMultiSelect.js.patch (445 bytes) - added by Kitson Kelly 8 years ago.
Updated 1.6.0 Patch
CheckedMultiSelect.html.patch (691 bytes) - added by Kitson Kelly 8 years ago.
Updated 1.6.0 Template Patch

Download all attachments as: .zip

Change History (19)

comment:1 Changed 10 years ago by Adam Peller

Owner: changed from dante to Nathan Toone

I think ${nameAttrSetting} needs to be inserted into the template for the SELECT element.

http://stackoverflow.com/questions/1908121/value-of-dojox-checkedmultiselect-not-being-posted-with-dojo-xhrpost

comment:2 Changed 10 years ago by Kitson Kelly

I attempted to insert ${nameAttrSetting} into the html template and I now get:

<select id="processTypeSelect" class="dojoxMultiSelectSelect" 
  dojoattachpoint="containerNode,focusNode" name="processTypeSelect" multiple="true" 
  tabindex="0" style="-moz-user-select: none;"/>

for the hidden select, but I still do not get the field submitted on the form xhrPost.

comment:3 Changed 9 years ago by Kitson Kelly

I was able to finally figure out how to get it to work. Basically the component doesn't set options in the <select> tag. What I did was follow the pattern of dijit.form.Select and modify the html template to change the hidden element from a <select> to an <input>, add the ${!nameAttrString}, set the "valueNode" on the dojoAttachPoint as well as ensure the waiRole is correct.

In the js object, I basically overrode the _setAttrValue to set the valueNode when setting the value, which is the same way the dijit.form.Select works. I validated the changes by adding a submit button to the Widget's test file, though I am not including that in my patch.

I also refactored to use set and get instead of attr.

comment:4 Changed 9 years ago by Kitson Kelly

After some more testing, I realised that the value of the hidden input field was only being updated when the Widget was set to multiple=false, so I have updated the patch to address that issue by setting the valueNode on the _updateSelection versus overriding the _setAttrValue.

comment:5 Changed 9 years ago by dante

Milestone: tbd1.5
Owner: changed from Nathan Toone to dante

looks like the bulk of this is attr->get changes that need to happen anyway. Taking for nathan. Do you have a CLA on file?

comment:6 Changed 9 years ago by Kitson Kelly

Yes, my CCLA was accepted on 28 April by Aimee. Bill has already committed a couple of other patches by me already.

comment:7 Changed 9 years ago by Adam Peller

Milestone: 1.51.6

comment:8 Changed 9 years ago by Adam Peller

Summary: dojox.form.CheckedMultiSelect does not set name attribute for form submission[patch][ccla]dojox.form.CheckedMultiSelect does not set name attribute for form submission

comment:9 Changed 9 years ago by dante

Milestone: 1.61.7
Status: newassigned

comment:10 Changed 8 years ago by Adam Peller

bump

Changed 8 years ago by Kitson Kelly

Attachment: CheckedMultiSelect.js.patch added

Updated 1.6.0 Patch

Changed 8 years ago by Kitson Kelly

Updated 1.6.0 Template Patch

comment:11 Changed 8 years ago by Kitson Kelly

I have updated my patch to make it current with 1.6.0. I also changed it to be in line more with other form widgets that have to set their value. Again, it changes the hidden <select> into a hidden <input> and then on the _updateSelection, setting the this.valueNode.value = this.value, which causes a value to be submitted when the form is submitted.

comment:12 Changed 8 years ago by jer

I think patch is ok for ajax submit as json, but is breaking normal browser submit. Values are passed as

field[]=value1,value2

instead of

field[]=value1&field[]=value2

my version: in .js patch instead of

this.valueNode.value = this.value;

use this:

dojo.empty(this.containerNode);
_this = this
dojo.forEach(this.value, function(item) {
	var opt = dojo.create('option', {
		'value': item,
		'label': item,
		'selected': 'selected'
	});
	opt.innerHTML = item;
	dojo.place(opt, _this.containerNode);
})

This will add selected options to hidden select and provide correct submition for both ajax/json and normal browser submit. In template original select should be kept, with addition of ${!nameAttrSetting}

comment:13 Changed 8 years ago by Macs75

I agree and confirm that this solution is better(apart for the missing semicolons). Having to mantain degradability this does the job, I was going crazy with the previous one.

comment:14 Changed 8 years ago by cobrakai

This issue apparently has not been fixed in the 1.7.1 (or 1.7.2) release. The AJAX form submission still does not include the values selected. Also, I noticed that when viewing the control through FireBug? when an option is selected one of the ancestor DIV nodes (the grandparent) has an attribute named aria-selected which turns true when you check the checkbox. However, when you uncheck the checkbox this field does not change to false. It will only change to false after check a different checkbox. I'm not sure that this is related to the primary issue, but it was something I noticed.

comment:15 Changed 7 years ago by selcuk

The code snippet from comment 12 when applied to CheckedMultiSelect?.js solved the submit problem without any other changes to the html template in 1.7.1. I have also changed the dijit.MenuItem? used in the dropdown version to dijit.CheckedMenuItem? for much better and consistent visual representation. Here's the diff:

***************
*** 17,23 ****
  	"dijit/_WidgetsInTemplateMixin",
  	"dijit/registry",
  	"dijit/Menu",
! 	"dijit/MenuItem",
  	"dijit/Tooltip",
  	"dijit/form/_FormSelectWidget",
  	"dijit/form/ComboButton",
--- 17,23 ----
  	"dijit/_WidgetsInTemplateMixin",
  	"dijit/registry",
  	"dijit/Menu",
! 	"dijit/CheckedMenuItem",
  	"dijit/Tooltip",
  	"dijit/form/_FormSelectWidget",
  	"dijit/form/ComboButton",
***************
*** 478,483 ****
--- 478,495 ----
  		array.forEach(this._getChildren(), function(item){
  			item._updateBox();
  		});
+ 		dojo.empty(this.containerNode);
+ 		_this = this;
+ 		dojo.forEach(this.value, function(item) {
+ 			var opt = dojo.create('option', {
+ 				'value': item,
+ 				'label': item,
+ 				'selected': 'selected'
+ 			    });
+ 			opt.innerHTML = item;
+ 			dojo.place(opt, _this.containerNode);
+ 		    });
+ 
  		if(this.dropDown && this.dropDownButton){
  			var i = 0, label = "";
  			array.forEach(this.options, function(option){

Last edited 7 years ago by selcuk (previous) (diff)

comment:16 Changed 7 years ago by Kitson Kelly

Owner: changed from dante to Kitson Kelly

comment:17 Changed 7 years ago by Kitson Kelly

Resolution: fixed
Status: assignedclosed

In [29095]:

Fix CheckedMultiSelect? to set HTML form value and refactor test case for AMD, fixes #10544 refs #14929 !strict

Note: See TracTickets for help on using tickets.