Merge pull request #1996 from open-sausages/pulls/4.0/fix-page-icons

BUG Fix page icons in vendor modules

code/Controllers/CMSMain.php

@@ -24,6 +24,7 @@ use SilverStripe\Control\HTTPResponse_Exception;
 use SilverStripe\Core\Convert;
 use SilverStripe\Core\Environment;
 use SilverStripe\Core\Injector\Injector;
+use SilverStripe\Core\Manifest\ModuleResource;
 use SilverStripe\Core\Manifest\ModuleResourceLoader;
 use SilverStripe\Forms\DateField;
 use SilverStripe\Forms\DropdownField;
@@ -1056,20 +1057,17 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
             }
 
             // skip this type if it is restricted
-            if ($instance->config()->get('need_permission') && !$this->can(singleton($class)->config()->get('need_permission'))) {
+            $needPermissions = $instance->config()->get('need_permission');
+            if ($needPermissions && !$this->can($needPermissions)) {
                 continue;
             }
 
-            $singularName = $instance->i18n_singular_name();
-            $description = $instance->i18n_classDescription();
-
             $result->push(new ArrayData(array(
                 'ClassName' => $class,
-                'AddAction' => $singularName,
-                'Description' => $description,
-                // TODO Sprite support
-                'IconURL' => ModuleResourceLoader::resourceURL($instance->config()->get('icon')),
-                'Title' => $singularName,
+                'AddAction' => $instance->i18n_singular_name(),
+                'Description' => $instance->i18n_classDescription(),
+                'IconURL' => $instance->getPageIconURL(),
+                'Title' => $instance->i18n_singular_name(),
             )));
         }
 

code/Controllers/LeftAndMainPageIconsExtension.php

@@ -3,11 +3,11 @@
 namespace SilverStripe\CMS\Controllers;
 
 use SilverStripe\CMS\Model\SiteTree;
-use SilverStripe\Core\Convert;
-use SilverStripe\View\Requirements;
 use SilverStripe\Core\ClassInfo;
-use SilverStripe\Control\Director;
+use SilverStripe\Core\Config\Config;
+use SilverStripe\Core\Convert;
 use SilverStripe\Core\Extension;
+use SilverStripe\View\Requirements;
 
 /**
  * Extension to include custom page icons
@@ -32,28 +32,16 @@ class LeftAndMainPageIconsExtension extends Extension
 
         $classes = ClassInfo::subclassesFor(SiteTree::class);
         foreach ($classes as $class) {
-            $obj = singleton($class);
-            $iconSpec = $obj->config()->get('icon');
-
-            if (!$iconSpec) {
+            $icon = Config::inst()->get($class, 'icon');
+            if (!$icon) {
                 continue;
             }
 
-            // Legacy support: We no longer need separate icon definitions for folders etc.
-            $iconFile = (is_array($iconSpec)) ? $iconSpec[0] : $iconSpec;
-
-            // Legacy support: Add file extension if none exists
-            if (!pathinfo($iconFile, PATHINFO_EXTENSION)) {
-                $iconFile .= '-file.gif';
-            }
-
-            $class = Convert::raw2htmlid($class);
-            $selector = ".page-icon.class-$class, li.class-$class > a .jstree-pageicon";
-            if (Director::fileExists($iconFile)) {
-                $css .= "$selector { background: transparent url('$iconFile') 0 0 no-repeat; }\n";
-            } else {
-                // Support for more sophisticated rules, e.g. sprited icons
-                $css .= "$selector { $iconFile }\n";
+            $cssClass = Convert::raw2htmlid($class);
+            $selector = ".page-icon.class-$cssClass, li.class-$cssClass > a .jstree-pageicon";
+            $iconURL = SiteTree::singleton($class)->getPageIconURL();
+            if ($iconURL) {
+                $css .= "$selector { background: transparent url('$iconURL') 0 0 no-repeat; }\n";
             }
         }
 

code/Model/SiteTree.php

@@ -17,6 +17,8 @@ use SilverStripe\Core\ClassInfo;
 use SilverStripe\Core\Config\Config;
 use SilverStripe\Core\Convert;
 use SilverStripe\Core\Injector\Injector;
+use SilverStripe\Core\Manifest\ModuleResource;
+use SilverStripe\Core\Manifest\ModuleResourceLoader;
 use SilverStripe\Core\Resettable;
 use SilverStripe\Dev\Deprecation;
 use SilverStripe\Forms\CheckboxField;
@@ -2841,6 +2843,33 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi
         return parent::plural_name();
     }
 
+    /**
+     * Generate link to this page's icon
+     *
+     * @return string
+     */
+    public function getPageIconURL()
+    {
+        $icon = $this->config()->get('icon');
+        if (!$icon) {
+            return null;
+        }
+
+        // Icon is relative resource
+        $iconResource = ModuleResourceLoader::singleton()->resolveResource($icon);
+        if ($iconResource instanceof ModuleResource) {
+            return $iconResource->getURL();
+        }
+
+        // Full path to file
+        if (Director::fileExists($icon)) {
+            return ModuleResourceLoader::resourceURL($icon);
+        }
+
+        // Skip invalid files
+        return null;
+    }
+
     /**
      * Get description for this page type
      *

tests/php/Controllers/LeftAndMainPageIconsExtensionTest.php

@@ -0,0 +1,30 @@
+<?php
+
+namespace SilverStripe\CMS\Tests\Controllers;
+
+use SilverStripe\CMS\Controllers\LeftAndMainPageIconsExtension;
+use SilverStripe\Dev\SapphireTest;
+
+class LeftAndMainPageIconsExtensionTest extends SapphireTest
+{
+    protected static $extra_dataobjects = [
+        LeftAndMainPageIconsExtensionTest\ModuleIconA::class,
+        LeftAndMainPageIconsExtensionTest\ModuleIconB::class,
+        LeftAndMainPageIconsExtensionTest\ModuleIconC::class,
+    ];
+
+    public function testGenerateIconCSS()
+    {
+        $extension = new LeftAndMainPageIconsExtension();
+        $css = $extension->generatePageIconsCss();
+        $this->assertNotContains('some invalid string', $css);
+        $this->assertContains(
+            'tests/php/Controllers/LeftAndMainPageIconsExtensionTest/icon_b.jpg?m=',
+            $css
+        );
+        $this->assertContains(
+            'tests/php/Controllers/LeftAndMainPageIconsExtensionTest/icon_c.jpg?m=',
+            $css
+        );
+    }
+}

tests/php/Controllers/LeftAndMainPageIconsExtensionTest/ModuleIconA.php

@@ -0,0 +1,11 @@
+<?php
+
+namespace SilverStripe\CMS\Tests\Controllers\LeftAndMainpageIconsExtensionTest;
+
+use SilverStripe\CMS\Model\SiteTree;
+use SilverStripe\Dev\TestOnly;
+
+class ModuleIconA extends SiteTree implements TestOnly
+{
+    private static $icon = 'some invalid string';
+}

tests/php/Controllers/LeftAndMainPageIconsExtensionTest/ModuleIconB.php

@@ -0,0 +1,11 @@
+<?php
+
+namespace SilverStripe\CMS\Tests\Controllers\LeftAndMainpageIconsExtensionTest;
+
+use SilverStripe\CMS\Model\SiteTree;
+use SilverStripe\Dev\TestOnly;
+
+class ModuleIconB extends SiteTree implements TestOnly
+{
+    private static $icon = 'silverstripe/cms:tests/php/Controllers/LeftAndMainPageIconsExtensionTest/icon_b.jpg';
+}

tests/php/Controllers/LeftAndMainPageIconsExtensionTest/ModuleIconC.php

@@ -0,0 +1,13 @@
+<?php
+
+namespace SilverStripe\CMS\Tests\Controllers\LeftAndMainpageIconsExtensionTest;
+
+use SilverStripe\CMS\Model\SiteTree;
+use SilverStripe\Dev\TestOnly;
+
+class ModuleIconC extends SiteTree implements TestOnly
+{
+    private static $extensions = [
+        ModuleIconExtension::class,
+    ];
+}

tests/php/Controllers/LeftAndMainPageIconsExtensionTest/ModuleIconExtension.php

@@ -0,0 +1,21 @@
+<?php
+
+namespace SilverStripe\CMS\Tests\Controllers\LeftAndMainpageIconsExtensionTest;
+
+use SilverStripe\CMS\Model\SiteTreeExtension;
+use SilverStripe\Core\Manifest\ModuleResourceLoader;
+use SilverStripe\Dev\TestOnly;
+
+class ModuleIconExtension extends SiteTreeExtension implements TestOnly
+{
+    public static function get_extra_config()
+    {
+        // Mock a "fixed" path, but use a non-fixed resource url
+        $path = ModuleResourceLoader::resourcePath(
+            'silverstripe/cms:tests/php/Controllers/LeftAndMainPageIconsExtensionTest/icon_c.jpg'
+        );
+        return [
+            'icon' => $path,
+        ];
+    }
+}