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

Noisy error on malformed CSS #237

Closed
Synchro opened this issue Sep 30, 2015 · 7 comments · Fixed by #319
Closed

Noisy error on malformed CSS #237

Synchro opened this issue Sep 30, 2015 · 7 comments · Fixed by #319

Comments

@Synchro
Copy link
Contributor

Synchro commented Sep 30, 2015

Much like #179, I've been getting this from something malformed:

2015-09-30 14:43:49 DOMXPath::query(): Invalid predicate
#0 System_ErrorHandler->ERROR_HANDLER
#1 DOMXPath->query line 287
#2 Pelago\Emogrifier->process line 211
#3 Pelago\Emogrifier->emogrify line 44

That line is:

$nodesMatchingCssSelectors = $xpath->query($this->translateCssToXpath($selector['selector']));

So it may well be fixed by a fix for #179 by eliminating the bogus selector, but it shouldn't throw a noisy error like this anyway. Better to just ignore the content. When I've tracked down the content responsible for this error, I'll add it in here so we can build a test for it.

@oliverklee oliverklee added the bug label Oct 1, 2015
@oliverklee
Copy link
Contributor

What CSS is causing this problem?

@oliverklee oliverklee added this to the 1.0.0 milestone Oct 9, 2015
@oliverklee oliverklee modified the milestones: 1.1.0, 1.0.0 Oct 10, 2015
@Renkas
Copy link
Contributor

Renkas commented Nov 15, 2015

I got the same problem.

Maybe this helps to track down the problem?

PHP Warning – yii\base\ErrorException

DOMXPath::query(): Invalid predicate
in /var/www/vendor/pelago/emogrifier/Classes/Emogrifier.php – DOMXPath::query('//a[href^="tel"]') at line 745

@oliverklee oliverklee changed the title Invalid predicate in DOMXPath query Support ^ attribute selectors Nov 15, 2015
@oliverklee
Copy link
Contributor

Okay, so it looks like Emogrifier currently does not support negative attribute selectors (i.e., this is missing feature). Anyone willing to implement this?

@oliverklee oliverklee removed this from the 1.1.0 milestone Nov 15, 2015
@Synchro
Copy link
Contributor Author

Synchro commented Nov 15, 2015

That just sounds like XPath and CSS selectors do not share the same syntax for that kind of lookup. I suspect in my case it was simply malformed CSS rather than a missing feature - e.g. if someone puts HTML tags in the CSS, which I've seen. From the source of my CSS, I strongly suspect it's just random junk rather than anything as sophisticated as this missing selector, so the rename may not be quite appropriate.

@oliverklee oliverklee added this to the 1.1.0 milestone Nov 15, 2015
@oliverklee oliverklee changed the title Support ^ attribute selectors Noisy error on malformed CSS Nov 15, 2015
@oliverklee
Copy link
Contributor

Okay, I've reverted the changes to this ticket. Thanks for the input! (We already have #227 for the missing feature anyway.)

@oliverklee
Copy link
Contributor

@Renkas: So it looks like you're experiencing #227.

@Synchro
Copy link
Contributor Author

Synchro commented Apr 27, 2016

I've managed to track down some CSS that's causing this error - it's an empty media query.

In the Emogrifier class, the extractMediaQueriesFromCss method finds the media queries, but having an empty one makes it go out of sync. Note that it will only go wrong if you have another media query after the empty one.

I've set up the content and the regex it uses here, and you can see it going wrong.

Following on from that, when it gets to existsMatchForCssSelector, the first CSS selector it passes is:

}

table.body

Which is invalid, but still gets translated into the invalid XPath:

//}//table[contains(concat(" ",@class," "),concat(" ","body"," "))]

After which I get the error:

2016-04-27 14:43:19 DOMXPath::query(): Invalid expression
#0 My_ErrorHandler->ERROR_HANDLER
#1 DOMXPath->query line 956
#2 Pelago\Emogrifier->existsMatchForCssSelector line 913
#3 Pelago\Emogrifier->copyCssWithMediaToStyleNode line 391
#4 Pelago\Emogrifier->process line 276
#5 Pelago\Emogrifier->emogrify line 44

So it would be good if Emogrifier could cope with a missing media query body.

Meanwhile, I'll try to track down why my media queries are empty - I supect an old version of CSSTidy may be at fault.

Synchro added a commit to Synchro/emogrifier that referenced this issue Apr 28, 2016
Synchro added a commit to Synchro/emogrifier that referenced this issue Apr 28, 2016
Synchro added a commit to Synchro/emogrifier that referenced this issue Apr 28, 2016
Synchro added a commit to Synchro/emogrifier that referenced this issue May 8, 2016
Synchro added a commit to Synchro/emogrifier that referenced this issue May 8, 2016
oliverklee pushed a commit that referenced this issue Jun 13, 2016
This fixes some noisy errors for empty media queries.

Fixes #237
Fixes #307
oliverklee pushed a commit that referenced this issue Jun 13, 2016
This fixes some noisy errors for empty media queries.

Fixes #237
Fixes #307
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants