Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore support for IDs containing a colon #214

Merged
merged 2 commits into from
Apr 7, 2017

Conversation

duncan3dc
Copy link
Contributor

#163 broke support for IDs containing a colon, eg:

<input type='text' id='#frmLogin:strCustomerLogin_userID'>

This pr restores it and adds a test

@taylorotwell taylorotwell merged commit 9a150be into laravel:master Apr 7, 2017
@taylorotwell
Copy link
Member

@JExperton any comment on this? Does this look ok?

@jxprtn
Copy link

jxprtn commented Apr 8, 2017

Two things:

  • The given example #frmLogin:strCustomerLogin_userID with a leading hash in the ID attribute won't match because the hash character is not part of the "\w" class.
    Generally speaking, you shouldn't start an ID attribute with the # character because it doesn't work in a stylesheet. I guess it's just a confusion in the example between CSS selector and HTML attribute.

  • Using a colon in an ID doesn't work in a stylesheet because the character is meant to be used in pseudo-selectors such as :hover or :first-child. Not a good idea to use it in an ID.

However, it looks good to me since an ID should be unique and there's no point to combine it with a pseudo-selector. Even if selectors like #element:first-child or #element:visited will match, I think it's ok in the context of Dusk.

I was thinking about replacing the regex with ^#[\S]+ to fit the W3C definition of ID and avoid future requests to add support for other characters. Then, selectors such as #element[prop] and #element>child will match which is not an expected behavior but might be acceptable if we take for granted that developers know what they are doing.

The question is do we stick to patterns that work in a stylesheet or to the W3C definition of an ID?

@duncan3dc duncan3dc deleted the id-with-colon branch April 9, 2017 15:06
@duncan3dc
Copy link
Contributor Author

I'm +1 on the W3C definition, looking for whitespace to end the ID

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants