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

Attribute selector values containing # broke in 2.4.0 #495

Closed
rixth opened this issue Jul 11, 2016 · 13 comments
Closed

Attribute selector values containing # broke in 2.4.0 #495

rixth opened this issue Jul 11, 2016 · 13 comments

Comments

@rixth
Copy link

rixth commented Jul 11, 2016

#314 caused a regression. It broke attribute selectors with values containing #, ex a[href='/page#anchor'].

Failing test case:

it('should query attributes with hashes in their values', () => {
  const wrapper = mount(
    <div>
      <a href="/page">Hello</a>
      <a href="/page#anchor">World</a>
    </div>
  );
  expect(wrapper.find("a[href='/page#anchor']")).to.have.length(1);
});
@aweary aweary added the bug label Jul 11, 2016
@rixth
Copy link
Author

rixth commented Jul 11, 2016

In the function splitSelector in Utils.js, the regular expression /(?=\.|\[.*\])|(?=#|\[#.*\])/ is a little over-zealous.

Input: a[href='/page#anchor']
Output before change: [ 'a', '[href=\'/page#anchor\']' ]
Output after change:  [ 'a', '[href=\'/page', '#anchor\']' ]

@aweary
Copy link
Collaborator

aweary commented Jul 11, 2016

Well, it's not necessary over-zealous, it's just not smart enough to handle compound selectors and attribute values containing #. #458 would likely fix this, but feel free to work on a PR to fix the regex for now, since #458 still has issues that haven't been worked out.

@ljharb
Copy link
Member

ljharb commented Jul 11, 2016

fwiw, before releasing or merging anything new, we urgently need to either fix this behavior regression or revert #314.

@aweary
Copy link
Collaborator

aweary commented Jul 11, 2016

Reverting #387 would be easiest if we need an urgent fix, I'm not 100% sure how difficult it's going to be to get this behavior working correctly. That regex is getting unwieldy

@aweary
Copy link
Collaborator

aweary commented Aug 17, 2016

@ljharb should we just revert #387? It seems that fixing this behavior is going to be non-trivial, and we've already let this sit for quite a while.

@ljharb
Copy link
Member

ljharb commented Aug 17, 2016

Yes, I think we should.

@ljharb
Copy link
Member

ljharb commented Aug 17, 2016

Let's keep the test cases tho, skipped.

@mik01aj
Copy link

mik01aj commented Aug 19, 2016

Maybe #547 is related to this too?

@rixth
Copy link
Author

rixth commented Aug 19, 2016

@mik01aj I don't think so -- the code that caused this bug was not present in 2.3.0 (as mentioned in the linked ticket). It's possible it's still this regex that's "responsible", though.

@rixth
Copy link
Author

rixth commented Nov 9, 2016

Why was 2.5.0+ released given that it was a regression, and there was earlier talk of reverting the PR that introduced it?

@ljharb
Copy link
Member

ljharb commented Nov 9, 2016

I think this has slipped through the cracks. When we get a fix in, I'll backport it to both the 2.4.x and 2.5.x lines.

ljharb added a commit to ljharb/enzyme that referenced this issue Nov 9, 2016
ljharb added a commit to ljharb/enzyme that referenced this issue Nov 9, 2016
@ljharb
Copy link
Member

ljharb commented Nov 9, 2016

I've put up #670 which fixes this issue without reverting #387.

ljharb added a commit to ljharb/enzyme that referenced this issue Nov 9, 2016
@ljharb ljharb closed this as completed in f979d99 Nov 9, 2016
ljharb added a commit that referenced this issue Nov 9, 2016
[Fix] `mount`/`shallow`: fix ID selectors

Fixes #495.
@ljharb
Copy link
Member

ljharb commented Nov 10, 2016

v2.4.2, v2.5.2, and v2.6.0 are all now published and all contain this fix.

Sorry for the delay!

ljharb added a commit that referenced this issue Nov 10, 2016
[Fix] `mount`/`shallow`: fix ID selectors

Fixes #495.
ljharb added a commit that referenced this issue Nov 10, 2016
[Fix] `mount`/`shallow`: fix ID selectors

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

No branches or pull requests

4 participants