Merge pull request #3955 from wallabag/fix/gif-animation-imagick

Use Imagick to keep GIF animation

.travis.yml

@@ -54,6 +54,10 @@ before_script:
     - echo "memory_limit=-1" >> ~/.phpenv/versions/$(phpenv version-name)/etc/conf.d/travis.ini
     - phpenv config-rm xdebug.ini || echo "xdebug not available"
     - composer self-update --no-progress
+    # install imagick
+    - pear config-set preferred_state beta
+    - pecl channel-update pecl.php.net
+    - yes | pecl install imagick
 
 script:
     - travis_wait bash composer install -o --no-interaction --no-progress --prefer-dist

composer.json

@@ -103,6 +103,9 @@
         "phpstan/phpstan-symfony": "^0.11.0",
         "phpstan/phpstan-doctrine": "^0.11.0"
     },
+    "suggest": {
+        "ext-imagick": "To keep GIF animation when downloading image is enabled"
+    },
     "scripts": {
         "post-cmd": [
             "Incenteev\\ParameterHandler\\ScriptHandler::buildParameters",

src/Wallabag/CoreBundle/Helper/DownloadImages.php

@@ -135,7 +135,21 @@ class DownloadImages
 
         switch ($ext) {
             case 'gif':
-                imagegif($im, $localPath);
+                // use Imagick if available to keep GIF animation
+                if (class_exists('\\Imagick')) {
+                    try {
+                        $imagick = new \Imagick();
+                        $imagick->readImageBlob($res->getBody());
+                        $imagick->setImageFormat('gif');
+                        $imagick->writeImages($localPath, true);
+                    } catch (\Exception $e) {
+                        // if Imagick fail, fallback to the default solution
+                        imagegif($im, $localPath);
+                    }
+                } else {
+                    imagegif($im, $localPath);
+                }
+
                 $this->logger->debug('DownloadImages: Re-creating gif');
                 break;
             case 'jpeg':

tests/Wallabag/CoreBundle/GuzzleSiteAuthenticator/GrabySiteConfigBuilderTest.php

@@ -33,7 +33,7 @@ class GrabySiteConfigBuilderTest extends TestCase
         $grabyConfigBuilderMock
             ->method('buildForHost')
             ->with('example.com')
-            ->will($this->returnValue($grabySiteConfig));
+            ->willReturn($grabySiteConfig);
 
         $logger = new Logger('foo');
         $handler = new TestHandler();
@@ -93,7 +93,7 @@ class GrabySiteConfigBuilderTest extends TestCase
         $grabyConfigBuilderMock
             ->method('buildForHost')
             ->with('unknown.com')
-            ->will($this->returnValue(new GrabySiteConfig()));
+            ->willReturn(new GrabySiteConfig());
 
         $logger = new Logger('foo');
         $handler = new TestHandler();
@@ -153,7 +153,7 @@ class GrabySiteConfigBuilderTest extends TestCase
         $grabyConfigBuilderMock
             ->method('buildForHost')
             ->with('example.com')
-            ->will($this->returnValue($grabySiteConfig));
+            ->willReturn($grabySiteConfig);
 
         $logger = new Logger('foo');
         $handler = new TestHandler();

tests/Wallabag/CoreBundle/ParamConverter/UsernameFeedTokenConverterTest.php

@@ -26,7 +26,7 @@ class UsernameFeedTokenConverterTest extends TestCase
 
         $registry->expects($this->once())
             ->method('getManagers')
-            ->will($this->returnValue([]));
+            ->willReturn([]);
 
         $params = new ParamConverter([]);
         $converter = new UsernameFeedTokenConverter($registry);
@@ -42,7 +42,7 @@ class UsernameFeedTokenConverterTest extends TestCase
 
         $registry->expects($this->once())
             ->method('getManagers')
-            ->will($this->returnValue(['default' => null]));
+            ->willReturn(['default' => null]);
 
         $params = new ParamConverter([]);
         $converter = new UsernameFeedTokenConverter($registry);
@@ -58,7 +58,7 @@ class UsernameFeedTokenConverterTest extends TestCase
 
         $meta->expects($this->once())
             ->method('getName')
-            ->will($this->returnValue('nothingrelated'));
+            ->willReturn('nothingrelated');
 
         $em = $this->getMockBuilder('Doctrine\Common\Persistence\ObjectManager')
             ->disableOriginalConstructor()
@@ -67,7 +67,7 @@ class UsernameFeedTokenConverterTest extends TestCase
         $em->expects($this->once())
             ->method('getClassMetadata')
             ->with('superclass')
-            ->will($this->returnValue($meta));
+            ->willReturn($meta);
 
         $registry = $this->getMockBuilder('Doctrine\Common\Persistence\ManagerRegistry')
             ->disableOriginalConstructor()
@@ -75,12 +75,12 @@ class UsernameFeedTokenConverterTest extends TestCase
 
         $registry->expects($this->once())
             ->method('getManagers')
-            ->will($this->returnValue(['default' => null]));
+            ->willReturn(['default' => null]);
 
         $registry->expects($this->once())
             ->method('getManagerForClass')
             ->with('superclass')
-            ->will($this->returnValue($em));
+            ->willReturn($em);
 
         $params = new ParamConverter(['class' => 'superclass']);
         $converter = new UsernameFeedTokenConverter($registry);
@@ -96,7 +96,7 @@ class UsernameFeedTokenConverterTest extends TestCase
 
         $meta->expects($this->once())
             ->method('getName')
-            ->will($this->returnValue('Wallabag\UserBundle\Entity\User'));
+            ->willReturn('Wallabag\UserBundle\Entity\User');
 
         $em = $this->getMockBuilder('Doctrine\Common\Persistence\ObjectManager')
             ->disableOriginalConstructor()
@@ -105,7 +105,7 @@ class UsernameFeedTokenConverterTest extends TestCase
         $em->expects($this->once())
             ->method('getClassMetadata')
             ->with('WallabagUserBundle:User')
-            ->will($this->returnValue($meta));
+            ->willReturn($meta);
 
         $registry = $this->getMockBuilder('Doctrine\Common\Persistence\ManagerRegistry')
             ->disableOriginalConstructor()
@@ -113,12 +113,12 @@ class UsernameFeedTokenConverterTest extends TestCase
 
         $registry->expects($this->once())
             ->method('getManagers')
-            ->will($this->returnValue(['default' => null]));
+            ->willReturn(['default' => null]);
 
         $registry->expects($this->once())
             ->method('getManagerForClass')
             ->with('WallabagUserBundle:User')
-            ->will($this->returnValue($em));
+            ->willReturn($em);
 
         $params = new ParamConverter(['class' => 'WallabagUserBundle:User']);
         $converter = new UsernameFeedTokenConverter($registry);
@@ -149,7 +149,7 @@ class UsernameFeedTokenConverterTest extends TestCase
         $repo->expects($this->once())
             ->method('findOneByUsernameAndFeedToken')
             ->with('test', 'test')
-            ->will($this->returnValue(null));
+            ->willReturn(null);
 
         $em = $this->getMockBuilder('Doctrine\Common\Persistence\ObjectManager')
             ->disableOriginalConstructor()
@@ -158,7 +158,7 @@ class UsernameFeedTokenConverterTest extends TestCase
         $em->expects($this->once())
             ->method('getRepository')
             ->with('WallabagUserBundle:User')
-            ->will($this->returnValue($repo));
+            ->willReturn($repo);
 
         $registry = $this->getMockBuilder('Doctrine\Common\Persistence\ManagerRegistry')
             ->disableOriginalConstructor()
@@ -167,7 +167,7 @@ class UsernameFeedTokenConverterTest extends TestCase
         $registry->expects($this->once())
             ->method('getManagerForClass')
             ->with('WallabagUserBundle:User')
-            ->will($this->returnValue($em));
+            ->willReturn($em);
 
         $params = new ParamConverter(['class' => 'WallabagUserBundle:User']);
         $converter = new UsernameFeedTokenConverter($registry);
@@ -187,7 +187,7 @@ class UsernameFeedTokenConverterTest extends TestCase
         $repo->expects($this->once())
             ->method('findOneByUsernameAndFeedtoken')
             ->with('test', 'test')
-            ->will($this->returnValue($user));
+            ->willReturn($user);
 
         $em = $this->getMockBuilder('Doctrine\Common\Persistence\ObjectManager')
             ->disableOriginalConstructor()
@@ -196,7 +196,7 @@ class UsernameFeedTokenConverterTest extends TestCase
         $em->expects($this->once())
             ->method('getRepository')
             ->with('WallabagUserBundle:User')
-            ->will($this->returnValue($repo));
+            ->willReturn($repo);
 
         $registry = $this->getMockBuilder('Doctrine\Common\Persistence\ManagerRegistry')
             ->disableOriginalConstructor()
@@ -205,7 +205,7 @@ class UsernameFeedTokenConverterTest extends TestCase
         $registry->expects($this->once())
             ->method('getManagerForClass')
             ->with('WallabagUserBundle:User')
-            ->will($this->returnValue($em));
+            ->willReturn($em);
 
         $params = new ParamConverter(['class' => 'WallabagUserBundle:User', 'name' => 'user']);
         $converter = new UsernameFeedTokenConverter($registry);

tests/Wallabag/UserBundle/EventListener/CreateConfigListenerTest.php

@@ -68,7 +68,7 @@ class CreateConfigListenerTest extends TestCase
 
         $this->em->expects($this->once())
             ->method('persist')
-            ->will($this->returnValue($config));
+            ->willReturn($config);
         $this->em->expects($this->once())
             ->method('flush');