Closed
Bug 162368
Opened 22 years ago
Closed 22 years ago
<option>s with no value attribute have no DOM value property
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
People
(Reporter: dr, Assigned: john)
References
()
Details
(Keywords: dom1, dom2, testcase, Whiteboard: [FIX])
Attachments
(3 files)
316 bytes,
text/html
|
Details | |
5.08 KB,
patch
|
rods
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
339 bytes,
text/html
|
Details |
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 ("").
Simple testcase. Click "test" to see the |value| property of the foo option.
Comment 3•22 years ago
|
||
John, wanna have a look at this?
Assignee | ||
Comment 4•22 years ago
|
||
Wow, you are totally right according to the spec and this would simplify some of our logic :)
Assignee: jst → jkeiser
Assignee | ||
Updated•22 years ago
|
Status: NEW → ASSIGNED
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!
Assignee | ||
Comment 6•22 years ago
|
||
This patch fixes the problem by removing GetValueOrText() and making GetValue() do that operation instead.
Assignee | ||
Comment 7•22 years ago
|
||
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.
Assignee | ||
Updated•22 years ago
|
Whiteboard: [FIX]
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.
Severity: normal → major
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...
Reporter | ||
Comment 10•22 years ago
|
||
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•22 years ago
|
||
Comment on attachment 95128 [details] [diff] [review] Patch r=rods
Attachment #95128 -
Flags: review+
Reporter | ||
Comment 12•22 years ago
|
||
jkeiser: does your patch fix the second issue?
Assignee | ||
Comment 13•22 years ago
|
||
Should, yes.
Comment 14•22 years ago
|
||
Comment on attachment 95128 [details] [diff] [review] Patch sr=jst
Attachment #95128 -
Flags: superreview+
Assignee | ||
Comment 15•22 years ago
|
||
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 16•22 years ago
|
||
(Second testcase is fixed too, BTW.)
Component: DOM: HTML → DOM: Core & HTML
QA Contact: stummala → general
You need to log in
before you can comment on or make changes to this bug.
Description
•