Bug in implementation of external filebrowsers

A place to report and discuss bugs - please mention CMSimple-version, server, platform and browser version
Post Reply
Holger
Site Admin
Posts: 3470
Joined: Mon May 19, 2008 7:10 pm
Location: Hessen, Germany

Bug in implementation of external filebrowsers

Post by Holger » Sat Sep 28, 2013 11:26 pm

Hi,

there is a bug (which I have introduced in XH 1.5 :oops: ) in the implementation of external filebrowsers - AFAIK in all versions since 1.5.1.

Caused by a wrong if/else clause, the fallback code for the internal filebrowser gets never executed, if $cf['filebrowser']['external'] != FALSE.
The active editor might crash, if the external filebrowser not provide a connector for this configured editor.

That was not a problem until now, because all available filebrowsers have compatible connectors for all available editors.
But with TinyMCE4, this behavior might change...

Beside this, the TinyMCE4-plugin in 1.6beta contains wrong code to detect the filebrowser - connector (in fact it's just a copy of the code for TinyMCE 3x).
The init-function to call the filebrowser must always reflect the name of the external filebrowser and the external editor:

Code: Select all

$init_function = $cf['filebrowser']['external'] . '_tinymce4_init'; 
The patches below are created on R941, but it's the same code in all CMSimple_XH - versions sice 1.5.1.

/plugins/tinymce/init.php:

Code: Select all

Index: init.php
===================================================================
--- init.php    (revision 941)
+++ init.php    (working copy)
@@ -22,26 +22,23 @@
     //Einbindung alternativer Filebrowser, gesteuert über $cf['filebrowser']['external']
     //und den Namen des aufrufenden Editors
     if ($cf['filebrowser']['external'] != FALSE) {
-    $fbConnector = CMSIMPLE_BASE . 'plugins/' . $cf['filebrowser']['external'] . '/connectors/tinymce/tinymce.php';
-    if (is_readable($fbConnector)) {
-        include_once($fbConnector);
-        $init_function = $cf['filebrowser']['external'] . '_tinymce_init';
-        if (function_exists($init_function)) {
-            $script = $init_function();
-        }
-    return $script;
+        $fbConnector = CMSIMPLE_BASE . 'plugins/' . $cf['filebrowser']['external'] . '/connectors/tinymce/tinymce.php';
+        if (is_readable($fbConnector)) {
+            include_once($fbConnector);
+            $init_function = $cf['filebrowser']['external'] . '_tinymce_init';
+            if (function_exists($init_function)) {
+                $script = $init_function();
+                return $script;
+            }
+        }
     }
 
-    } else {
-
     //default filebrowser
     $_SESSION['tinymce_fb_callback'] = 'wrFilebrowser';
     $url =  CMSIMPLE_ROOT . 'plugins/filebrowser/editorbrowser.php?editor=tinymce&prefix=' . CMSIMPLE_BASE . '&base=./';
     $script = file_get_contents(dirname(__FILE__) . '/filebrowser.js');
     $script = str_replace('%URL%',  $url, $script);
     return $script;
-
-    }
 }
 
And additional for XH 1.6beta
/plugins/tinymce4/init.php:

Code: Select all

Index: init.php
===================================================================
--- init.php    (revision 941)
+++ init.php    (working copy)
@@ -18,26 +18,23 @@
     //Einbindung alternativer Filebrowser, gesteuert über $cf['filebrowser']['external']
     //und den Namen des aufrufenden Editors
     if ($cf['filebrowser']['external'] != FALSE) {
-    $fbConnector = CMSIMPLE_BASE . 'plugins/' . $cf['filebrowser']['external'] . '/connectors/tinymce/tinymce.php';
-    if (is_readable($fbConnector)) {
-        include_once($fbConnector);
-        $init_function = $cf['filebrowser']['external'] . '_tinymce_init';
-        if (function_exists($init_function)) {
-            $script = $init_function();
-        }
-    return $script;
+        $fbConnector = CMSIMPLE_BASE . 'plugins/' . $cf['filebrowser']['external'] . '/connectors/tinymce4/tinymce4.php';
+        if (is_readable($fbConnector)) {
+            include_once($fbConnector);
+            $init_function = $cf['filebrowser']['external'] . '_tinymce4_init';
+            if (function_exists($init_function)) {
+                $script = $init_function();
+                return $script;
+            }
+        }
     }
 
-    } else {
-
     //default filebrowser
     $_SESSION['tinymce_fb_callback'] = 'wrFilebrowser';
     $url =  CMSIMPLE_ROOT . 'plugins/filebrowser/editorbrowser.php?editor=tinymce4&prefix=' . CMSIMPLE_BASE . '&base=./';
     $script = file_get_contents(dirname(__FILE__) . '/filebrowser.js');
     $script = str_replace('%URL%',  $url, $script);
     return $script;
-
-    }
 }
 
@Christoph: would you please apply the patches above before a new beta of 1.6 gets published? I have no write access at SF.

Holger

cmb
Posts: 14225
Joined: Tue Jun 21, 2011 11:04 am
Location: Bingen, RLP, DE
Contact:

Re: Bug in implementation of external filebrowsers

Post by cmb » Sun Sep 29, 2013 12:04 pm

Holger wrote:The active editor might crash, if the external filebrowser not provide a connector for this configured editor.
Indeed. :o
Holger wrote:The init-function to call the filebrowser must always reflect the name of the external filebrowser and the external editor:
Agreed. However, I'm not sure how to handle this for TinyMCE. In the long run TinyMCE 3 will be replaced by TinyMCE 4. Then it may be reasonable to change its name to tinymce. If the existing tinymce connectors work for both TinyMCE versions, we might consider to stick with the current behavior. :?
Holger wrote:@Christoph: would you please apply the patches above before a new beta of 1.6 gets published? I have no write access at SF.
As manu is the maintainer of the TinyMCE plugins, he should do that. Anyway, do you have a SF account? Than I'll grant you write access (should have been done already :)).

Christoph
Christoph M. Becker – Plugins for CMSimple_XH

Holger
Site Admin
Posts: 3470
Joined: Mon May 19, 2008 7:10 pm
Location: Hessen, Germany

Re: Bug in implementation of external filebrowsers

Post by Holger » Sun Sep 29, 2013 9:34 pm

cmb wrote:If the existing tinymce connectors work for both TinyMCE versions, we might consider to stick with the current behavior.
In TinyMCE4 has the API for the filebrowsers, or to be more exact, the API to include HTML-Dialogs changed.
Of course it might be reasonable to make one compatible version for TinyMCE 3x and 4x.
But the code is different for the two branches. So the filebrowser-connector has to check which API is availiabe which seems
to be impossible or only hard to manage.
A "solution" for 3x and 4x might be a check if Tiny_MCE_Popup.js is available, but that's IMO a "dirty hack".
And BTW, a user could install the compatipility-plugin in 4x which contains Tiny_MCE_Popup.js... :?

From my experience with FCK & CKeditor I would suggest to use different connectors and editor-plugins / names.
Maybe the naming "Tinymce4" is not best choice for a 5x. But I bet that in the 5x branch the API changes again...

Another idea could be a function or another way to tell the connector the version of the editor-core.
But, to be honest, why should we do a lot of coding just to keep on the same name? And, not to forget, you need a different plugin name anyway, when you
want to keep the possibility to install different versions together on one installation as it's done for now in XH 1.6.

Holger

cmb
Posts: 14225
Joined: Tue Jun 21, 2011 11:04 am
Location: Bingen, RLP, DE
Contact:

Re: Bug in implementation of external filebrowsers

Post by cmb » Mon Sep 30, 2013 10:49 am

Holger wrote:In TinyMCE4 has the API for the filebrowsers, or to be more exact, the API to include HTML-Dialogs changed.
That's a good reason to have separate connectors, so it's best to stick with the name tinymce4.
Holger wrote:And, not to forget, you need a different plugin name anyway, when you
want to keep the possibility to install different versions together on one installation as it's done for now in XH 1.6.
That's only a workaround, because TinyMCE 4 is not yet mature enough to replace 3.x, but 3.x actually seems to be discontinued. :(
Christoph M. Becker – Plugins for CMSimple_XH

manu
Posts: 1086
Joined: Wed Jun 04, 2008 12:05 pm
Location: St. Gallen - Schweiz
Contact:

Re: Bug in implementation of external filebrowsers

Post by manu » Tue Oct 01, 2013 4:06 pm

Bug corrected in both tinymce/init.php and tinymce4/init.php (rev 962).
manu

Post Reply