Bug 88749 - HTMLSelectElement::selectedOptions returns wrong collection
Summary: HTMLSelectElement::selectedOptions returns wrong collection
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL: http://jsfiddle.net/f39cC/5/
Keywords: InRadar, WebExposed
Depends on:
Blocks:
 
Reported: 2012-06-10 21:30 PDT by Kent Tamura
Modified: 2012-06-27 18:36 PDT (History)
9 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Tamura 2012-06-10 21:30:21 PDT
http://code.google.com/p/chromium/issues/detail?id=131890

1. Open http://jsfiddle.net/f39cC/5/
2. Select One, then add Two to the selection, then Three
3. Select Two only

What is the expected result?
The following results are respectively expected: "One", "One,Two", "One,Two,Three", "Two".

What happens instead?
We get "One", "One", "One,Two", "Two,,".
Comment 1 Kent Tamura 2012-06-14 23:47:20 PDT
I'll roll out http://trac.webkit.org/changeset/110340 because this feature is incomplete at all.
For example, If we change select-selectedOptions.html so that

debug((++i) + ") Select an option should update the selected options collection.");
mySelect.options[0].selected = true;
shouldBe("mySelect.selectedOptions.length", "1"); // reorder these two lines
shouldBe("mySelect.options.length", "2"); // ditto.
shouldBe("mySelect.selectedOptions[0].text", "'one'");

the test fails.

HTMLCollection invalidates the content if domTreeVersion is changed. But selected state change doesn't update domTreeVersion.  The test works accidentally because shouldBe() updates domTreeVersion.
Comment 2 Kent Tamura 2012-06-15 00:13:51 PDT
Committed r120413: <http://trac.webkit.org/changeset/120413>
Comment 3 Radar WebKit Bug Importer 2012-06-15 08:13:20 PDT
<rdar://problem/11676534>
Comment 4 Simon Fraser (smfr) 2012-06-26 14:15:52 PDT
This rollout broke Garden-o-matic.
Comment 5 Alexis Menard (darktears) 2012-06-26 14:18:38 PDT
The rollout is overkill. I can just fix the bug. Now I will need to land again the feature + fix it :(.
Comment 6 Alexis Menard (darktears) 2012-06-26 18:09:39 PDT
(In reply to comment #5)
> The rollout is overkill. I can just fix the bug. Now I will need to land again the feature + fix it :(.

For the record :

http://jsfiddle.net/f39cC/20/

works fine.

What seems to get crazy is when using :

output.innerHTML = ([]).map.call(this.selectedOptions, function(so) {return so.value;}).join();
console.log(([]).map.call(this.selectedOptions, function(so) {return so.value;}).join());

together. Using one of them at a time is fine.

"because this feature is incomplete at all."

Well we just spotted a bug, but as the jsfiddle shows the basic use case work.

We do have a problem I agree with that and it's related to the selected state not invalidating properly the selectedOptions collection.

Also I couldn't reproduce with the suggestion of comment #1 unless I missed something.

I do have a patch here to fix the original issue, I'll cleanup and upload tomorrow.
Comment 7 Kent Tamura 2012-06-26 18:27:47 PDT
I'm sorry for rolling out.  I was too aggressive. I wanted to resolve this issue before Chromium M21 branch, and the bug had no response for four days.
Comment 8 Alexis Menard (darktears) 2012-06-26 18:39:29 PDT
(In reply to comment #7)
> I'm sorry for rolling out.  I was too aggressive. I wanted to resolve this issue before Chromium M21 branch, and the bug had no response for four days.

I'm on IRC, don't hesitate to ping me. For some reason you CC me but it was in my "to read" bugzilla folder, sorry to have missed the email.
Comment 9 Alexis Menard (darktears) 2012-06-27 18:36:15 PDT
(In reply to comment #8)
> (In reply to comment #7)
> > I'm sorry for rolling out.  I was too aggressive. I wanted to resolve this issue before Chromium M21 branch, and the bug had no response for four days.
> 
> I'm on IRC, don't hesitate to ping me. For some reason you CC me but it was in my "to read" bugzilla folder, sorry to have missed the email.

I fixed the bug.