Opened 12 years ago

Closed 9 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 10 years ago.
Updated 1.6.0 Patch
CheckedMultiSelect.html.patch (691 bytes) - added by Kitson Kelly 10 years ago.
Updated 1.6.0 Template Patch

Download all attachments as: .zip

Change History (19)

comment:1 Changed 12 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 12 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 11 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 11 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 11 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 11 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 11 years ago by Adam Peller

Milestone: 1.51.6

comment:8 Changed 11 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 11 years ago by dante

Milestone: 1.61.7
Status: newassigned

comment:10 Changed 11 years ago by Adam Peller

bump

Changed 10 years ago by Kitson Kelly

Attachment: CheckedMultiSelect.js.patch added

Updated 1.6.0 Patch

Changed 10 years ago by Kitson Kelly

Updated 1.6.0 Template Patch

comment:11 Changed 10 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 10 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 10 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 10 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 9 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 9 years ago by selcuk (previous) (diff)

comment:16 Changed 9 years ago by Kitson Kelly

Owner: changed from dante to Kitson Kelly

comment:17 Changed 9 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.