Skip to content

Commit

Permalink
Selector: Remove "#" exception for identifier tokens
Browse files Browse the repository at this point in the history 
Port Sizzle test change from:
jquery/sizzle@f204a61

(cherry-picked from 86e62d8)
  • Loading branch information
@mgol
mgol committed Jun 30, 2014
1 parent b3edc61 commit 41f522a
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions test/unit/selector.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,8 @@ test("attributes", function() {
t( "Attribute Equals Number", "#qunit-fixture li[tabIndex=-1]", ["foodWithNegativeTabIndex"] );

document.getElementById("anchor2").href = "#2";
t( "href Attribute", "p a[href^=#]", ["anchor2"] );
t( "href Attribute", "p a[href*=#]", ["simon1", "anchor2"] );
t( "href Attribute", "p a[href^='#']", ["anchor2"] );
t( "href Attribute", "p a[href*='#']", ["simon1", "anchor2"] );

t( "for Attribute", "form label[for]", ["label-for"] );
t( "for Attribute in form", "#form [for=action]", ["label-for"] );
Expand Down

10 comments on commit 41f522a

@Basilakis
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Half web broken down, compatibility with older versions has to be Number 1 rule.

@Basilakis
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Next time, learn about backwords compatibility.

@FagnerMartinsBrack
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is backwards compatible, see #2824 (comment). Although I agree that, if it is being tested, it should be mentioned in the changelog.

@timmywil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is in the changelog, but was not given special attention in the blog post. It was an invalid selector that unfortunately got used. The good news is that by adding the quotes, you will not only be using a valid selector, but you'll make it faster.

@georgestephanis
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe running the following command against your codebase will turn up instances of the old inaccurate selector, that you can correct to make your code work correctly with jQuery 1.12 --

ack "\[href.?=#"

(the .? is to catch either *= and ^= -- the latter of which I can't seem to get working without the . wildcard)

Hopefully this should help some folks find the bit in the code that they're using that is conflicting with this changeset.

@dmethvin
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We warn about this in jQuery Migrate, but cannot auto-fix it because a regex cannot parse the selector well enough to fix the invalid cases without also breaking valid ones.

AFAICT this seems to be breaking a couple of common selector expressions that are either in a relatively popular plugin or copy-pastaed from StackOverflow. jQuery Migrate should pinpoint them for you, including a stack trace. If you find that it is coming from a plugin you are using, look for an update and if there isn't one please submit a pull request to the author to fix it.

@georgestephanis
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dmethvin Yeah, there were several WordPress themes doing it the old incorrect way that didn't test against the betas, so yesterday when the 4.5 update shipped, breakage :( . We're trying to get the word out on our side and scan as many as we can and notify theme/plugin authors on our side, hopefully that'll be able to take some of the heat off y'all and get things running merrily for most users soon.

@emcniece
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This theme has over 87,000 sales and uses this selector in such a way that it breaks huge parts of the theme (namely image-related animation-y things). The problem is that while WordPress auto-updates, the theme does not - you have to enter your marketplace API credentials, and initiate the theme update manually... and you can't do that if you have modified the theme files.

It's gonna be a busy week!

@mgol
Copy link
Member Author

@mgol mgol commented on 41f522a Apr 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@emcniece Such problems are unfortunately inevitable from time to time. That's why we advise to pin jQuery versions and upgrade by yourself even though we now technically follow semver.

@geliyoo
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi i can fixed from my js file href problem with this code.
a[href=#]:not([href=#])
to
a[href=#]:not([href=#])

Can you check please if it possiblem fixed from jquery

Please sign in to comment.