162368 - <option>s with no value attribute have no DOM value property

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Dan Rosen]

The specs:

  http://www.w3.org/TR/DOM-Level-2-HTML/html.html#ID-6185554 and
  http://www.w3.org/TR/REC-DOM-Level-1/level-one-html.html#ID-6185554

both define the |value| property of |HTMLOptionElement| to be the option's value
as defined by the HTML spec:

  http://www.w3.org/TR/html4/interact/forms.html#adef-value-OPTION

This says that if the <option>'s value attribute isn't set, the initial value is
set to the <option> element's content. However, we don't seem to follow that
rule. I have an <option>foo</option>, and when I access its |value| property, it
gives me an empty DOMString ("").

Comment 1

[Dan Rosen]

Attachment: Testcase

Simple testcase. Click "test" to see the |value| property of the foo option.

Comment 2

[Dan Rosen]

Note: this is in Mozilla 1.1a (20020702).

Comment 3

[Johnny Stenback (:jst)]

John, wanna have a look at this?

Comment 4

[John Keiser (jkeiser)]

Wow, you are totally right according to the spec and this would simplify some of
our logic :)

Comment 5

[Dan Rosen]

IE5 seems to have the same behavior as we do currently. I don't have IE6 sitting
around, but it might do the same... So I'm wondering if maybe we did this on
purpose?

Regardless, the current workaround is a big pain in the ass. The brainless
workaround would be to test if the |value| property is empty, and if so fall
back to the <option> content. But that doesn't work for the cases where the
value attribute or property is intentionally empty, for example:

  <option value="">none</option>

or cases where the |value| property was set to |null| by script... So if we
changed our behavior to interpret the spec correctly (regardless of what IE
happens to do) that would really clean this problem up. Hope this can make it
into mozilla1.2!

Comment 6

[John Keiser (jkeiser)]

Attachment: Patch

This patch fixes the problem by removing GetValueOrText() and making GetValue()
do that operation instead.

Comment 7

[John Keiser (jkeiser)]

This patch only gets text if there is no value attribute at all--if it's empty
.value will be empty.  We should be safe going with the spec instead of IE in
this case because if people ARE using the value="" attribute, things will remain
exactly the same, and if they are NOT, it is unlikely they will be using .value
since .value will return blank.

Comment 8

[Dan Rosen]

I just discovered a second half to this bug.

The |HTMLSelectElement| object has a |value| property that reflects the value of
the selected |HTMLOptionElement| child (or the first selected option, if there
are multiple). This |value| property is writable; writing to it has the effect
of selecting the option child with that value.

For example:

  <select id="foo">
    <option value="a">a</option>
    <option value="b">b</option>
  </select>

  ...

  document.getElementById("foo").value = "b";

would select the "b" option appropriately.

If the |HTMLOptionElement| child doesn't have a value attribute, then writing to
the parent |HTMLSelectElement| doesn't work at all.

I'll attach a testcase in a second. Bumping severity to "major" since the
effects of this seem larger in scale.

Comment 9

[Dan Rosen]

Attachment: Testcase #2

Here's the second testcase. Clicking the "test" button should select option
"b", but it does not.

This probably obsoletes patch 95128, but I don't want to jump the gun...

Comment 10

[Dan Rosen]

Just a thought: seems like QA might want to add these testcases to the DOM
testsuite, or whatever you use to automate spec compatibility testing...

Comment 11

[rods (gone)]

Comment on attachment 95128 [details] [diff] [review]
Patch

r=rods

Comment 12

[Dan Rosen]

jkeiser: does your patch fix the second issue?

Comment 13

[John Keiser (jkeiser)]

Should, yes.

Comment 14

[Johnny Stenback (:jst)]

Comment on attachment 95128 [details] [diff] [review]
Patch

sr=jst

Comment 15

[John Keiser (jkeiser)]

Fix checked in.

Comment 16

[John Keiser (jkeiser)]

(Second testcase is fixed too, BTW.)