silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS
https://www.silverstripe.org
BSD 3-Clause "New" or "Revised" License
722 stars 821 forks source link

Add support for module-only resource identifiers for module loading #8523

Open webbower opened 5 years ago

webbower commented 5 years ago

Affected Version

SS 4.2.1

Description

I discovered this when trying to add a CSS file to load in the TinyMCE HTMLEditorField and discovered that it's a core code issue. Not sure if it's as designed or unintentional.

Given the following YAML config file:

---
Name: mytinymce
---
SilverStripe\Forms\HTMLEditor\TinyMCEConfig:
  editor_css:
    - 'mymodule:css/editor.css'

I'm trying to add a CSS file to the WYSIWYG editor. This file lives at <root>/mymodule/css/editor.css (I've broken up various areas of logic into submodules sibling to app). After running composer vendor-expose to expose the CSS file in resources and flushing the cache, when I load a ModelAdmin record editor view, the WYSIWYG field shows a red toast message "Failed to load content css: my module:css/editor.css."

I have discovered that this is a broader shortcoming. According to https://docs.silverstripe.org/en/4/changelogs/4.0.0/#module-paths, I should be able to use a module-only identifier (example from link ModuleLoader::getModule('mysite')->getResource('css/styles.css')->getRelativePath();). ModuleLoader::getModule() works with vendor + module and module-only prefixes. However, the loading of the editor_css values in TinyMCEConfig::getEditorCSS(), as well as css() and javascript() from the Requirements class goes through ModuleResourceLoader::resolveResource() in their codepaths, which has the following regex in the code path to match the resource: https://github.com/silverstripe/silverstripe-framework/blob/4/src/Core/Manifest/ModuleResourceLoader.php#L99

Manually running my CSS file reference against that regex does not successfully match (run on PHP CLI):

php > var_export(preg_match('#^ *(?<module>[^/: ]+/[^/: ]+) *: *(?<resource>[^ ]*)$#', "mymodule:css/editor.css", $matches));
0
php > print_r($matches);
Array
(
)

This seems like inconsistent behavior among module and resource loading, not to mention makes it difficult to use custom CSS and JS in custom module folders. A simple tweak to the regex enables it to match and load resources with module-only identifiers like the one I posted toward the top.

php > var_export(preg_match('#^ *(?<module>[^/: ]+(?:/[^/: ]+)?) *: *(?<resource>[^ ]*)$#', "mymodule:css/editor.css", $matches));
1
php > print_r($matches);
Array
(
    [0] => mymodule:css/editor.css
    [module] => mymodule
    [1] => mymodule
    [resource] => css/editor.css
    [2] => css/editor.css
)
robbieaverill commented 5 years ago

I feel like your example code should work, so marking as a bug

webbower commented 5 years ago

It does work. I've injected a subclass of ModuleResourceLoader that overrides that single method with the modified regex. Will post as a PR soon.