Don't try to display a custom logo or favicon if they don't exist (#10000)

* fix #9966 Do not display a custom logo or favicon in admin if they don't exist

Prevents 404 errors.

* refs #9966 Fix the custom logo instant preview

* refs #9966 Fix style

[ci skip]

* refs #9966 Introduce CustomLogo::logoExists() to remove duplicated code

4 files changed, 59 insertions, 24 deletions

diff --git a/plugins/CoreAdminHome/Controller.php b/plugins/CoreAdminHome/Controller.php
index 7fa434b21f..2511b74686 100644
--- a/plugins/CoreAdminHome/Controller.php
+++ b/plugins/CoreAdminHome/Controller.php
@@ -72,7 +72,9 @@ class Controller extends ControllerAdmin
         $view->branding              = array('use_custom_logo' => $logo->isEnabled());
         $view->fileUploadEnabled     = $logo->isFileUploadEnabled();
         $view->logosWriteable        = $logo->isCustomLogoWritable();
+        $view->hasUserLogo           = CustomLogo::hasUserLogo();
         $view->pathUserLogo          = CustomLogo::getPathUserLogo();
+        $view->hasUserFavicon        = CustomLogo::hasUserFavicon();
         $view->pathUserFavicon       = CustomLogo::getPathUserFavicon();
         $view->pathUserLogoSmall     = CustomLogo::getPathUserLogoSmall();
         $view->pathUserLogoSVG       = CustomLogo::getPathUserSvgLogo();
diff --git a/plugins/CoreAdminHome/CustomLogo.php b/plugins/CoreAdminHome/CustomLogo.php
index 32fa636dd3..75948fdc12 100644
--- a/plugins/CoreAdminHome/CustomLogo.php
+++ b/plugins/CoreAdminHome/CustomLogo.php
@@ -24,7 +24,7 @@ class CustomLogo
     {
         $defaultLogo = 'plugins/Morpheus/images/logo.png';
         $themeLogo = 'plugins/%s/images/logo.png';
-        $userLogo = CustomLogo::getPathUserLogo();
+        $userLogo = static::getPathUserLogo();
         return $this->getPathToLogo($pathOnly, $defaultLogo, $themeLogo, $userLogo);
     }
 
@@ -32,7 +32,7 @@ class CustomLogo
     {
         $defaultLogo = 'plugins/Morpheus/images/logo-header.png';
         $themeLogo = 'plugins/%s/images/logo-header.png';
-        $customLogo = CustomLogo::getPathUserLogoSmall();
+        $customLogo = static::getPathUserLogoSmall();
         return $this->getPathToLogo($pathOnly, $defaultLogo, $themeLogo, $customLogo);
     }
 
@@ -40,7 +40,7 @@ class CustomLogo
     {
         $defaultLogo = 'plugins/Morpheus/images/logo.svg';
         $themeLogo = 'plugins/%s/images/logo.svg';
-        $customLogo = CustomLogo::getPathUserSvgLogo();
+        $customLogo = static::getPathUserSvgLogo();
         $svg = $this->getPathToLogo($pathOnly, $defaultLogo, $themeLogo, $customLogo);
         return $svg;
     }
@@ -67,9 +67,7 @@ class CustomLogo
             return true;
         }
 
-        if ($this->isEnabled()
-            && file_exists(Filesystem::getPathToPiwikRoot() . '/' . CustomLogo::getPathUserSvgLogo())
-        ) {
+        if ($this->isEnabled() && static::logoExists(static::getPathUserSvgLogo())) {
             return true;
         }
 
@@ -111,8 +109,6 @@ class CustomLogo
 
     protected function getPathToLogo($pathOnly, $defaultLogo, $themeLogo, $customLogo)
     {
-        $pathToPiwikRoot = Filesystem::getPathToPiwikRoot();
-
         $logo = $defaultLogo;
 
         $theme = \Piwik\Plugin\Manager::getInstance()->getThemeEnabled();
@@ -123,39 +119,38 @@ class CustomLogo
         }
         $themeLogo = sprintf($themeLogo, $themeName);
 
-        if (file_exists($pathToPiwikRoot . '/' . $themeLogo)) {
+        if (static::logoExists($themeLogo)) {
             $logo = $themeLogo;
         }
-        if ($this->isEnabled()
-            && file_exists($pathToPiwikRoot . '/' . $customLogo)
-        ) {
+        if ($this->isEnabled() && static::logoExists($customLogo)) {
             $logo = $customLogo;
         }
 
         if (!$pathOnly) {
             return SettingsPiwik::getPiwikUrl() . $logo;
         }
-        return $pathToPiwikRoot . '/' . $logo;
+
+        return Filesystem::getPathToPiwikRoot() . '/' . $logo;
     }
 
     public static function getPathUserLogo()
     {
-        return self::rewritePath('misc/user/logo.png');
+        return static::rewritePath('misc/user/logo.png');
     }
 
     public static function getPathUserFavicon()
     {
-        return self::rewritePath('misc/user/favicon.png');
+        return static::rewritePath('misc/user/favicon.png');
     }
 
     public static function getPathUserSvgLogo()
     {
-        return self::rewritePath('misc/user/logo.svg');
+        return static::rewritePath('misc/user/logo.svg');
     }
 
     public static function getPathUserLogoSmall()
     {
-        return self::rewritePath('misc/user/logo-header.png');
+        return static::rewritePath('misc/user/logo-header.png');
     }
 
     protected static function rewritePath($path)
@@ -163,6 +158,22 @@ class CustomLogo
         return SettingsPiwik::rewriteMiscUserPathWithInstanceId($path);
     }
 
+    /**
+     * @return bool
+     */
+    public static function hasUserLogo()
+    {
+        return static::logoExists(static::getPathUserLogo());
+    }
+
+    /**
+     * @return bool
+     */
+    public static function hasUserFavicon()
+    {
+        return static::logoExists(static::getPathUserFavicon());
+    }
+
     public function copyUploadedLogoToFilesystem()
     {
         $uploadFieldName = 'customLogo';
@@ -229,4 +240,12 @@ class CustomLogo
         return true;
     }
 
+    /**
+     * @return bool
+     */
+    private static function logoExists($relativePath)
+    {
+        return file_exists(Filesystem::getPathToPiwikRoot() . '/' . $relativePath);
+    }
+
 }
diff --git a/plugins/CoreAdminHome/javascripts/generalSettings.js b/plugins/CoreAdminHome/javascripts/generalSettings.js
index 5ac87afb57..7e594e705a 100644
--- a/plugins/CoreAdminHome/javascripts/generalSettings.js
+++ b/plugins/CoreAdminHome/javascripts/generalSettings.js
@@ -48,6 +48,10 @@ function isSmtpEnabled() {
     return $('input[name="mailUseSmtp"]:checked').val();
 }
 function showCustomLogoSettings(value) {
+    if (value == 1) {
+        // Refresh custom logo only if we're going to display it
+        refreshCustomLogo();
+    }
     $('#logoSettings').toggle(value == 1);
 }
 function isCustomLogoEnabled() {
@@ -59,8 +63,8 @@ function refreshCustomLogo() {
     var index;
     for (index = 0; index < selectors.length; index++) {
         var imageDiv = $(selectors[index]);
-        if (imageDiv && imageDiv.attr("src")) {
-            var logoUrl = imageDiv.attr("src").split("?")[0];
+        if (imageDiv && imageDiv.data("src") && imageDiv.data("srcExists")) {
+            var logoUrl = imageDiv.data("src");
             imageDiv.attr("src", logoUrl + "?" + (new Date()).getTime());
         }
     }
@@ -100,7 +104,6 @@ $(document).ready(function () {
         showSmtpSettings($(this).val());
     });
     $('input[name=useCustomLogo]').click(function () {
-        refreshCustomLogo();
         showCustomLogoSettings($(this).val());
     });
     $('input').keypress(function (e) {
@@ -113,20 +116,31 @@ $(document).ready(function () {
 
     $("#logoUploadForm").submit(function (data) {
         var submittingForm = $(this);
+        var isSubmittingLogo = ($('#customLogo').val() != '')
+        var isSubmittingFavicon = ($('#customFavicon').val() != '')
         $('.uploaderror').fadeOut();
         var frameName = "upload" + (new Date()).getTime();
         var uploadFrame = $("<iframe name=\"" + frameName + "\" />");
         uploadFrame.css("display", "none");
         uploadFrame.load(function (data) {
             setTimeout(function () {
-                refreshCustomLogo();
-
                 var frameContent = $(uploadFrame.contents()).find('body').html();
                 frameContent = $.trim(frameContent);
 
                 if ('0' === frameContent) {
                     $('.uploaderror').show();
                 }
+                else {
+                    // Upload succeed, so we update the images availability
+                    // according to what have been uploaded
+                    if (isSubmittingLogo) {
+                        $('#currentLogo').data("srcExists", true)
+                    }
+                    if (isSubmittingFavicon) {
+                        $('#currentFavicon').data("srcExists", true)
+                    }
+                    refreshCustomLogo();
+                }
 
                 if ('1' === frameContent || '0' === frameContent) {
                     uploadFrame.remove();
diff --git a/plugins/CoreAdminHome/templates/generalSettings.twig b/plugins/CoreAdminHome/templates/generalSettings.twig
index e70d537472..57373468ae 100644
--- a/plugins/CoreAdminHome/templates/generalSettings.twig
+++ b/plugins/CoreAdminHome/templates/generalSettings.twig
@@ -211,13 +211,13 @@
                         <label for="customLogo">{{ 'CoreAdminHome_LogoUpload'|translate }}</label>
                         <div class="form-help">{{ 'CoreAdminHome_LogoUploadHelp'|translate("JPG / PNG / GIF", 110) }}</div>
                         <input name="customLogo" type="file" id="customLogo"/>
-                        <img src="{{ pathUserLogo }}?r={{ random() }}" id="currentLogo" style="max-height: 150px"/>
+                        <img data-src="{{ pathUserLogo }}" data-src-exists="{{ hasUserLogo ? '1':'0' }}" id="currentLogo" style="max-height: 150px"/>
                     </div>
                     <div class="form-group">
                         <label for="customLogo">{{ 'CoreAdminHome_FaviconUpload'|translate }}</label>
                         <div class="form-help">{{ 'CoreAdminHome_LogoUploadHelp'|translate("JPG / PNG / GIF", 16) }}</div>
                         <input name="customFavicon" type="file" id="customFavicon"/>
-                        <img src="{{ pathUserFavicon }}?r={{ random() }}" id="currentFavicon" width="16" height="16"/>
+                        <img data-src="{{ pathUserFavicon }}" data-src-exists="{{ hasUserFavicon ? '1':'0' }}" id="currentFavicon" width="16" height="16"/>
                     </div>
                 {% else %}
                     <div class="alert alert-warning">