From 078a508d718f9a37dc6ac413be2db5ecd124a03b Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Mon, 31 Jul 2017 18:04:20 +1200 Subject: [PATCH 1/2] API Replace legacy tiny_mce_gzip compressor with asset generator Fixes https://github.com/silverstripe/silverstripe-admin/issues/74 --- _config/html.yml | 5 + .../HTMLEditor/TinyMCECombinedGenerator.php | 204 ++++++++++++++++++ src/Forms/HTMLEditor/TinyMCEConfig.php | 39 +--- src/Forms/HTMLEditor/TinyMCEGZIPGenerator.php | 57 +++++ .../HTMLEditor/TinyMCEScriptGenerator.php | 17 ++ .../Forms/HTMLEditor/HTMLEditorConfigTest.php | 61 +----- .../TinyMCECombinedGeneratorTest.php | 84 ++++++++ .../mycode/plugin1.js | 1 + .../mycode/plugin2.js | 1 + .../mycode/plugin3.js | 1 + .../mycode/plugin6.js | 1 + .../tinymce/langs/en.js | 1 + .../tinymce/plugins/plugin4/langs/en.js | 1 + .../tinymce/plugins/plugin4/plugin.js | 1 + .../tinymce/plugins/plugin4/plugin.min.js | 1 + .../tinymce/plugins/plugin5/plugin.js | 1 + .../tinymce/themes/testtheme/langs/en.js | 1 + .../tinymce/themes/testtheme/theme.js | 1 + .../tinymce/tinymce.js | 1 + 19 files changed, 391 insertions(+), 88 deletions(-) create mode 100644 src/Forms/HTMLEditor/TinyMCECombinedGenerator.php create mode 100644 src/Forms/HTMLEditor/TinyMCEGZIPGenerator.php create mode 100644 src/Forms/HTMLEditor/TinyMCEScriptGenerator.php create mode 100644 tests/php/Forms/HTMLEditor/TinyMCECombinedGeneratorTest.php create mode 100644 tests/php/Forms/HTMLEditor/TinyMCECombinedGeneratorTest/mycode/plugin1.js create mode 100644 tests/php/Forms/HTMLEditor/TinyMCECombinedGeneratorTest/mycode/plugin2.js create mode 100644 tests/php/Forms/HTMLEditor/TinyMCECombinedGeneratorTest/mycode/plugin3.js create mode 100644 tests/php/Forms/HTMLEditor/TinyMCECombinedGeneratorTest/mycode/plugin6.js create mode 100644 tests/php/Forms/HTMLEditor/TinyMCECombinedGeneratorTest/tinymce/langs/en.js create mode 100644 tests/php/Forms/HTMLEditor/TinyMCECombinedGeneratorTest/tinymce/plugins/plugin4/langs/en.js create mode 100644 tests/php/Forms/HTMLEditor/TinyMCECombinedGeneratorTest/tinymce/plugins/plugin4/plugin.js create mode 100644 tests/php/Forms/HTMLEditor/TinyMCECombinedGeneratorTest/tinymce/plugins/plugin4/plugin.min.js create mode 100644 tests/php/Forms/HTMLEditor/TinyMCECombinedGeneratorTest/tinymce/plugins/plugin5/plugin.js create mode 100644 tests/php/Forms/HTMLEditor/TinyMCECombinedGeneratorTest/tinymce/themes/testtheme/langs/en.js create mode 100644 tests/php/Forms/HTMLEditor/TinyMCECombinedGeneratorTest/tinymce/themes/testtheme/theme.js create mode 100644 tests/php/Forms/HTMLEditor/TinyMCECombinedGeneratorTest/tinymce/tinymce.js diff --git a/_config/html.yml b/_config/html.yml index 0c3d04c2f..20479728c 100644 --- a/_config/html.yml +++ b/_config/html.yml @@ -8,3 +8,8 @@ SilverStripe\Core\Injector\Injector: HTMLValue: '%$SilverStripe\View\Parsers\HTMLValue' SilverStripe\Forms\HTMLEditor\HTMLEditorConfig: class: SilverStripe\Forms\HTMLEditor\TinyMCEConfig + SilverStripe\Forms\HTMLEditor\TinyMCEScriptGenerator: '%$SilverStripe\Forms\HTMLEditor\TinyMCECombinedGenerator' + SilverStripe\Forms\HTMLEditor\TinyMCECombinedGenerator: + class: SilverStripe\Forms\HTMLEditor\TinyMCECombinedGenerator + properties: + AssetHandler: '%$SilverStripe\Assets\Storage\GeneratedAssetHandler' diff --git a/src/Forms/HTMLEditor/TinyMCECombinedGenerator.php b/src/Forms/HTMLEditor/TinyMCECombinedGenerator.php new file mode 100644 index 000000000..ace718319 --- /dev/null +++ b/src/Forms/HTMLEditor/TinyMCECombinedGenerator.php @@ -0,0 +1,204 @@ +assetHandler = $assetHandler; + return $this; + } + + /** + * Get backend for assets + * @return GeneratedAssetHandler + */ + public function getAssetHandler() + { + return $this->assetHandler; + } + + /** + * Generate a script URL for the given config + * + * @param TinyMCEConfig $config + * @return string + */ + public function getScriptURL(TinyMCEConfig $config) + { + // Build URL for this config based on named config and hash of settings + $url = $this->generateFilename($config); + + // Pass content generator + return $this->getAssetHandler()->getContentURL($url, function () use ($config) { + return $this->generateContent($config); + }); + } + + /** + * Build raw config for tinymce + * + * @param TinyMCEConfig $config + * @return string + */ + public function generateContent(TinyMCEConfig $config) + { + $tinymceDir = $config->getTinyMCEPath(); + + // Core JS file + $files = [ $tinymceDir . '/tinymce' ]; + + // Add core languages + $language = $config->getOption('language'); + if ($language) { + $files[] = $tinymceDir . '/langs/' . $language; + } + + // Add plugins, along with any plugin specific lang files + foreach ($config->getPlugins() as $plugin => $path) { + // Add external plugin + if ($path) { + // Convert URLS to relative paths + if (Director::is_absolute_url($path) || strpos($path, '/') === 0) { + // De-absolute site urls + $path = Director::makeRelative($path); + if ($path) { + $files[] = $path; + } + } else { + // Relative URLs are safe + $files[] = $path; + } + continue; + } + + // Core tinymce plugin + $files[] = $tinymceDir . '/plugins/' . $plugin . '/plugin'; + if ($language) { + $files[] = $tinymceDir . '/plugins/' . $plugin . '/langs/' . $language; + } + } + + // Add themes + $theme = $config->getTheme(); + if ($theme) { + $files[] = $tinymceDir . '/themes/' . $theme . '/theme'; + if ($language) { + $files[] = $tinymceDir . '/themes/' . $theme . '/langs/' . $language; + } + } + + // Process source files + $files = array_filter(array_map(function ($file) { + // 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)); + + // Set base URL for where tinymce is loaded from + $buffer = "var tinyMCEPreInit={base:'" . Convert::raw2js($tinymceDir) . "',suffix:'.min'};\n"; + + // Load all tinymce script files into buffer + foreach ($files as $file) { + $buffer .= $this->getFileContents(Director::baseFolder() . '/' . $file) . "\n"; + } + + // Mark all themes, plugins and languages as done + $buffer .= 'tinymce.each("' . Convert::raw2js(implode(',', $files)) . '".split(","),function(f){tinymce.ScriptLoader.markDone(f);});'; + + return $buffer . "\n"; + } + + + /** + * 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. + * @return string File contents or empty string if it doesn't exist. + */ + protected function getFileContents($file) + { + $content = file_get_contents($file); + + // Remove UTF-8 BOM + if (substr($content, 0, 3) === pack("CCC", 0xef, 0xbb, 0xbf)) { + $content = substr($content, 3); + } + + return $content; + } + + /** + * Check if this config is registered under a given key + * + * @param TinyMCEConfig $config + * @return string + */ + protected function checkName(TinyMCEConfig $config) + { + $configs = HTMLEditorConfig::get_available_configs_map(); + foreach ($configs as $id => $name) { + if (HTMLEditorConfig::get($id) === $config) { + return $id; + } + } + return 'custom'; + } + + /** + * Get filename to use for this config + * + * @param TinyMCEConfig $config + * @return mixed + */ + public function generateFilename(TinyMCEConfig $config) + { + $hash = substr(sha1(json_encode($config->getAttributes())), 0, 10); + $name = $this->checkName($config); + $url = str_replace( + ['{name}', '{hash}'], + [$name, $hash], + $this->config()->get('filename_base') + ); + return $url; + } +} diff --git a/src/Forms/HTMLEditor/TinyMCEConfig.php b/src/Forms/HTMLEditor/TinyMCEConfig.php index e394effea..b11789e25 100644 --- a/src/Forms/HTMLEditor/TinyMCEConfig.php +++ b/src/Forms/HTMLEditor/TinyMCEConfig.php @@ -2,9 +2,11 @@ namespace SilverStripe\Forms\HTMLEditor; -use SilverStripe\Core\Convert; +use Exception; use SilverStripe\Control\Controller; use SilverStripe\Control\Director; +use SilverStripe\Core\Convert; +use SilverStripe\Core\Injector\Injector; use SilverStripe\Core\Manifest\Module; use SilverStripe\Core\Manifest\ModuleLoader; use SilverStripe\Dev\Deprecation; @@ -12,8 +14,6 @@ use SilverStripe\i18n\i18n; use SilverStripe\View\Requirements; use SilverStripe\View\SSViewer; use SilverStripe\View\ThemeResourceLoader; -use TinyMCE_Compressor; -use Exception; /** * Default configuration for HtmlEditor specific to tinymce @@ -627,8 +627,11 @@ class TinyMCEConfig extends HTMLEditorConfig $editor = array(); // Add standard editor.css - foreach ($this->config()->get('editor_css') as $editorCSS) { - $editor[] = Director::absoluteURL($this->resolvePath($editorCSS)); + $editorCSSFiles = $this->config()->get('editor_css'); + if ($editorCSSFiles) { + foreach ($editorCSSFiles as $editorCSS) { + $editor[] = Director::absoluteURL($this->resolvePath($editorCSS)); + } } // Themed editor.css @@ -651,29 +654,9 @@ class TinyMCEConfig extends HTMLEditorConfig */ public function getScriptURL() { - // If gzip is disabled just return core script url - $useGzip = HTMLEditorField::config()->get('use_gzip'); - if (!$useGzip) { - return $this->getTinyMCEPath() . '/tinymce.min.js'; - } - - // tinyMCE JS requirement - $gzipPath = BASE_PATH . '/' . $this->getTinyMCEPath() . '/tiny_mce_gzip.php'; - if (!file_exists($gzipPath)) { - throw new Exception("HTMLEditorField.use_gzip enabled, but file $gzipPath does not exist!"); - } - - require_once $gzipPath; - - $tag = TinyMCE_Compressor::renderTag(array( - 'url' => $this->getTinyMCEPath() . '/tiny_mce_gzip.php', - 'plugins' => implode(',', $this->getInternalPlugins()), - 'themes' => $this->getTheme(), - 'languages' => $this->getOption('language') - ), true); - preg_match('/src="([^"]*)"/', $tag, $matches); - - return html_entity_decode($matches[1]); + /** @var TinyMCEScriptGenerator $generator */ + $generator = Injector::inst()->get(TinyMCEScriptGenerator::class); + return $generator->getScriptURL($this); } public function init() diff --git a/src/Forms/HTMLEditor/TinyMCEGZIPGenerator.php b/src/Forms/HTMLEditor/TinyMCEGZIPGenerator.php new file mode 100644 index 000000000..b575b5ae6 --- /dev/null +++ b/src/Forms/HTMLEditor/TinyMCEGZIPGenerator.php @@ -0,0 +1,57 @@ +get('use_gzip'); + if (!$useGzip) { + return $config->getTinyMCEPath() . '/tinymce.min.js'; + } + + // tinyMCE JS requirement + $gzipPath = BASE_PATH . '/' . $config->getTinyMCEPath() . '/tiny_mce_gzip.php'; + if (!file_exists($gzipPath)) { + throw new Exception("HTMLEditorField.use_gzip enabled, but file $gzipPath does not exist!"); + } + + require_once $gzipPath; + + $tag = TinyMCE_Compressor::renderTag(array( + 'url' => $config->getTinyMCEPath() . '/tiny_mce_gzip.php', + 'plugins' => implode(',', $config->getInternalPlugins()), + 'themes' => $config->getTheme(), + 'languages' => $config->getOption('language') + ), true); + preg_match('/src="([^"]*)"/', $tag, $matches); + + return html_entity_decode($matches[1]); + } +} diff --git a/src/Forms/HTMLEditor/TinyMCEScriptGenerator.php b/src/Forms/HTMLEditor/TinyMCEScriptGenerator.php new file mode 100644 index 000000000..e358bce32 --- /dev/null +++ b/src/Forms/HTMLEditor/TinyMCEScriptGenerator.php @@ -0,0 +1,17 @@ +assertEquals(['plugin4', 'plugin5'], $c->getInternalPlugins()); } - public function testPluginCompression() - { - // This test requires the real tiny_mce_gzip.php file - $module = ModuleLoader::inst()->getManifest()->getModule('silverstripe/admin'); - if (!$module) { - $this->markTestSkipped('No silverstripe/admin module loaded'); - } - TinyMCEConfig::config()->set('base_dir', 'silverstripe/admin:thirdparty/tinymce'); - Config::modify()->set(Director::class, 'alternate_base_url', 'http://mysite.com/subdir'); - - // Build new config - $c = new TinyMCEConfig(); - $c->setTheme('modern'); - $c->setOption('language', 'es'); - $c->disablePlugins('table', 'emoticons', 'paste', 'code', 'link', 'importcss'); - $c->enablePlugins( - array( - 'plugin1' => 'mypath/plugin1.js', - 'plugin2' => '/anotherbase/mypath/plugin2.js', - 'plugin3' => 'https://www.google.com/plugin.js', - 'plugin4' => null, - 'plugin5' => null, - ) - ); - $attributes = $c->getAttributes(); - $config = Convert::json2array($attributes['data-config']); - $plugins = $config['external_plugins']; - $this->assertNotEmpty($plugins); - - // Test plugins included via gzip compresser - HTMLEditorField::config()->update('use_gzip', true); - $this->assertEquals( - 'silverstripe-admin/thirdparty/tinymce/tiny_mce_gzip.php?js=1&plugins=plugin4,plugin5&themes=modern&languages=es&diskcache=true&src=true', - $c->getScriptURL() - ); - - // If gzip is disabled only the core plugin is loaded - HTMLEditorField::config()->remove('use_gzip'); - $this->assertEquals( - 'silverstripe-admin/thirdparty/tinymce/tinymce.min.js', - $c->getScriptURL() - ); - } - public function testDisablePluginsByString() { $c = new TinyMCEConfig(); @@ -199,7 +155,7 @@ class HTMLEditorConfigTest extends SapphireTest $this->assertNotEmpty($cAttributes['data-config']); } - public function testExceptionThrownWhenTinyMCEPathCannotBeComputed() + public function testExceptionThrownWhenBaseDirAbsent() { TinyMCEConfig::config()->remove('base_dir'); ModuleLoader::inst()->pushManifest(new ModuleManifest(__DIR__)); @@ -213,19 +169,4 @@ class HTMLEditorConfigTest extends SapphireTest ModuleLoader::inst()->popManifest(); } } - - public function testExceptionThrownWhenTinyMCEGZipPathDoesntExist() - { - HTMLEditorField::config()->set('use_gzip', true); - /** @var TinyMCEConfig|PHPUnit_Framework_MockObject_MockObject $stub */ - $stub = $this->getMockBuilder(TinyMCEConfig::class) - ->setMethods(['getTinyMCEPath']) - ->getMock(); - $stub->method('getTinyMCEPath') - ->willReturn('fail'); - - $this->expectException(Exception::class); - $this->expectExceptionMessageRegExp('/does not exist/'); - $stub->getScriptURL(); - } } diff --git a/tests/php/Forms/HTMLEditor/TinyMCECombinedGeneratorTest.php b/tests/php/Forms/HTMLEditor/TinyMCECombinedGeneratorTest.php new file mode 100644 index 000000000..99eb1cf21 --- /dev/null +++ b/tests/php/Forms/HTMLEditor/TinyMCECombinedGeneratorTest.php @@ -0,0 +1,84 @@ +set('alternate_base_folder', __DIR__ . '/TinyMCECombinedGeneratorTest'); + Director::config()->set('alternate_base_url', 'http://www.mysite.com/basedir/'); + TinyMCEConfig::config()->set('base_dir', 'tinymce'); + } + + public function testConfig() + { + // Disable nonces + $c = new TinyMCEConfig(); + $c->setTheme('testtheme'); + $c->setOption('language', 'en'); + $c->disablePlugins('table', 'emoticons', 'paste', 'code', 'link', 'importcss'); + $c->enablePlugins( + array( + 'plugin1' => 'mycode/plugin1.js', // + 'plugin2' => '/anotherbase/mycode/plugin2.js', + 'plugin3' => 'https://www.google.com/mycode/plugin3.js', + 'plugin4' => null, + 'plugin5' => null, + 'plugin6' => '/basedir/mycode/plugin6.js', + 'plugin7' => '/basedir/mycode/plugin7.js', + ) + ); + HTMLEditorConfig::set_config('testconfig', $c); + + // Get config for this + /** @var TinyMCECombinedGenerator $generator */ + $generator = Injector::inst()->create(TinyMCECombinedGenerator::class); + $this->assertEquals( + '_tinymce/tinymce-testconfig-8d695fc0be.js', + $generator->generateFilename($c), + "Filename for config: " . json_encode($c->getAttributes()) . " should match expected value" + ); + $content = $generator->generateContent($c); + $this->assertStringStartsWith("var tinyMCEPreInit={base:'tinymce',suffix:'.min'};\n", $content); + // Main script file + $this->assertContains("/* tinymce.js */\n", $content); + // Locale file + $this->assertContains("/* en.js */\n", $content); + // Local plugins + $this->assertContains("/* plugin1.js */\n", $content); + $this->assertContains("/* plugin4.min.js */\n", $content); + $this->assertContains("/* plugin4/langs/en.js */\n", $content); + $this->assertContains("/* plugin5.js */\n", $content); + $this->assertContains("/* plugin6.js */\n", $content); + // Exclude non-local plugins + $this->assertNotContains('plugin2.js', $content); + $this->assertNotContains('plugin3.js', $content); + // Exclude missing file + $this->assertNotContains('plugin7.js', $content); + + // Check themes + $this->assertContains("/* theme.js */\n", $content); + $this->assertContains("/* testtheme/langs/en.js */\n", $content); + + // Register done scripts + $this->assertStringEndsWith( + << Date: Tue, 1 Aug 2017 13:59:35 +1200 Subject: [PATCH 2/2] ENHANCEMENT Soft-code CSS explicit height and compute against rows --- src/Forms/HTMLEditor/HTMLEditorField.php | 19 ++++++++++++++++++- src/Forms/HTMLEditor/TinyMCEConfig.php | 1 - .../TinyMCECombinedGeneratorTest.php | 4 +++- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/Forms/HTMLEditor/HTMLEditorField.php b/src/Forms/HTMLEditor/HTMLEditorField.php index 0e2af5cf0..c84555571 100644 --- a/src/Forms/HTMLEditor/HTMLEditorField.php +++ b/src/Forms/HTMLEditor/HTMLEditorField.php @@ -51,7 +51,14 @@ class HTMLEditorField extends TextareaField * @config * @var int */ - private static $default_rows = 30; + private static $default_rows = 20; + + /** + * Extra height per row + * + * @var int + */ + private static $fixed_row_height = 20; /** * ID or instance of editorconfig @@ -110,7 +117,17 @@ class HTMLEditorField extends TextareaField public function getAttributes() { + // Fix CSS height based on rows + $rowHeight = $this->config()->get('fixed_row_height'); + $attributes = []; + if ($rowHeight) { + $height = $this->getRows() * $rowHeight; + $attributes['style'] = sprintf('height: %dpx;', $height); + } + + // Merge attributes return array_merge( + $attributes, parent::getAttributes(), $this->getEditorConfig()->getAttributes() ); diff --git a/src/Forms/HTMLEditor/TinyMCEConfig.php b/src/Forms/HTMLEditor/TinyMCEConfig.php index b11789e25..2bd132aeb 100644 --- a/src/Forms/HTMLEditor/TinyMCEConfig.php +++ b/src/Forms/HTMLEditor/TinyMCEConfig.php @@ -332,7 +332,6 @@ class TinyMCEConfig extends HTMLEditorConfig return [ 'data-editor' => 'tinyMCE', // Register ss.editorWrappers.tinyMCE 'data-config' => Convert::array2json($this->getConfig()), - 'style' => 'height:350px' ]; } diff --git a/tests/php/Forms/HTMLEditor/TinyMCECombinedGeneratorTest.php b/tests/php/Forms/HTMLEditor/TinyMCECombinedGeneratorTest.php index 99eb1cf21..46ceed868 100644 --- a/tests/php/Forms/HTMLEditor/TinyMCECombinedGeneratorTest.php +++ b/tests/php/Forms/HTMLEditor/TinyMCECombinedGeneratorTest.php @@ -8,6 +8,7 @@ use SilverStripe\Dev\SapphireTest; use SilverStripe\Forms\HTMLEditor\HTMLEditorConfig; use SilverStripe\Forms\HTMLEditor\TinyMCECombinedGenerator; use SilverStripe\Forms\HTMLEditor\TinyMCEConfig; +use SilverStripe\View\SSViewer; class TinyMCECombinedGeneratorTest extends SapphireTest { @@ -18,6 +19,7 @@ class TinyMCECombinedGeneratorTest extends SapphireTest // Set custom base_path for tinymce Director::config()->set('alternate_base_folder', __DIR__ . '/TinyMCECombinedGeneratorTest'); Director::config()->set('alternate_base_url', 'http://www.mysite.com/basedir/'); + SSViewer::config()->set('themes', [ SSViewer::DEFAULT_THEME ]); TinyMCEConfig::config()->set('base_dir', 'tinymce'); } @@ -45,7 +47,7 @@ class TinyMCECombinedGeneratorTest extends SapphireTest /** @var TinyMCECombinedGenerator $generator */ $generator = Injector::inst()->create(TinyMCECombinedGenerator::class); $this->assertEquals( - '_tinymce/tinymce-testconfig-8d695fc0be.js', + '_tinymce/tinymce-testconfig-6422b3814d.js', $generator->generateFilename($c), "Filename for config: " . json_encode($c->getAttributes()) . " should match expected value" );