Skip to content

Commit

Permalink
Extend BUTTON elements with everything defined in Form.Element.Method…
Browse files Browse the repository at this point in the history
…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
savetheclocktower committed Oct 17, 2010
1 parent a7e51ae commit 1fcf2e0
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 47 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG
@@ -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)
Expand Down
3 changes: 2 additions & 1 deletion src/dom/dom.js
Expand Up @@ -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)
});
}

Expand Down
93 changes: 51 additions & 42 deletions src/dom/form.js
Expand Up @@ -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));

This comment has been minimized.

Copy link
@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
};
})();

/*--------------------------------------------------------------------------*/

Expand Down
1 change: 1 addition & 0 deletions test/unit/fixtures/form.html
Expand Up @@ -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>

This comment has been minimized.

Copy link
@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

This comment has been minimized.

Copy link
@savetheclocktower

savetheclocktower Oct 18, 2010

Author 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.)

This comment has been minimized.

Copy link
@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 :(

This comment has been minimized.

Copy link
@jdalton

jdalton Oct 18, 2010

Contributor

Just checked and the BUTTON bug affects IE7 too.

This comment has been minimized.

Copy link
@savetheclocktower

savetheclocktower Oct 18, 2010

Author 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.

This comment has been minimized.

Copy link
@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.

This comment has been minimized.

Copy link
@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.

This comment has been minimized.

Copy link
@savetheclocktower

savetheclocktower Oct 18, 2010

Author 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?

This comment has been minimized.

Copy link
@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>

Expand Down
25 changes: 21 additions & 4 deletions test/unit/form_test.js
Expand Up @@ -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'))
},

Expand All @@ -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
Expand All @@ -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));
Expand Down

0 comments on commit 1fcf2e0

Please sign in to comment.