BUG Fix resource mapping for TinyMCE

API add ModuleResource::getRelativeResource()

.travis.yml

@@ -68,14 +68,11 @@ before_script:
   - composer validate
   - if [[ $DB == PGSQL ]]; then composer require silverstripe/postgresql:2.0.x-dev --no-update; fi
   - if [[ $DB == SQLITE ]]; then composer require silverstripe/sqlite3:2.0.x-dev --no-update; fi
-  - composer require silverstripe/config:1.0.x-dev silverstripe/admin:1.0.x-dev silverstripe/assets:1.0.x-dev silverstripe/versioned:1.0.x-dev --no-update
+  - composer require silverstripe/recipe-core:1.0.x-dev silverstripe/admin:1.0.x-dev silverstripe/versioned:1.0.x-dev --no-update
-  - if [[ $PHPUNIT_TEST == cms ]] || [[ $BEHAT_TEST == cms ]]; then composer require silverstripe/cms:4.0.x-dev silverstripe/campaign-admin:1.0.x-dev silverstripe/siteconfig:4.0.x-dev silverstripe/reports:4.0.x-dev --no-update; fi
+  - if [[ $PHPUNIT_TEST == cms ]] || [[ $BEHAT_TEST == cms ]]; then composer require silverstripe/recipe-cms:1.0.x-dev --no-update; fi
   - if [[ $PHPCS_TEST ]]; then composer global require squizlabs/php_codesniffer:^3 --prefer-dist --no-interaction --no-progress --no-suggest -o; fi
   - composer install --prefer-dist --no-interaction --no-progress --no-suggest --optimize-autoloader --verbose --profile
 
-# Bootstrap dependencies
-  - if [[ $PHPUNIT_TEST == cms ]] || [[ $BEHAT_TEST == cms ]]; then php ./cms/tests/bootstrap/mysite.php; fi
-
 # Log constants to CI for debugging purposes
   - php ./tests/dump_constants.php
 

src/Core/Manifest/Module.php

@@ -188,7 +188,7 @@ class Module implements Serializable
      */
     public function getResource($path)
     {
-        $path = trim($path, '/\\');
+        $path = trim($path, self::TRIM_CHARS);
         if (isset($this->resources[$path])) {
             return $this->resources[$path];
         }

src/Core/Manifest/ModuleResource.php

@@ -23,6 +23,13 @@ class ModuleResource
      */
     protected $relativePath = null;
 
+    /**
+     * Nested resources for this parent resource
+     *
+     * @var ModuleResource[]
+     */
+    protected $resources = [];
+
     /**
      * ModuleResource constructor.
      *
@@ -78,7 +85,7 @@ class ModuleResource
     {
         /** @var ResourceURLGenerator $generator */
         $generator = Injector::inst()->get(ResourceURLGenerator::class);
-        return $generator->urlForResource($this->getRelativePath());
+        return $generator->urlForResource($this);
     }
 
     /**
@@ -118,4 +125,25 @@ class ModuleResource
     {
         return $this->module;
     }
+
+    /**
+     * Get nested resource relative to this.
+     * Note: Doesn't support `..` or `.` relative syntax
+     *
+     * @param string $path
+     * @return ModuleResource
+     */
+    public function getRelativeResource($path)
+    {
+        // Check cache
+        $path = trim($path, Module::TRIM_CHARS);
+        if (isset($this->resources[$path])) {
+            return $this->resources[$path];
+        }
+
+        // Build new relative path
+        $relativeBase = rtrim($this->relativePath, Module::TRIM_CHARS);
+        $relativePath = "{$relativeBase}/{$path}";
+        return $this->resources[$path] = new ModuleResource($this->getModule(), $relativePath);
+    }
 }

src/Core/Manifest/ModuleResourceLoader.php

@@ -22,8 +22,8 @@ class ModuleResourceLoader implements TemplateGlobalProvider
      */
     public function resolvePath($resource)
     {
-        $resourceObj = $this->resolveModuleResource($resource);
+        $resourceObj = $this->resolveResource($resource);
-        if ($resourceObj) {
+        if ($resourceObj instanceof ModuleResource) {
             return $resourceObj->getRelativePath();
         }
         return $resource;
@@ -37,8 +37,8 @@ class ModuleResourceLoader implements TemplateGlobalProvider
      */
     public function resolveURL($resource)
     {
-        $resourceObj = $this->resolveModuleResource($resource);
+        $resourceObj = $this->resolveResource($resource);
-        if ($resourceObj) {
+        if ($resourceObj instanceof ModuleResource) {
             return $resourceObj->getURL();
         }
         return $resource;
@@ -76,16 +76,16 @@ class ModuleResourceLoader implements TemplateGlobalProvider
 
     /**
      * Return module resource for the given path, if specified as one.
-     * Returns null if not a module resource.
+     * Returns the original resource otherwise.
      *
      * @param string $resource
-     * @return ModuleResource|null
+     * @return ModuleResource|string The resource, or input string if not a module resource
      */
-    protected function resolveModuleResource($resource)
+    public function resolveResource($resource)
     {
         // String of the form vendor/package:resource. Excludes "http://bla" as that's an absolute URL
         if (!preg_match('#^ *(?<module>[^/: ]+/[^/: ]+) *: *(?<resource>[^ ]*)$#', $resource, $matches)) {
-            return null;
+            return $resource;
         }
         $module = $matches['module'];
         $resource = $matches['resource'];

src/Forms/HTMLEditor/TinyMCECombinedGenerator.php

@@ -77,72 +77,54 @@ class TinyMCECombinedGenerator implements TinyMCEScriptGenerator, Flushable
      */
     public function generateContent(TinyMCEConfig $config)
     {
-        $tinymceDir = $config->getTinyMCEPath();
+        $tinymceDir = $config->getTinyMCEResource();
 
-        $files = [ ];
+        // List of string / ModuleResource references to embed
+        $files = [];
 
         // Add core languages
         $language = $config->getOption('language');
         if ($language) {
-            $files[] = $tinymceDir . '/langs/' . $language;
+            $files[] = $this->resolveRelativeResource($tinymceDir, "langs/{$language}");
         }
 
         // Add plugins, along with any plugin specific lang files
         foreach ($config->getPlugins() as $plugin => $path) {
             // Add external plugin
             if ($path) {
-                if ($path instanceof ModuleResource) {
+                // Convert URLS to relative paths
-                    // Resolve path / url later
+                if (is_string($path)
-                    $files[] = $path;
+                    && (Director::is_absolute_url($path) || strpos($path, '/') === 0)
-                } elseif (Director::is_absolute_url($path) || strpos($path, '/') === 0) {
+                ) {
-                    // Convert URLS to relative paths
                     $path = Director::makeRelative($path);
-                    if ($path) {
+                }
-                        $files[] = $path;
+                // Ensure file exists
-                    }
+                if ($this->resourceExists($path)) {
-                } else {
-                    // Relative URLs are safe
                     $files[] = $path;
                 }
                 continue;
             }
 
             // Core tinymce plugin
-            $files[] = $tinymceDir . '/plugins/' . $plugin . '/plugin';
+            $files[] = $this->resolveRelativeResource($tinymceDir, "plugins/{$plugin}/plugin");
             if ($language) {
-                $files[] = $tinymceDir . '/plugins/' . $plugin . '/langs/' . $language;
+                $files[] = $this->resolveRelativeResource($tinymceDir, "plugins/{$plugin}/langs/{$language}");
             }
         }
 
         // Add themes
         $theme = $config->getTheme();
         if ($theme) {
-            $files[] = $tinymceDir . '/themes/' . $theme . '/theme';
+            $files[] = $this->resolveRelativeResource($tinymceDir, "themes/{$theme}/theme");
             if ($language) {
-                $files[] = $tinymceDir . '/themes/' . $theme . '/langs/' . $language;
+                $files[] = $this->resolveRelativeResource($tinymceDir, "themes/{$theme}/langs/{$language}");
             }
         }
 
         // Process source files
-        $files = array_filter(array_map(function ($file) {
+        $files = array_filter($files);
-            if ($file instanceof ModuleResource) {
+        $libResource = $this->resolveRelativeResource($tinymceDir, 'tinymce');
-                return $file;
+        $libContent = $this->getFileContents($libResource);
-            }
-            // Prioritise preferred paths
-            $paths = [
-                $file,
-                $file . '.min.js',
-                $file . '.js',
-            ];
-            foreach ($paths as $path) {
-                if (file_exists(Director::baseFolder() . '/' . $path)) {
-                    return $path;
-                }
-            }
-            return null;
-        }, $files));
-
-        $libContent = $this->getFileContents(Director::baseFolder() . '/' . $tinymceDir . '/tinymce.min.js');
 
         // Register vars for config
         $baseDirJS = Convert::raw2js(Director::absoluteBaseURL());
@@ -169,18 +151,14 @@ SCRIPT;
 
         // Load all tinymce script files into buffer
         foreach ($files as $path) {
-            if ($path instanceof ModuleResource) {
-                $path = $path->getPath();
-            } else {
-                $path = Director::baseFolder() . '/' . $path;
-            }
             $buffer[] = $this->getFileContents($path);
         }
 
         // Generate urls for all files
+        // Note all urls must be relative to base_dir
         $fileURLS = array_map(function ($path) {
             if ($path instanceof ModuleResource) {
-                return $path->getURL();
+                return Director::makeRelative($path->getURL());
             }
             return $path;
         }, $files);
@@ -188,7 +166,7 @@ SCRIPT;
         // Join list of paths
         $filesList = Convert::raw2js(implode(',', $fileURLS));
         // Mark all themes, plugins and languages as done
-        $buffer[] = "window.tinymce.each('$filesList'.split(','),".
+        $buffer[] = "window.tinymce.each('$filesList'.split(',')," .
             "function(f){tinymce.ScriptLoader.markDone(baseURL+f);});";
 
         $buffer[] = '})();';
@@ -198,12 +176,20 @@ SCRIPT;
     /**
      * Returns the contents of the script file if it exists and removes the UTF-8 BOM header if it exists.
      *
-     * @param string $file File to load.
+     * @param string|ModuleResource $file File to load.
      * @return string File contents or empty string if it doesn't exist.
      */
     protected function getFileContents($file)
     {
-        $content = file_get_contents($file);
+        if ($file instanceof ModuleResource) {
+            $path = $file->getPath();
+        } else {
+            $path = Director::baseFolder() . '/' . $file;
+        }
+        if (!file_exists($path)) {
+            return null;
+        }
+        $content = file_get_contents($path);
 
         // Remove UTF-8 BOM
         if (substr($content, 0, 3) === pack("CCC", 0xef, 0xbb, 0xbf)) {
@@ -260,4 +246,47 @@ SCRIPT;
         $dir = dirname(static::config()->get('filename_base'));
         static::singleton()->getAssetHandler()->removeContent($dir);
     }
+
+    /**
+     * Get relative resource for a given base and string
+     *
+     * @param ModuleResource|string $base
+     * @param string $resource
+     * @return ModuleResource|string
+     */
+    protected function resolveRelativeResource($base, $resource)
+    {
+        // Return resource path based on relative resource path
+        foreach (['', '.min.js', '.js'] as $ext) {
+            // Map resource
+            if ($base instanceof ModuleResource) {
+                $next = $base->getRelativeResource($resource . $ext);
+            } else {
+                $next = rtrim($base, '/') . '/' . $resource . $ext;
+            }
+            // Ensure resource exists
+            if ($this->resourceExists($next)) {
+                return $next;
+            }
+        }
+        return null;
+    }
+
+    /**
+     * Check if the given resource exists
+     *
+     * @param string|ModuleResource $resource
+     * @return bool
+     */
+    protected function resourceExists($resource)
+    {
+        if (!$resource) {
+            return false;
+        }
+        if ($resource instanceof ModuleResource) {
+            return $resource->exists();
+        }
+        $base = rtrim(Director::baseFolder(), '/');
+        return file_exists($base . '/' . $resource);
+    }
 }

src/Forms/HTMLEditor/TinyMCEConfig.php

@@ -9,6 +9,7 @@ use SilverStripe\Core\Convert;
 use SilverStripe\Core\Injector\Injector;
 use SilverStripe\Core\Manifest\Module;
 use SilverStripe\Core\Manifest\ModuleLoader;
+use SilverStripe\Core\Manifest\ModuleResource;
 use SilverStripe\Core\Manifest\ModuleResourceLoader;
 use SilverStripe\Dev\Deprecation;
 use SilverStripe\i18n\i18n;
@@ -282,20 +283,20 @@ class TinyMCEConfig extends HTMLEditorConfig
      *
      * @var array
      */
-    protected $buttons = array(
+    protected $buttons = [
-        1 => array(
+        1 => [
             'bold', 'italic', 'underline', 'removeformat', '|',
             'alignleft', 'aligncenter', 'alignright', 'alignjustify', '|',
             'bullist', 'numlist', 'outdent', 'indent',
-        ),
+        ],
-        2 => array(
+        2 => [
             'formatselect', '|',
             'paste', 'pastetext', '|',
             'table', 'sslink', 'unlink', '|',
             'code'
-        ),
+        ],
-        3 => array()
+        3 => []
-    );
+    ];
 
     public function getOption($key)
     {
@@ -570,13 +571,20 @@ class TinyMCEConfig extends HTMLEditorConfig
         $settings['document_base_url'] = Director::absoluteBaseURL();
 
         // https://www.tinymce.com/docs/api/class/tinymce.editormanager/#baseURL
-        $tinyMCEBaseURL = Controller::join_links(Director::baseURL(), $this->getTinyMCEResourcePath());
+        $baseResource = $this->getTinyMCEResource();
+        if ($baseResource instanceof ModuleResource) {
+            $tinyMCEBaseURL = $baseResource->getURL();
+        } else {
+            $tinyMCEBaseURL = Controller::join_links(Director::baseURL(), $baseResource);
+        }
         $settings['baseURL'] = $tinyMCEBaseURL;
 
         // map all plugins to absolute urls for loading
         $plugins = array();
         foreach ($this->getPlugins() as $plugin => $path) {
-            if (!$path) {
+            if ($path instanceof ModuleResource) {
+                $path = Director::absoluteURL($path->getURL());
+            } elseif (!$path) {
                 // Empty paths: Convert to urls in standard base url
                 $path = Controller::join_links(
                     $tinyMCEBaseURL,
@@ -687,18 +695,50 @@ class TinyMCEConfig extends HTMLEditorConfig
     }
 
     /**
-     * Returns the base path to TinyMCE resources (which could be different from the original tinymce
+     * Returns the full filesystem path to TinyMCE resources (which could be different from the original tinymce
      * location in the module).
      *
+     * Path will be absolute.
+     *
      * @return string
      * @throws Exception
      */
     public function getTinyMCEResourcePath()
+    {
+        $resource = $this->getTinyMCEResource();
+        if ($resource instanceof ModuleResource) {
+            return $resource->getPath();
+        }
+        return Director::baseFolder() . '/' . $resource;
+    }
+
+    /**
+     * Get front-end url to tinymce resources
+     *
+     * @return string
+     * @throws Exception
+     */
+    public function getTinyMCEResourceURL()
+    {
+        $resource = $this->getTinyMCEResource();
+        if ($resource instanceof ModuleResource) {
+            return $resource->getURL();
+        }
+        return $resource;
+    }
+
+    /**
+     * Get resource root for TinyMCE, either as a string or ModuleResource instance
+     * Path will be relative to BASE_PATH if string.
+     *
+     * @return ModuleResource|string
+     * @throws Exception
+     */
+    public function getTinyMCEResource()
     {
         $configDir = static::config()->get('base_dir');
         if ($configDir) {
-            return ModuleResourceLoader::singleton()
+            return ModuleResourceLoader::singleton()->resolveResource($configDir);
-                ->resolveURL($configDir);
         }
 
         throw new Exception(sprintf(
@@ -708,21 +748,12 @@ class TinyMCEConfig extends HTMLEditorConfig
     }
 
     /**
-     * @return string
+     * @deprecated 4.0..5.0
-     * @throws Exception
      */
     public function getTinyMCEPath()
     {
-        $configDir = static::config()->get('base_dir');
+        Deprecation::notice('5.0', 'use getTinyMCEResourcePath instead');
-        if ($configDir) {
+        return $this->getTinyMCEResourcePath();
-            return ModuleResourceLoader::singleton()
-                ->resolveURL($configDir);
-        }
-
-        throw new Exception(sprintf(
-            'If the silverstripe/admin module is not installed you must set the TinyMCE path in %s.base_dir',
-            __CLASS__
-        ));
     }
 
     /**

src/Forms/HTMLEditor/TinyMCEGZIPGenerator.php

@@ -3,6 +3,7 @@
 namespace SilverStripe\Forms\HTMLEditor;
 
 use Exception;
+use SilverStripe\Control\Controller;
 use SilverStripe\Core\Injector\Injectable;
 use SilverStripe\Dev\Deprecation;
 use TinyMCE_Compressor;
@@ -33,12 +34,12 @@ class TinyMCEGZIPGenerator implements TinyMCEScriptGenerator
         // If gzip is disabled just return core script url
         $useGzip = HTMLEditorField::config()->get('use_gzip');
         if (!$useGzip) {
-            return $config->getTinyMCEResourcePath() . '/tinymce.min.js';
+            return Controller::join_links($config->getTinyMCEResourceURL(), 'tinymce.min.js');
         }
 
         // tinyMCE JS requirement - use the original module path,
         // don't assume the PHP file is copied alongside the resources
-        $gzipPath = BASE_PATH . '/' . $config->getTinyMCEPath() . '/tiny_mce_gzip.php';
+        $gzipPath = $config->getTinyMCEResourcePath() . '/tiny_mce_gzip.php';
         if (!file_exists($gzipPath)) {
             throw new Exception("HTMLEditorField.use_gzip enabled, but file $gzipPath does not exist!");
         }
@@ -46,7 +47,7 @@ class TinyMCEGZIPGenerator implements TinyMCEScriptGenerator
         require_once $gzipPath;
 
         $tag = TinyMCE_Compressor::renderTag(array(
-            'url' => $config->getTinyMCEPath() . '/tiny_mce_gzip.php',
+            'url' => $config->getTinyMCEResourceURL() . '/tiny_mce_gzip.php',
             'plugins' => implode(',', $config->getInternalPlugins()),
             'themes' => $config->getTheme(),
             'languages' => $config->getOption('language')

tests/php/Core/Manifest/ModuleResourceTest.php

@@ -0,0 +1,87 @@
+<?php
+
+namespace SilverStripe\Core\Tests\Manifest;
+
+use SilverStripe\Control\Director;
+use SilverStripe\Core\Manifest\ModuleLoader;
+use SilverStripe\Core\Manifest\ModuleManifest;
+use SilverStripe\Dev\SapphireTest;
+
+class ModuleResourceTest extends SapphireTest
+{
+    /**
+     * @var string
+     */
+    protected $base;
+
+    /**
+     * @var ModuleManifest
+     */
+    protected $manifest;
+
+    protected function setUp()
+    {
+        parent::setUp();
+
+        $this->base = dirname(__FILE__) . '/fixtures/classmanifest';
+        $this->manifest = new ModuleManifest($this->base);
+        $this->manifest->init();
+        Director::config()->set('alternate_base_url', 'http://www.mysite.com/basefolder/');
+    }
+
+    public function testBaseModuleResource()
+    {
+        $modulea = $this->manifest->getModule('module');
+        $resource = $modulea->getResource('client/script.js');
+
+        // Test main resource
+        $this->assertTrue($resource->exists());
+        $this->assertEquals('module/client/script.js', $resource->getRelativePath());
+        $this->assertEquals(
+            __DIR__ . '/fixtures/classmanifest/module/client/script.js',
+            $resource->getPath()
+        );
+        $this->assertStringStartsWith(
+            '/basefolder/module/client/script.js?m=',
+            $resource->getURL()
+        );
+    }
+
+    public function testVendorModuleResources()
+    {
+        $modulec = $this->manifest->getModule('silverstripe/modulec');
+        $resource = $modulec->getResource('client/script.js');
+
+        // Test main resource
+        $this->assertTrue($resource->exists());
+        $this->assertEquals('vendor/silverstripe/modulec/client/script.js', $resource->getRelativePath());
+        $this->assertEquals(
+            __DIR__ . '/fixtures/classmanifest/vendor/silverstripe/modulec/client/script.js',
+            $resource->getPath()
+        );
+        $this->assertStringStartsWith(
+            '/basefolder/resources/silverstripe/modulec/client/script.js?m=',
+            $resource->getURL()
+        );
+    }
+
+    public function testRelativeResources()
+    {
+        $modulec = $this->manifest->getModule('silverstripe/modulec');
+        $resource = $modulec
+            ->getResource('client')
+            ->getRelativeResource('script.js');
+
+        // Test main resource
+        $this->assertTrue($resource->exists());
+        $this->assertEquals('vendor/silverstripe/modulec/client/script.js', $resource->getRelativePath());
+        $this->assertEquals(
+            __DIR__ . '/fixtures/classmanifest/vendor/silverstripe/modulec/client/script.js',
+            $resource->getPath()
+        );
+        $this->assertStringStartsWith(
+            '/basefolder/resources/silverstripe/modulec/client/script.js?m=',
+            $resource->getURL()
+        );
+    }
+}

tests/php/Core/Manifest/fixtures/classmanifest/module/client/script.js

@@ -0,0 +1 @@
+/* script.js */

tests/php/Core/Manifest/fixtures/classmanifest/vendor/silverstripe/modulec/client/script.js

@@ -0,0 +1 @@
+/* script.js */

tests/php/Forms/HTMLEditor/HTMLEditorConfigTest.php

@@ -3,7 +3,6 @@
 namespace SilverStripe\Forms\Tests\HTMLEditor;
 
 use Exception;
-use PHPUnit_Framework_MockObject_MockObject;
 use SilverStripe\Control\Director;
 use SilverStripe\Control\SimpleResourceURLGenerator;
 use SilverStripe\Core\Config\Config;
@@ -14,7 +13,6 @@ use SilverStripe\Core\Manifest\ModuleManifest;
 use SilverStripe\Core\Manifest\ResourceURLGenerator;
 use SilverStripe\Dev\SapphireTest;
 use SilverStripe\Forms\HTMLEditor\HTMLEditorConfig;
-use SilverStripe\Forms\HTMLEditor\HTMLEditorField;
 use SilverStripe\Forms\HTMLEditor\TinyMCEConfig;
 
 class HTMLEditorConfigTest extends SapphireTest

tests/php/Forms/HTMLEditor/TinyMCECombinedGeneratorTest.php

@@ -89,7 +89,7 @@ class TinyMCECombinedGeneratorTest extends SapphireTest
         // Check plugin links included
         $this->assertContains(
             <<<EOS
-tinymce.each('tinymce/langs/en.js,mycode/plugin1.js,tinymce/plugins/plugin4/plugin.min.js,tinymce/plugins/plugin4/langs/en.js,tinymce/plugins/plugin5/plugin.js,mycode/plugin6.js,/basedir/mycode/plugin8.js?m=
+tinymce.each('tinymce/langs/en.js,mycode/plugin1.js,tinymce/plugins/plugin4/plugin.min.js,tinymce/plugins/plugin4/langs/en.js,tinymce/plugins/plugin5/plugin.js,mycode/plugin6.js,mycode/plugin8.js?m=
 EOS
             ,
             $content