MyIntervals / emogrifier

Converts CSS styles into inline style attributes in your HTML code.
https://www.myintervals.com/emogrifier.php
MIT License
906 stars 154 forks source link

Parsing CSS with animations will lead to an fatal error in DOMXPath #628

Closed elakeduck closed 5 years ago

elakeduck commented 5 years ago

Following CSS will lead to an fatal error in emogrifier V1.2.1:

@keyframes pulse {
  0% {
    background-color: #001F3F;
  }
  100% {
    background-color: #FF4136;
  }
}

The thrown error: Fatal error: Uncaught InvalidArgumentException: DOMXPath::query(): Invalid expression in selector >> 100% << in Emogrifier.php on line 371 in Emogrifier.php:1533 Stack trace: #0 [internal function]: Pelago\Emogrifier->handleXpathError(2, 'DOMXPath::query...', '...', 371, Array) #1 Emogrifier.php(371): DOMXPath->query('//100%') #2 Emogrifier.php(289): Pelago\Emogrifier->process(Object(DOMDocument)) #3 functions_inc.php(1182): Pelago\Emogrifier->emogrify() #4 form.php(117): sendmail('mich in Emogrifier.php on line 1533

The parsed css rule looks like this:

Array
        (
            [selector] => 100%
            [declarationsBlock] => background-color: #FF4136;
            [line] => 6
        )

I guess removing animations before parsing the css will do the trick.

elakeduck commented 5 years ago

Using the newer version 2.0.0 seems to not have this issue.

oliverklee commented 5 years ago

Thanks for the feedback. I'm glad to read that this works for you now. Closing.

riversy commented 5 years ago

Hello, It still reproduced for me in 2.1.1. The following piece of styles causes error: DOMXPath::query(): Invalid expression in selector >> 100% <<

@keyframes fadeOut {
  0% {
    opacity: 1;
  }
  100% {
    opacity: 0;
  }
}
riversy commented 5 years ago

To test it I added the following test method into tests/Unit/EmogrifierTest.php:

    /**
     * @test
     */
    public function emogrifyAnimationCssInDebugModeShouldNotThrowsException()
    {
        $this->subject->setDebug(true);

        $this->subject->setHtml('<html></html>');
        $this->subject->setCss('@keyframes fadeOut { opacity: 1; } 100% { opacity: 0;}}');

        $this->subject->emogrify();

        static::assertTrue(true);
    }

And as result it caused the error: image

oliverklee commented 5 years ago

@riversy Thanks for reporting. As your CSS is different, could you please open a new ticket for this? Thanks!