Opened 8 years ago

Closed 8 years ago

#1386 closed defect (fixed)

Setting default checkbox settings in dialogs does not work in Safari

Reported by: nicholasbs Owned by: nicholasbs
Priority: normal Milestone: 0.96
Component: Dialogs Version: trunk
Severity: normal Keywords:
Cc:

Description

Setting the default value for checkboxes in dialogs does not currently work in Safari. This is because if no value attribute is specified on the input element, Safari returns the default value as an empty string, whereas Firefox, IE, and Opera all return the string "on" as the default value. I don't know which of these is right, but the code is presently written to expect a default value of "on" (at least in the couple of plugins I've looked at, e.g., TableOperations and SmartReplace?).

We should probably always specify a value for our checkboxes, if only because it's a required attribute in the W3C spec.

However, setting the value of checkboxes to "on" seems a little wonky to me. The Xinha.Dialog.prototype.setValues method works by comparing the value for the given element in the dictionary passed in to the value attribute of the element itself. This too seems a bit weird to me -- wouldn't it make more sense to check against a predetermined string, e.g., 'checked' instead? Is there a good reason why it's done the way it is?

Change History (6)

comment:1 in reply to: ↑ description Changed 8 years ago by gogo

Replying to nicholasbs:

The Xinha.Dialog.prototype.setValues method works by comparing the value for the given element in the dictionary passed in to the value attribute of the element itself.

Wouldn't doing otherwise preclude using different values for checkboxes? Maybe you don't do it, but it's pretty common and useful...

  <input type="checkbox" name="EditTheseIds[]" value="1" />
  <input type="checkbox" name="EditTheseIds[]" value="2" />
  <input type="checkbox" name="EditTheseIds[]" value="3" />

Or have I misunderstood you.

comment:2 follow-up: Changed 8 years ago by nicholasbs

I don't think this would preclude using different values for checkboxes, as the parameter passed in would be checked against a string, independent of what the input element's value was.

I think part of the issue is defining what exactly the setValues function is supposed to do. Its name suggests to some extent that it sets the value attributes on the form elements, but that's not entirely true. It sets the element values for text fields and text areas, but for select boxes, radio buttons, and checkboxes, it compares the values passed in to the value attributes of the elements and, if they match, selects/checks that element. This works fine, except that the code using setValues is assuming that all browsers automatically set checkboxes' value attributes to "on" if they're not explicitly set, which Safari doesn't do.

comment:3 in reply to: ↑ 2 ; follow-up: Changed 8 years ago by gogo

Replying to nicholasbs:

It sets the element values for text fields and text areas, but for select boxes, radio buttons, and checkboxes, it compares the values passed in to the value attributes of the elements and, if they match, selects/checks that element.

What else would you propose it do? The purpose of setValues is clearly (IMHO) to set the values that would be returned by the form when/if it is submitted. For text/hidden/password/textarea elements that means writing the value. For selects/checkboxes/radiobuttons it means selecting/checking the option(s)/input(s) with that value(s).

The HTML of the dialog specifies the valid set of values for a select or checkbox/radiobutton set, by the presence of the options and the values of the radiobuttons/checkboxes.

I don't see how it would make sense for setValues to actually set the value of an option/checkbox/radiobutton. Not in the general sense, and if somebody wants to do that for some special reason, then let them make thier own setValues function.

This works fine, except that the code using setValues is assuming that all browsers automatically set checkboxes' value attributes to "on" if they're not explicitly set, which Safari doesn't do.

I think that this is the real error, that people using setValues (and therefore the ones who made the inputs without values in the first place) make this faulty assumption. Values are required for checkboxes and radio buttons, it makes no sense NOT to have a value, simple as that.

comment:4 Changed 8 years ago by ray

Wouldn't it be most pragmatic to simply set values to "on" when empty, e.g. in Xinha.Dialog.prototype.fixupDOM?

comment:5 in reply to: ↑ 3 Changed 8 years ago by nicholasbs

Replying to gogo:

</snip>

I think that this is the real error, that people using setValues (and therefore the ones who made the inputs without values in the first place) make this faulty assumption. Values are required for checkboxes and radio buttons, it makes no sense NOT to have a value, simple as that.

Agreed. We should add value attributes to all the input elements that don't have them already.

I'll also add a comment clarifying that setValues sets the value attributes of text inputs, but the checked/selected status of checkboxes, etc. I.e., as gogo said, it sets the default values that will be submitted by the form if the user does not alter any of the input elements.

I'm running out the door now but will make these changes tomorrow.

comment:6 Changed 8 years ago by nicholasbs

  • Resolution set to fixed
  • Status changed from new to closed

r1165 fixes this by adding value attributes to all checkboxes/radio buttons that did not previously have them. In many cases, the values for the checkboxes were never being used, so this should have no effect (other than making the markup valid), however, in a few cases (SmartReplace?, InsertTable?), the checkboxes will now be set correctly in Safari.

Note: See TracTickets for help on using tickets.