Bug 35256 - HTMLSelectElement::setSelectedIndexByUser() can trigger unnecessary JS when there is no change to the selected index
: HTMLSelectElement::setSelectedIndexByUser() can trigger unnecessary JS when t...
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: Forms
: 528+ (Nightly build)
: PC Mac OS X 10.5
: P2 Normal
Assigned To: John Sullivan
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-02-22 12:38 PST by John Sullivan
Modified: 2010-02-22 14:43 PST (History)
1 user (show)

See Also:


Attachments
Patch to bail out of setSelectedIndexByUser when the new index matches the old one (1.34 KB, patch)
2010-02-22 12:46 PST, John Sullivan
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Sullivan 2010-02-22 12:38:28 PST
This is a followup to <http://trac.webkit.org/changeset/51602>.

With that patch, HTMLSelectElement::setSelectedIndexByUser() is now triggered by Safari's forms autofill code. Some pages run JS that resets various form elements when the selected Country changes. As of 51602, that JS was being run even when there is no change to a pop-up element, which does no good and causes trouble for autofill.
Comment 1 John Sullivan 2010-02-22 12:46:45 PST
Created attachment 49235 [details]
Patch to bail out of setSelectedIndexByUser when the new index matches the old one
Comment 2 John Sullivan 2010-02-22 13:04:56 PST
Fixed in <http://trac.webkit.org/changeset/55101>
Comment 3 Eric Seidel 2010-02-22 14:06:42 PST
Comment on attachment 49235 [details]
Patch to bail out of setSelectedIndexByUser when the new index matches the old one

Can't we write a manual test?  One which would simply alert() on the change?
Comment 4 Eric Seidel 2010-02-22 14:06:57 PST
:(  for lack of testing.
Comment 5 John Sullivan 2010-02-22 14:43:42 PST
A manual test would have to be dependent on a particular client, since setSelectedIndexByUser() is not called if the user brings up the pop-up menu and then dismisses it with the initially-selected item still selected. So that's more in the realm of testing the browser than testing WebCore, I think.