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

Invalid selector is causing unrecoverable error #19

Closed
givankin opened this issue Apr 8, 2016 · 18 comments
Closed

Invalid selector is causing unrecoverable error #19

givankin opened this issue Apr 8, 2016 · 18 comments
Assignees

Comments

@givankin
Copy link

givankin commented Apr 8, 2016

Hi again Philip, it's been a while!

I am having the following situation and I wonder if you could give any advice here. It's possible that it's not your parser's responsibility but perhaps you could point to a way to work around this with your APIs.

So, one of the goals of our application when it comes to CSS parsing is to transform all the url()-s in it. We do so by using the CSSVisitor.visitCSSUrl() method.

However, when we have CSS like this:

body {
    background: url(../bg.png);
}
.#f { color: red; }

the parser fails unrecoverably on the .#f selector and so we're unable to transform the URLs. We're using v4.0.0 with CSS3 parser and .setBrowserCompliantMode(true) - which, as I imagined, shouldn't have treated such error as unrecoverable - as browser doesn't.

Sorry if my questions are noob - I am by no means a Java programmer. Thanks

@phax
Copy link
Owner

phax commented Apr 8, 2016

I will look at it after my vacation :)

@givankin
Copy link
Author

Thanks! Have a great vacation : )

@phax
Copy link
Owner

phax commented Apr 18, 2016

The problem is the invalid selector. As you may know, there can be many selectors (e.g. .a,.b,.c) and I need to know how browsers handle this. Do they skip only the selector parts they cannot handle or the whole rule.
Basically the same problem as in subissue 3 of #17

@phax phax self-assigned this Apr 18, 2016
@givankin
Copy link
Author

@phax, according to Specs, the entire rule should be dropped. See https://www.w3.org/TR/css3-selectors/#Conformance:

In the case of CSS, the entire rule in which the selector is used is dropped.

and https://www.w3.org/TR/CSS2/syndata.html#rule-sets:

...the whole statement should be ignored if there is an error anywhere in the selector...

@givankin
Copy link
Author

I've also created a simple test and all browsers seem to comply with the specs: https://jsfiddle.net/aqdtzcac/1/ (tried Chrome, FF, IE 9/10/11, Safari on iOS and some androids as well).

@phax
Copy link
Owner

phax commented Apr 18, 2016

Thanks for the clarification. I'm working on a solution that works for you when you enable "browser compliant mode".

@givankin
Copy link
Author

Thanks Philip, good to know =)
Will it work for invalid selectors only or more generally for any problem in rule parsing?

@phax
Copy link
Owner

phax commented Apr 18, 2016

For invalid selectors only. As the parser is grammar based I need to differentiate between recoverable and unrecoverable errors. Creating a completely lax parser is currently out of scope :)

@givankin
Copy link
Author

@phax just so I know, do you have an estimate for the fix?

@phax
Copy link
Owner

phax commented Apr 19, 2016

within this week - but after 6 weeks of vacation there is plenty to do :)

@phax
Copy link
Owner

phax commented Apr 19, 2016

I have it locally working but been struggeling with some tests....

@phax
Copy link
Owner

phax commented Apr 20, 2016

Locally things are working. Please check with the latest SNAPSHOT

@givankin
Copy link
Author

Hi Philip, it is working just fine! The only question is - if it is going to be released in 5.0 stream, it will be incompatible with Java 1.7, right? Any chance to include this into 4.0 stream? We can't drop support for Java 7 unfortunately now...

@givankin
Copy link
Author

If necessary, we can contribute - under your guidance. We highly value your project and depend on it heavily, so we would be glad to do that.

@phax
Copy link
Owner

phax commented Apr 20, 2016

Well I already created a 4.1 backport and I think I can do the same for this fix.
Let me check tonight, if a 4.1.1 can be done with reasonable effort.
A donation link can be found at http://peppol.helger.com/public :)

@phax
Copy link
Owner

phax commented Apr 20, 2016

Release 4.1.1 is on its way to Maven central

@phax phax closed this as completed Apr 20, 2016
@pmouawad
Copy link

@phax , it's not a big thing, but just to let you know I added a mention of you and your project in our changes.html#thanks section
: apache/jmeter@d607b3a

Apache JMeter 3.0 will be using your library.

@phax
Copy link
Owner

phax commented Apr 20, 2016

Thanks for the info and good luck for JMeter!! It's a great tool I'm also using!

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

No branches or pull requests

3 participants