Browse files

Fix destructive methods for selecting options

Both `select_option` and `unselect_option` were modifying the
`selected` attribute of their target `<option>` element. This attribute
is meant to be used solely as the marker for which `<option>`(s) is
(are) the default selection(s) in a (multi-)`<select>` element. The
actual _selectedness_ of an `<option>` should be tracked using its
`selected` **property**. Read [the spec][] for more info.

This change removes any code which modified the `selected` attribute of
`<option>` elements, which leaves only the code that modifies the
`selected` property. In addition, `Node#value` needed to be changed to
return `<option>` elements whose selectedness is true rather than just
those with a `selected` attribute.

The tests introduced in the previous commit now pass.

[the spec]:
  • Loading branch information...
jasonmp85 committed Feb 12, 2012
1 parent acd6e47 commit 2fef84403173c8c789efae911dff380034bbca81
Showing with 1 addition and 7 deletions.
  1. +1 −5 lib/capybara/driver/webkit/node.rb
  2. +0 −2 src/capybara.js
@@ -22,11 +22,7 @@ def [](name)
def value
if multiple_select?
- self.find(".//option").select do |option|
- option["selected"] == "selected"
- do |option|
- option.value
- end
+ self.find(".//option").select(&:selected?).map(&:value)
invoke "value"
@@ -196,13 +196,11 @@ Capybara = {
selectOption: function(index) {
this.nodes[index].selected = true;
- this.nodes[index].setAttribute("selected", "selected");
this.trigger(index, "change");
unselectOption: function(index) {
this.nodes[index].selected = false;
- this.nodes[index].removeAttribute("selected");
this.trigger(index, "change");

0 comments on commit 2fef844

Please sign in to comment.