simple_selector: attributes with brackets in double quotes not recognized #2418

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
6 participants
Contributor

Arsen7 commented Aug 4, 2011

simple_selector: attributes with brackets in double quotes not recognized

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.

@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
Contributor

isaacsanders commented Apr 28, 2012

@Arsen7 Is this still an issue?

Contributor

Arsen7 commented May 9, 2012

@isaacsanders Yes, it is. If I change the test, as shown in my diff, and run cd actionpack && rake test, then the test fails. If I apply the fix in that regular expression and run rake test, all the tests pass.

I have tested the issue on the 3-2-stable branch (pulled just 5 minutes ago). I do not have ruby1.9.

Contributor

kytrinyx commented Jul 14, 2012

I applied this patch to master (at d08fee3) and verified that all the tests run using ruby 1.9.3-p194.

Owner

rafaelfranca commented Jul 14, 2012

@kytrinyx thank you. But we are going to remove the html-scanner with a better solution that will solve some issues. I'm assigning this pull request to me and will close when fixed.

rafaelfranca was assigned Jul 14, 2012

Member

steveklabnik commented Nov 17, 2012

Since this is vendored code, is it even eligible to be touched?

As far as I know, html-scanner is frozen the same was as the inflector. As @rafaelfranca said, I think we'll have to wait.

Member

steveklabnik commented Nov 17, 2012

Right; I guess what I'm saying is, if it's not going to get changed, then why even keep the issue open?

I think @rafaelfranca wants to keep track of it. But lets ask him later.

Owner

rafaelfranca commented Nov 18, 2012

I'll close the issue and add it to my icebox. I know the solution but it will add another dependency to Rails, and it is the blocker now.

@steveklabnik @carlosantoniodasilva ❤️

Member

steveklabnik commented Nov 18, 2012

Yeah. @wycats said that he'd be okay with it for Tokaido, but causing Rails to have a cext dependency is a big step.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment