From: Jérémy Benoist <j0k3r@users.noreply.github.com>
Date: Fri, 10 May 2019 18:24:06 +0000 (+0300)
Subject: Merge pull request #3955 from wallabag/fix/gif-animation-imagick
X-Git-Url: https://git.immae.eu/?a=commitdiff_plain;h=de1162b91a205a98a3f8ed01bd80285793b18380;hp=6e67f41152ad025e74696e7140de0f9fb0d601de;p=github%2Fwallabag%2Fwallabag.git

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

Use Imagick to keep GIF animation
---

diff --git a/.travis.yml b/.travis.yml
index 393d0033..8c1ec5cb 100644
--- a/.travis.yml
+++ b/.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
diff --git a/composer.json b/composer.json
index b28404e3..b1c144c7 100644
--- a/composer.json
+++ b/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",
diff --git a/src/Wallabag/CoreBundle/Helper/DownloadImages.php b/src/Wallabag/CoreBundle/Helper/DownloadImages.php
index cc3dcfce..9a7e9828 100644
--- a/src/Wallabag/CoreBundle/Helper/DownloadImages.php
+++ b/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':
diff --git a/tests/Wallabag/CoreBundle/GuzzleSiteAuthenticator/GrabySiteConfigBuilderTest.php b/tests/Wallabag/CoreBundle/GuzzleSiteAuthenticator/GrabySiteConfigBuilderTest.php
index 7beccd30..277d8012 100644
--- a/tests/Wallabag/CoreBundle/GuzzleSiteAuthenticator/GrabySiteConfigBuilderTest.php
+++ b/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();
diff --git a/tests/Wallabag/CoreBundle/ParamConverter/UsernameFeedTokenConverterTest.php b/tests/Wallabag/CoreBundle/ParamConverter/UsernameFeedTokenConverterTest.php
index 92fe38cd..48c82dde 100644
--- a/tests/Wallabag/CoreBundle/ParamConverter/UsernameFeedTokenConverterTest.php
+++ b/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);
diff --git a/tests/Wallabag/UserBundle/EventListener/CreateConfigListenerTest.php b/tests/Wallabag/UserBundle/EventListener/CreateConfigListenerTest.php
index fd32f380..d976c4ac 100644
--- a/tests/Wallabag/UserBundle/EventListener/CreateConfigListenerTest.php
+++ b/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');