User tests: Successful: Unsuccessful:
TinyMCE options are included in page HTML as inline JSON. This PR cleans up the script by replacing \r\n\t with spaces making the resulting HTML both smaller and more readable.
View HTML of existing page with TinyMCE.
Apply PR.
Compare revised HTML.
script": [ "!(function(){\n\t\t\t\tvar getBtnOptions = new
Function(\"return {handler: \\'iframe\\', size: {x: 800, y:
500}}\"),\n\t\t\t\t\tbtnOptions = getBtnOptions(),\n\t\t\t\t\tmodalWidth
= btnOptions.size && btnOptions.size.x ? btnOptions.size.x :
null,\n\t\t\t\t\tmodalHeight = btnOptions.size && btnOptions.size.y ?
btnOptions.size.y : null;\n\t\t\t\teditor.addButton(\"button-0Module\",
{\n\t\t\t\t\ttext: \"Module\",\n\t\t\t\t\ttitle:
\"Module\",\n\t\t\t\t\ticon: \"none icon-file-add\",\n\t\t\t\t\tonclick:
function () {\n\t\t\t\t\t\t\tvar modalOptions = {\n\t\t\t\t\t\t\t\ttitle
: \"Module\",\n\t\t\t\t\t\t\t\turl :
'http:\/\/www.longwinter.co.uk\/administrator\/index.php?option=com_modules&view=modules&layout=modal&tmpl=component&editor=jform_intro_text&780ac24446fbf734ef8b34ddbd2b92a1=1',\n\t\t\t\t\t\t\t\tbuttons:
[{\n\t\t\t\t\t\t\t\t\ttext : \"Close\",\n\t\t\t\t\t\t\t\t\tonclick:
\"close\"\n\t\t\t\t\t\t\t\t}]\n\t\t\t\t\t\t\t}\n\t\t\t\t\t\t\tif(modalWidth){\n\t\t\t\t\t\t\t\tmodalOptions.width
=
modalWidth;\n\t\t\t\t\t\t\t}\n\t\t\t\t\t\t\tif(modalHeight){\n\t\t\t\t\t\t\t\tmodalOptions.height
=
modalHeight;\n\t\t\t\t\t\t\t}\n\t\t\t\t\t\t\teditor.windowManager.open(modalOptions);\n\t\t\t\t\t}\n\t\t\t\t});\n\t\t\t})();"]
script": [ "!(function(){\nvar getBtnOptions = new Function(\"return
{handler: \\'iframe\\', size: {x: 800, y: 500}}\"),\nbtnOptions =
getBtnOptions(),\nmodalWidth = btnOptions.size && btnOptions.size.x ?
btnOptions.size.x : null,\nmodalHeight = btnOptions.size &&
btnOptions.size.y ? btnOptions.size.y : null;\neditor.addButton(\"button-0Module\", {\ntext: \"Module\",\ntitle:
\"Module\",\nicon: \"none icon-file-add\",\nonclick: function () {\nvar
modalOptions = {\ntitle : \"Module\",\nurl :
'http:\/\/www.longwinter.co.uk\/administrator\/index.php?option=com_modules&view=modules&layout=modal&tmpl=component&editor=jform_intro_text&780ac24446fbf734ef8b34ddbd2b92a1=1',\nbuttons: [{\ntext : \"Close\",\nonclick: \"close\"\n}]\n}\nif(modalWidth){\nmodalOptions.width = modalWidth;\n}\nif(modalHeight){\nmodalOptions.height
= modalHeight;\n}\neditor.windowManager.open(modalOptions);\n}\n});\n})();",]
Status | New | ⇒ | Pending |
Category | ⇒ | Front End Plugins |
Labels |
Added:
?
|
That will teach me to make changes without fully testing.
This is the old issue about the javascript parser inserting missing semicolons when needed when there is a line break. Obviously you cannot simply replace every line break with a semi-colon, nor follow every }
with a semi-colon either.
I will change this to replace only \t and spaces and not all \s characters.
Not as nice looking as before but it remains functional which is more important.
@Sophist-UK ok this crappy output is my fault but there is a much better solution here:
convert the tempConstructor
to and array and add each line of the script there, then implode($tempConstructor, ''); instead of the regex you used here. Also should be more performant and sorry for this piece of code
That might be a better solution - and I know it is a standard approach for HTML. However it would be likely to suffer from the same issues as noted earlier i.e. the removal of line endings might cause issues with implied semi-colons that are inserted by the parser i.e. when you have "}\n something" the parser inserts a ";" to make it "};\n something". When you remove the "\n" to make "} something", the semi-colon is not inserted and you get JS errors.
So IMO we should stick with this solution which starts from well indented and hence readable JS.
That might be a better solution
Well no way! I mean we are constructing some html and then we are processing what we already constructed few lines above, that's plain wrong! Construct the html/js correctly in the first place.
Just to give you the history behind this mess:
When I added the xtd-button to tinymce, the editor was using an inline script for initialisation. Later on @Fedik did some great work for the customisation part and the inline script was converted to static file. So when this code was merged the tabs and returns was kept intensionally to output a prettified output. What we never did was to convert this code to be uglified as now it's passed in the JSON string (which by the way is also totally wrong). Long story sort please fix this in the construction time don't do it through regex.
This should be ok
// Now we can built the script
$tempConstructor[] = '!(function(){';
$tempConstructor[] = 'editor.addButton("' . $name . '", {';
$tempConstructor[] = 'text: "' . $title . '",';
$tempConstructor[] = 'title: "' . $title . '",';
$tempConstructor[] = 'icon: "' . $icon . '",';
$tempConstructor[] = 'onclick: function () {';
if ($href || $button->get('modal'))
{
$tempConstructor[] = 'var modalOptions = {';
$tempConstructor[] = 'title: "' . $title . '",';
$tempConstructor[] = 'url: "' . $href . '",';
$tempConstructor[] = 'buttons: [{text: "Close",onclick:"close"}]};';
$tempConstructor[] = 'modalOptions.width=parseInt(' . intval($options["width"]) . ', 10);';
$tempConstructor[] = 'modalOptions.height=parseInt(' . intval($options["height"]) . ', 10);';
$tempConstructor[] = 'editor.windowManager.open(modalOptions);';
if ($onclick && ($button->get('modal') || $href))
{
$tempConstructor[] = $onclick;
}
}
else
{
$tempConstructor[] = $onclick;
}
$tempConstructor[] = '}';
$tempConstructor[] = '});';
$tempConstructor[] = '})();';
// The array with the toolbar buttons
$btnsNames[] = $name . ' | ';
// The array with code for each button
$tinyBtns[] = implode($tempConstructor, '');
break;
Ok.
Just to be safe please use $tempConstructor[] = $onclick . ';';
instead of $tempConstructor[] = $onclick;
in both places
Sorry - but I am not going to make this change.
It involves essentially a rewrite of the javascript, and I do not have the time to check that what you have written - or my own alternative - is functionally correct and to adequately test the changed code.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-02-18 21:54:15 |
Closed_By | ⇒ | Sophist-UK |
By the way the code that i posted above is tested/working as I tested it so all you have to do is copy paste
Sorry - but if it is not my code I cannot take responsibility for it. Since you have the code working and tested, perhaps you can submit a replacement PR.
P.S. If you submit a PR I will happily undertake a formal test for it.
After applying the patch, the editor is no longer in WYSIWYG mode. I have to toggle the editor. Also the editor switched to Set 1 from Set 0.