Permalink
Browse files

Extend BUTTON elements with everything defined in Form.Element.Method…

…s. Ensure BUTTON elements are traversed in Form.getElements and serialized in Form.serialize. [#394 state:resolved] [#688 state:resolved] (Luis Gomez, Samuel Lebeau, kangax, Andrew Dupont)
  • Loading branch information...
1 parent a7e51ae commit 1fcf2e029977869e3389f363607cbb1c0c36fd94 @savetheclocktower savetheclocktower committed Oct 17, 2010
Showing with 77 additions and 47 deletions.
  1. +2 −0 CHANGELOG
  2. +2 −1 src/dom/dom.js
  3. +51 −42 src/dom/form.js
  4. +1 −0 test/unit/fixtures/form.html
  5. +21 −4 test/unit/form_test.js
View
@@ -1,3 +1,5 @@
+* Extend BUTTON elements with everything defined in Form.Element.Methods. Ensure BUTTON elements are traversed in Form.getElements and serialized in Form.serialize. (Luis Gomez, Samuel Lebeau, kangax, Andrew Dupont)
+
* Ensure Object.isFunction returns `false` for RegExp objects. [#661 state:resolved] (James, kangax, Andrew Dupont)
* Revert Opera-specific behavior for calling Element#getStyle with (left|right|top|bottom). [#268 state:resolved] (kangax, Andrew Dupont)
View
@@ -3292,7 +3292,8 @@ Element.addMethods = function(methods) {
"FORM": Object.clone(Form.Methods),
"INPUT": Object.clone(Form.Element.Methods),
"SELECT": Object.clone(Form.Element.Methods),
- "TEXTAREA": Object.clone(Form.Element.Methods)
+ "TEXTAREA": Object.clone(Form.Element.Methods),
+ "BUTTON": Object.clone(Form.Element.Methods)
});
}
View
@@ -724,68 +724,77 @@ var $F = Form.Element.Methods.getValue;
/*--------------------------------------------------------------------------*/
-Form.Element.Serializers = {
- input: function(element, value) {
+Form.Element.Serializers = (function() {
+ function input(element, value) {
switch (element.type.toLowerCase()) {
case 'checkbox':
case 'radio':
- return Form.Element.Serializers.inputSelector(element, value);
+ return inputSelector(element, value);
default:
- return Form.Element.Serializers.textarea(element, value);
+ return valueSelector(element, value);
}
- },
-
- inputSelector: function(element, value) {
- if (Object.isUndefined(value)) return element.checked ? element.value : null;
- else element.checked = !!value;
- },
-
- textarea: function(element, value) {
+ }
+
+ function inputSelector(element, value) {
+ if (Object.isUndefined(value))
+ return element.checked ? element.value : null;
+ else element.checked = !!value;
+ }
+
+ function valueSelector(element, value) {
if (Object.isUndefined(value)) return element.value;
else element.value = value;
- },
-
- select: function(element, value) {
+ }
+
+ function select(element, value) {
if (Object.isUndefined(value))
- return this[element.type == 'select-one' ?
- 'selectOne' : 'selectMany'](element);
- else {
- var opt, currentValue, single = !Object.isArray(value);
- for (var i = 0, length = element.length; i < length; i++) {
- opt = element.options[i];
- currentValue = this.optionValue(opt);
- if (single) {
- if (currentValue == value) {
- opt.selected = true;
- return;
- }
+ return (element.type === 'select-one' ? selectOne : selectMany)(element);
+
+ var opt, currentValue, single = !Object.isArray(value);
+ for (var i = 0, length = element.length; i < length; i++) {
+ opt = element.options[i];
+ currentValue = this.optionValue(opt);
+ if (single) {
+ if (currentValue == value) {
+ opt.selected = true;
+ return;
}
- else opt.selected = value.include(currentValue);
}
+ else opt.selected = value.include(currentValue);
}
- },
-
- selectOne: function(element) {
+ }
+
+ function selectOne(element) {
var index = element.selectedIndex;
- return index >= 0 ? this.optionValue(element.options[index]) : null;
- },
-
- selectMany: function(element) {
+ return index >= 0 ? optionValue(element.options[index]) : null;
+ }
+
+ function selectMany(element) {
var values, length = element.length;
if (!length) return null;
for (var i = 0, values = []; i < length; i++) {
var opt = element.options[i];
- if (opt.selected) values.push(this.optionValue(opt));
+ if (opt.selected) values.push(optionValue(opt));
@jdalton

jdalton Oct 18, 2010

Contributor

This might affect back compat because now you are referencing the private optionValue method so changes to the exposed Form.Element.Serializers.optionValue won't be reflected by this methods use.

}
return values;
- },
-
- optionValue: function(opt) {
- // extend element because hasAttribute may not be native
- return Element.extend(opt).hasAttribute('value') ? opt.value : opt.text;
}
-};
+
+ function optionValue(opt) {
+ return Element.hasAttribute(opt, 'value') ? opt.value : opt.text;
+ }
+
+ return {
+ input: input,
+ inputSelector: inputSelector,
+ textarea: valueSelector,
+ select: select,
+ selectOne: selectOne,
+ selectMany: selectMany,
+ optionValue: optionValue,
+ button: valueSelector
+ };
+})();
/*--------------------------------------------------------------------------*/
@@ -72,6 +72,7 @@
<input type="radio" name="tf_radio" value="on" />
<input type="hidden" name="tf_hidden" />
<input type="password" name="tf_password" />
+ <button name="tf_button"></button>
@jdalton

jdalton Oct 18, 2010

Contributor

Buttons can have inner content. IE6 will set the inner content instead of the value when setting button.value

@savetheclocktower

savetheclocktower Oct 18, 2010

Collaborator

Right, it's not a comprehensive solution. But we might as well extend the methods onto BUTTON elements and let users sort it out. (If you're using BUTTONs in your app, presumably you're not supporting IE6 at all in the first place.)

@jdalton

jdalton Oct 18, 2010

Contributor

As far as I know IE6 supports the BUTTON element.
http://code.google.com/p/doctype/wiki/ButtonElement

It feels kinda wonky that Prototype is full of less than comprehensive solutions... doesn't fix cross-browser delegation bugs, doesn't fix cross-browser clone() bugs, doesn't fix cross-browser button value bugs.

I think it does your users a disservice because on the surface Prototype says "Yes, we support these things.", but when devs use them they find that it was just lip service and that the cross-browser issues they thought the lib was smoothing over, like others do, where left for them to sort out :(

@jdalton

jdalton Oct 18, 2010

Contributor

Just checked and the BUTTON bug affects IE7 too.

@savetheclocktower

savetheclocktower Oct 18, 2010

Collaborator

We have these arguments every single week. We get nowhere.

How would you fix the BUTTON element in IE? Many have tried. How would you keep its value distinct from its innerHTML in IE 6-7? If you know of a fix, I will happily apply it.

A user requested that we add our convenience methods (like enable and disable) to BUTTONs. Another user requested that BUTTON elements be included in form serializations. These are both valid requests that have nothing to do with the issue you described. You seem to be saying that unless we can provide a "comprehensive" solution, we shouldn't provide any solution at all.

@jdalton

jdalton Oct 18, 2010

Contributor

Another user requested that BUTTON elements be included in form serializations. These are both valid requests that have nothing to do with the issue you described.

You can't serialize the BUTTON element's value correctly if you're getting its inner content instead of its value which has everything to do with the issue I described.

@jdalton

jdalton Oct 18, 2010

Contributor

You seem to be saying that unless we can provide a "comprehensive" solution, we shouldn't provide any solution at all.

Yes, other libs like MooTools and jQuery fix these issues. Providing a half-working solution doesn't help anyone.

@savetheclocktower

savetheclocktower Oct 18, 2010

Collaborator

You didn't answer my question. How would you fix how the BUTTON element behaves in IE? Or, rather, how would you want Prototype to fix it?

@jdalton

jdalton Oct 18, 2010

Contributor

I fixed the issue by setting the element's attribute node value. http://github.com/jdalton/fusejs/blob/master/src/dom/element/attribute.js#L316-331

</div>
</form>
View
@@ -204,7 +204,7 @@ new Test.Unit.Runner({
testFormGetElements: function() {
var elements = Form.getElements('various'),
- names = $w('tf_selectOne tf_textarea tf_checkbox tf_selectMany tf_text tf_radio tf_hidden tf_password');
+ names = $w('tf_selectOne tf_textarea tf_checkbox tf_selectMany tf_text tf_radio tf_hidden tf_password tf_button');
this.assertEnumEqual(names, elements.pluck('name'))
},
@@ -226,7 +226,15 @@ new Test.Unit.Runner({
testFormSerialize: function() {
// form is initially empty
var form = $('bigform');
- var expected = { tf_selectOne:'', tf_textarea:'', tf_text:'', tf_hidden:'', tf_password:'' };
+ var expected = {
+ tf_selectOne: '',
+ tf_textarea: '',
+ tf_text: '',
+ tf_hidden: '',
+ tf_password: '',
+ tf_button: ''
+ };
+
this.assertHashEqual(expected, Form.serialize('various', true));
// set up some stuff
@@ -235,10 +243,19 @@ new Test.Unit.Runner({
form['tf_text'].value = "123öäü";
form['tf_hidden'].value = "moo%hoo&test";
form['tf_password'].value = 'sekrit code';
+ form['tf_button'].value = 'foo bar';
form['tf_checkbox'].checked = true;
form['tf_radio'].checked = true;
- var expected = { tf_selectOne:1, tf_textarea:"boo hoo!", tf_text:"123öäü",
- tf_hidden:"moo%hoo&test", tf_password:'sekrit code', tf_checkbox:'on', tf_radio:'on' }
+
+ var expected = {
+ tf_selectOne: 1, tf_textarea: "boo hoo!",
+ tf_text: "123öäü",
+ tf_hidden: "moo%hoo&test",
+ tf_password: 'sekrit code',
+ tf_button: 'foo bar',
+ tf_checkbox: 'on',
+ tf_radio: 'on'
+ };
// return params
this.assertHashEqual(expected, Form.serialize('various', true));

0 comments on commit 1fcf2e0

Please sign in to comment.