From: Chuck Hagenbuch Date: Sun, 3 Oct 2010 01:10:52 +0000 (-0400) Subject: Re-work Horde_Editor to not do work in the constructor, and to use a simpler injector... X-Git-Url: https://git.internetallee.de/?a=commitdiff_plain;h=b924f983cfbc1accf3edebd15c8b2e7e4fd3fae5;p=horde.git Re-work Horde_Editor to not do work in the constructor, and to use a simpler injector factory --- diff --git a/ansel/img/ecard.php b/ansel/img/ecard.php index 4b61e3428..1b38050fa 100644 --- a/ansel/img/ecard.php +++ b/ansel/img/ecard.php @@ -95,7 +95,7 @@ $vars->set('image_desc', strlen($image->caption) ? $image->caption : $image->fil $form = new Ansel_Form_Ecard($vars, $title); $renderer = new Horde_Form_Renderer(); -$editor = $injector->getInstance('Horde_Editor')->getEditor('ckeditor', array('id' => 'ecard_comments')); +$editor = $injector->getInstance('Horde_Editor')->initialize(array('id' => 'ecard_comments')); if ($editor->supportedByBrowser()) { $vars->set('rtemode', 1); $form->addHidden('', 'rtemode', 'text', false); diff --git a/framework/Core/lib/Horde/Core/Binder/Editor.php b/framework/Core/lib/Horde/Core/Binder/Editor.php deleted file mode 100644 index c6bba70d8..000000000 --- a/framework/Core/lib/Horde/Core/Binder/Editor.php +++ /dev/null @@ -1,17 +0,0 @@ -_injector->getInstance('Horde_Browser'); - if (!$browser->hasFeature('rte')) { - return Horde_Editor::factory(); - } - - $params = array_merge( - Horde::getDriverConfig('editor', $driver), - $params, - array( - 'browser' => $browser - ) - ); - - return Horde_Editor::factory($driver, $params); + return $this->_injector->getInstance('Horde_Editor_Ckeditor'); } - } diff --git a/framework/Core/lib/Horde/Core/Ui/VarRenderer/Html.php b/framework/Core/lib/Horde/Core/Ui/VarRenderer/Html.php index bcbc917f0..a40082845 100644 --- a/framework/Core/lib/Horde/Core/Ui/VarRenderer/Html.php +++ b/framework/Core/lib/Horde/Core/Ui/VarRenderer/Html.php @@ -290,7 +290,7 @@ class Horde_Core_Ui_VarRenderer_Html extends Horde_Core_Ui_VarRenderer @htmlspecialchars($var->getValue($vars), ENT_QUOTES, $this->_charset)); if ($var->type->hasHelper('rte')) { - $GLOBALS['injector']->getInstance('Horde_Editor')->getEditor('ckeditor', array('id' => $varname, 'relativelinks' => $var->type->hasHelper('relativelinks'))); + $GLOBALS['injector']->getInstance('Horde_Editor')->initialize(array('id' => $varname, 'relativelinks' => $var->type->hasHelper('relativelinks'))); } if ($var->type->hasHelper() && $browser->hasFeature('javascript')) { diff --git a/framework/Core/lib/Horde/Registry.php b/framework/Core/lib/Horde/Registry.php index 99354eb1e..d2ae23f2f 100644 --- a/framework/Core/lib/Horde/Registry.php +++ b/framework/Core/lib/Horde/Registry.php @@ -319,6 +319,10 @@ class Horde_Registry 'Horde_Core_Controller_RequestMapper', 'getRequestConfiguration', ), + 'Horde_Editor' => array( + 'Horde_Core_Factor_Editor', + 'create', + ), 'Horde_Kolab_Server_Composite' => array( 'Horde_Core_Factory_KolabServer', 'getComposite', diff --git a/framework/Core/package.xml b/framework/Core/package.xml index 13c051f08..ad1a3b218 100644 --- a/framework/Core/package.xml +++ b/framework/Core/package.xml @@ -120,7 +120,6 @@ Application Framework. - @@ -445,7 +444,6 @@ Application Framework. - diff --git a/framework/Editor/lib/Horde/Editor.php b/framework/Editor/lib/Horde/Editor.php index 3d13c7877..a7c7f9986 100644 --- a/framework/Editor/lib/Horde/Editor.php +++ b/framework/Editor/lib/Horde/Editor.php @@ -30,31 +30,6 @@ class Horde_Editor protected $_js = ''; /** - * Attempts to return a concrete instance based on $driver. - * - * @param string $driver The type of concrete subclass to return. - * @param array $params A hash containing any additional configuration - * or connection parameters a subclass might need. - * - * @return Horde_Editor The newly created concrete instance. - * @throws Horde_Editor_Exception. - */ - static public function factory($driver = null, $params = null) - { - $driver = ucfirst(basename($driver)); - if (empty($driver) || (strcmp($driver, 'None') == 0)) { - return new Horde_Editor(); - } - - $class = __CLASS__ . '_' . $driver; - if (class_exists($class)) { - return new $class($params); - } - - throw new Horde_Editor_Exception('Driver ' . $driver . ' not found'); - } - - /** * Constructor. * * @param array $params The following configuration parameters: @@ -62,11 +37,13 @@ class Horde_Editor * 'browser' - (Horde_Browser) A browser object. * */ - public function __construct($params = array()) + public function __construct(Horde_Browser $browser) + { + $this->_browser = $params['browser']; + } + + public function initialize(array $params = array()) { - if (isset($params['browser'])) { - $this->_browser = $params['browser']; - } } /** @@ -88,21 +65,4 @@ class Horde_Editor { return false; } - - /** - * List the available editors. - * - * @return array List of available editors. - */ - static public function availableEditors() - { - $eds = array(); - - foreach (glob(dirname(__FILE__) . '/Editor/*.php') as $val) { - $eds[] = basename($val, '.php'); - } - - return $eds; - } - } diff --git a/framework/Editor/lib/Horde/Editor/Ckeditor.php b/framework/Editor/lib/Horde/Editor/Ckeditor.php index 5eba92a7d..13b3b008f 100644 --- a/framework/Editor/lib/Horde/Editor/Ckeditor.php +++ b/framework/Editor/lib/Horde/Editor/Ckeditor.php @@ -15,8 +15,6 @@ class Horde_Editor_Ckeditor extends Horde_Editor { /** - * Constructor. - * * @param array $params The following configuration parameters: * 
      * 'basic' - (boolean) Load "basic" editor (a small javascript stub that
@@ -29,18 +27,16 @@ class Horde_Editor_Ckeditor extends Horde_Editor
      *               instead be stored for access via getJS().
      *
*/ - public function __construct(array $params = array()) + public function initialize(array $params = array()) { - $params = array_merge(array( - 'config' => array() - ), $params); - - parent::__construct($params); - if (!$this->supportedByBrowser()) { return; } + $params = array_merge(array( + 'config' => array() + ), $params); + $ck_file = empty($params['basic']) ? 'ckeditor.js' : 'ckeditor_basic.js'; @@ -90,5 +86,4 @@ class Horde_Editor_Ckeditor extends Horde_Editor return false; } } - } diff --git a/framework/Model/lib/Horde/Form/VarRenderer/Xhtml.php b/framework/Model/lib/Horde/Form/VarRenderer/Xhtml.php index 856519b9a..081092961 100644 --- a/framework/Model/lib/Horde/Form/VarRenderer/Xhtml.php +++ b/framework/Model/lib/Horde/Form/VarRenderer/Xhtml.php @@ -156,7 +156,7 @@ class Horde_Form_VarRenderer_Xhtml extends Horde_Form_VarRenderer htmlspecialchars($var->getValue($vars))); if ($var->type->hasHelper('rte')) { - $GLOBALS['injector']->getInstance('Horde_Editor')->getEditor('ckeditor', array('id' => $var->getVarName())); + $GLOBALS['injector']->getInstance('Horde_Editor')->initialize(array('id' => $var->getVarName())); } if ($var->type->hasHelper() && $browser->hasFeature('javascript')) { diff --git a/framework/Model/notes.txt b/framework/Model/notes.txt new file mode 100644 index 000000000..c78e88c5a --- /dev/null +++ b/framework/Model/notes.txt @@ -0,0 +1,29 @@ +form tweaks: +Thanks for moving this to a public port, Lowell. Based on Chuck's request, +I do have a couple suggestions for making the form more easily +customizable. While it may seem like overkill, I find the best bet is to +follow this pattern: +
+
+
+
+ +The reason I prefer to do this is that it negates the need for
tags +since divs are block-level elements (using the br tag is technically +considered a non-semantic tag, and therefore bad practice). It also allows +you, through a stylesheet, to control whether or not the labels are above +the fields or to the side, just by adding classnames to the containing +divs. + +A couple of other suggestions: the label tags should have a "for" +attribute that matches the id value of the input they're referring to. This +is for accessibility reasons, and helps aid in tab indexing for +keyboard-oriented users. While we're talking about labels, it might be a +good idea to remove the colon after each label. It's better to group +related fields with white-space, and actually makes the form look less +complicated, believe it or not. + +Also, since the HTML 4.01 and XHTML 1.0 spec, the "name" attribute has +been replaced by the "id" attribute. You can safely remove the "name" +attribute from your form inputs and use "id" instead, which will allow for +compatibility with either doctype in any A-Grade browsers. diff --git a/gollem/edit.php b/gollem/edit.php index 99ff74f4a..3e55c001e 100644 --- a/gollem/edit.php +++ b/gollem/edit.php @@ -53,7 +53,7 @@ case 'edit_file': } if ($mime_type == 'text/html') { - $injector->getInstance('Horde_Editor')->getEditor('ckeditor', array('id' => 'content')); + $injector->getInstance('Horde_Editor')->initialize(array('id' => 'content')); } require GOLLEM_TEMPLATES . '/common-header.inc'; Gollem::status(); diff --git a/imp/lib/Auth.php b/imp/lib/Auth.php index ffafa0b01..65644e8d5 100644 --- a/imp/lib/Auth.php +++ b/imp/lib/Auth.php @@ -460,8 +460,7 @@ class IMP_Auth /* Is the HTML editor available? */ $imp_ui = new IMP_Ui_Compose(); - $editor = $GLOBALS['injector']->getInstance('Horde_Editor')->getEditor('Ckeditor', array('no_notify' => true)); - $sess['rteavail'] = $editor->supportedByBrowser(); + $sess['rteavail'] = $GLOBALS['injector']->getInstance('Horde_Editor')->supportedByBrowser(); /* Determine view. */ $setcookie = false; diff --git a/imp/lib/Prefs/Ui.php b/imp/lib/Prefs/Ui.php index 193015110..a8a940f33 100644 --- a/imp/lib/Prefs/Ui.php +++ b/imp/lib/Prefs/Ui.php @@ -186,7 +186,7 @@ class IMP_Prefs_Ui $ui->suppress[] = 'signature_html_select'; } else { Horde::addScriptFile('signaturehtml.js', 'imp'); - $GLOBALS['injector']->getInstance('Horde_Editor')->getEditor('Ckeditor', array('id' => 'signature_html')); + $GLOBALS['injector']->getInstance('Horde_Editor')->initialize(array('id' => 'signature_html')); } break; @@ -1723,7 +1723,7 @@ class IMP_Prefs_Ui $stationery = $GLOBALS['injector']->getInstance('IMP_Compose_Stationery'); if ($ob->type == 'html') { - $GLOBALS['injector']->getInstance('Horde_Editor')->getEditor('Ckeditor', array('id' => 'content')); + $GLOBALS['injector']->getInstance('Horde_Editor')->initialize(array('id' => 'content')); } $t = $GLOBALS['injector']->createInstance('Horde_Template'); diff --git a/imp/lib/Ui/Compose.php b/imp/lib/Ui/Compose.php index 1d850d21b..3b279ec6f 100644 --- a/imp/lib/Ui/Compose.php +++ b/imp/lib/Ui/Compose.php @@ -154,7 +154,7 @@ class IMP_Ui_Compose */ public function initRTE($basic = false) { - $GLOBALS['injector']->getInstance('Horde_Editor')->getEditor('Ckeditor', array('basic' => $basic)); + $GLOBALS['injector']->getInstance('Horde_Editor')->initialize(array('basic' => $basic)); $font_family = $GLOBALS['prefs']->getValue('compose_html_font_family'); if (!$font_family) { diff --git a/news/add.php b/news/add.php index 1fa187e51..8f39d818c 100644 --- a/news/add.php +++ b/news/add.php @@ -593,7 +593,7 @@ if ($form->validate()) { // Add editor now to avoud JS error notifications no redirect foreach ($conf['attributes']['languages'] as $key) { - $injector->getInstance('Horde_Editor')->getEditor('Ckeditor', array('id' => 'content_' . $key)); + $injector->getInstance('Horde_Editor')->initialize(array('id' => 'content_' . $key)); } require_once NEWS_TEMPLATES . '/common-header.inc';