Skip to content

Conversation

@ludofischer
Copy link
Contributor

I think css-selector-parser fails to correctly parse attribute selectors that contain new lines, as it believes the quotes are part of the value. This makes it hard to port postcss-minify-selectors to the css-selector-parser 6.0.

@alexander-akait
Copy link
Collaborator

Feel free to send a fix, should be easy

@alexander-akait
Copy link
Collaborator

@ludofischer ludofischer changed the title test: demonstrate failing quote recognition when string contains newline fix: fix parsing of attributes that contain newline character May 7, 2021
Also match new lines in the regex that checks for quoted attributes.
@ludofischer
Copy link
Contributor Author

Here place to fix https://github.com/postcss/postcss-selector-parser/blob/master/src/selectors/attribute.js (I think)

Seems changing a regex was all that was needed.

@ludofischer ludofischer marked this pull request as ready for review May 7, 2021 21:09
@coveralls
Copy link

Coverage Status

Coverage remained the same at 95.435% when pulling 40bc7cd on ludofischer:newline-bug into 54f71ef on postcss:master.

t.deepEqual(tree.nodes[0].nodes[0].operator, '=');
t.deepEqual(tree.nodes[0].nodes[0].value, 'woop \nwoop woop');
t.true(tree.nodes[0].nodes[0].quoted);
});
Copy link
Collaborator

@alexander-akait alexander-akait May 10, 2021

Choose a reason for hiding this comment

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

Let's add other popular example:

// http://jsfiddle.net/BoltClock/AHuvh/6
a[href="\a  http://google.com"] {
}

Just avoid future regressions. Anyway good job!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately this example fails at the moment. It turns into \a into a (removes only the \). It seems a different problem though.

@alexander-akait alexander-akait merged commit c5035bc into postcss:master May 11, 2021
@ludofischer ludofischer deleted the newline-bug branch May 11, 2021 17:48
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