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

alert.js: Avoid calling jQuery('#'), it's a syntax error in jQuery 3.0 #20019

Merged
merged 1 commit into from
Jul 12, 2016

Conversation

dmethvin
Copy link
Contributor

@dmethvin dmethvin commented Jun 2, 2016

No description provided.

@twbs-savage
Copy link

Tests passed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: CONFIRMED

Commit: dd77ecc
Build details: https://travis-ci.org/twbs-savage/bootstrap/builds/134788284

(Please note that this is a fully automated comment.)

@cvrebert
Copy link
Collaborator

@dmethvin Is there a plausible scenario where this could occur that doesn't involve user error?

@cvrebert cvrebert mentioned this pull request Jun 28, 2016
2 tasks
@XhmikosR
Copy link
Member

I wonder if this is the same error $('a[href*=#].

@dmethvin
Copy link
Contributor Author

Is there a plausible scenario where this could occur that doesn't involve user error?

I'm honestly not sure but since it was happening in a unit test I assumed it might be done by users as well.

I wonder if this is the same error $('a[href*=#].

No, although that is also an invalid CSS selector. Special characters in attribute values must be quoted, per the spec.

@XhmikosR
Copy link
Member

No, although that is also an invalid CSS selector. Special characters in attribute values must be quoted, per the spec.

Agreed, it's just that it stop working in a patch version build so it was a little unexpected :/

Now, about this PR, do you think you can add a test for this case?

@dmethvin
Copy link
Contributor Author

Now, about this PR, do you think you can add a test for this case?

It was failing an existing test case before this PR, so I figured that fixing that one was enough. The failing test has this markup.

@XhmikosR
Copy link
Member

Oh, cool. Then I have no objection in merging this. @cvrebert push the button if you agree.

@cvrebert
Copy link
Collaborator

The current v3 docs don't use the <a class="close" href="#" data-dismiss="alert"> markup anywhere. v3.0.0's docs had it in one place, where the live demo and the syntax-highlighted code were out of sync 😢

On account of compatibility paranoia, I'm okay with merging this.

@cvrebert cvrebert merged commit 1956146 into twbs:master Jul 12, 2016
@mdo mdo mentioned this pull request Jul 12, 2016
n0nick added a commit to wazeHQ/bootstrap that referenced this pull request Dec 14, 2016
Calling jQuery('#') is considered a syntax error in jQuery 3.
This change avoids this issue, similar to the change in arrow.js
introduced in twbs#20019.
n0nick added a commit to wazeHQ/bootstrap-sass that referenced this pull request Dec 14, 2016
Calling jQuery('#') is considered a syntax error in jQuery 3.
This change avoids this issue, similar to the change in arrow.js
introduced in twbs/bootstrap#20019.
chiraggmodi pushed a commit to chiraggmodi/bootstrap that referenced this pull request Apr 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants