HTML::Selector#simple_selector (assert_select) does not recognize attributes with brackets in double quotes #2412

Closed
Arsen7 opened this Issue Aug 3, 2011 · 5 comments

Projects

None yet

4 participants

@Arsen7
Arsen7 commented Aug 3, 2011

Consider this example in functional tests:

assert_select "input[name='user[name]']"
assert_select 'input[name="user[name]"]'

The first line will work perfectly, but the second raises an exception: ArgumentError: Invalid selector: "]

The problem lies in the actionpack/lib/action_controller/vendor/html-scanner/html/selector.rb, where someone apparently made a typo.

Please accept a patch:

diff --git a/actionpack/lib/action_controller/vendor/html-scanner/html/selector.rb b/actionpack/lib/action_controller/vendor/html-scanner/html/selector.rb
index 1eadfc0..17088c1 100644
--- a/actionpack/lib/action_controller/vendor/html-scanner/html/selector.rb
+++ b/actionpack/lib/action_controller/vendor/html-scanner/html/selector.rb
@@ -558,7 +558,7 @@ module HTML
         end

         # Attribute value.
-        next if statement.sub!(/^\[\s*([[:alpha:]][\w\-:]*)\s*((?:[~|^$*])?=)?\s*('[^']*'|"[^*]"|[^\]]*)\s*\]/) do |match|
+        next if statement.sub!(/^\[\s*([[:alpha:]][\w\-:]*)\s*((?:[~|^$*])?=)?\s*('[^']*'|"[^"]*"|[^\]]*)\s*\]/) do |match|
           name, equality, value = $1, $2, $3
           if value == "?"
             value = values.shift
diff --git a/actionpack/test/controller/selector_test.rb b/actionpack/test/controller/selector_test.rb
index 8ce9e43..e25ffad 100644
--- a/actionpack/test/controller/selector_test.rb
+++ b/actionpack/test/controller/selector_test.rb
@@ -102,7 +102,7 @@ class SelectorTest < Test::Unit::TestCase


   def test_attribute_quoted
-    parse(%Q{<div id="1" title="foo"></div><div id="2" title="bar"></div><div id="3" title="  bar  "></div>})
+    parse(%Q{<div id="1" title="foo"></div><div id="2" title="bar"></div><div id="3" title="  bar  "></div><div id="4" name="item[baz]"></div>})
     # Match without quotes.
     select("[title = bar]")
     assert_equal 1, @matches.size
@@ -119,6 +119,14 @@ class SelectorTest < Test::Unit::TestCase
     select("[title = \"  bar  \" ]")
     assert_equal 1, @matches.size
     assert_equal "3", @matches[0].attributes["id"]
+    # Match brackets in single quotes.
+    select("[name = 'item[baz]' ]")
+    assert_equal 1, @matches.size
+    assert_equal "4", @matches[0].attributes["id"]
+    # Match brackets in double quotes.
+    select("[name = \"item[baz]\" ]")
+    assert_equal 1, @matches.size
+    assert_equal "4", @matches[0].attributes["id"]
   end

I am almost sure I reported the issue in 2008 or earlier, but now I cannot find any trace. Maybe I forgot?

@dmathieu
dmathieu commented Aug 4, 2011

Thank you for your patch. It'd be better if you could fork the project, push your commit to your fork and open a pull request.
See the contributing guide

@Arsen7 Arsen7 added a commit to Arsen7/rails that referenced this issue Aug 4, 2011
@Arsen7 Arsen7 simple_selector: attributes with brackets in double quotes not recogn…
…ized

HTML::Selector#simple_selector can recognize attributes in single and
double quotes. But when the attribute value contains square brackets,
and the value is enclosed in double quotes, an exception
`ArgumentError: Invalid selector: "]` is thrown.

Related to issue #2412.
988b6b4
@Arsen7
Arsen7 commented Aug 4, 2011

Done. By the way, you finally managed to force me to actually use github ;-)

I assume that any further discussion will be eventually placed under the pull request, so it is safe to close this issue.

@Arsen7 Arsen7 closed this Aug 4, 2011
@Arsen7 Arsen7 reopened this Nov 15, 2011
@Arsen7
Arsen7 commented Nov 15, 2011

I believe that closing the issue was a mistake. The pull request apparently went unnoticed. The problem still exists in current head.

@isaacsanders

Is this still an issue?

@vijaydev
Member
vijaydev commented May 5, 2012

Closing in favor of the PR.

@vijaydev vijaydev closed this May 5, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment