Support for TinyMCE 3.3
| Project: | Wysiwyg |
| Version: | 6.x-2.x-dev |
| Component: | Editor - TinyMCE |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
TinyMCE 3.3b1 was released today. The changest most relevant to us include the removal of the 'safari' plugin and the addition of the 'advlist' extension. (The advlist extension lets you change the list style to things like 'circle', 'square' etc. At first glance it doesn't seem to be working for unordered lists, but that's just because Garland's styles override the <ul style="...."> set by the editor using more targeted styles for each <li>.)
Wysiwyg module does not recognize the new version because the minor version string is now partially outside the 80 char wide chunk ready by the version callback. Extending it to 100 chars works fine.
I also noted problems when loading the advlist extension after turning on script aggregation (I had changed the implementation to include tiny_mce.js for testing). It boiled down to the editorBasePath setting being an array instead of the expected string, the path to all other loaded editors were then merged into a single string!
Because of that, this patch also includes a fix for the todo in wysiwyg.module about moving the global editor parameters somewhere else. I simply put them right below Drupal.settings.wysiwyg.configs.editorName and it seems to work just fine after changing the TinyMCE implementation's init() method to look for the path there (no other implementation uses those variables yet).
Aggregating tiny_mce.js from 3.3 does not throw any errors that I can see in FF, but since versions 3.2.x can't be aggregated (even with the editroBasePath fix) I put a new version key in wysiwyg_tinymce_editor() to only turn on aggregation (or rather leave it enabled) for 3.3 and above.
I could only create a patch against 6.x-2.x-dev now due to problems reaching drupal.org, had to go via a proxy...
| Attachment | Size |
|---|---|
| wysiwyg-tinymce-3.3.patch | 4.14 KB |
#1
Works fine! Nice work.
#2
install sucess,but button not display.
why?(All buttons already choice)
#3
@pouvoir, I do not see any toolbar problems. Are you getting any JavaScript errors? Have you tried deleting the editor profile and creating a new one for TinyMCE?
#4
And here's a backported diff against wysiwyg 6.x-2.0, correctly detects tinymce here, didn't make more tests yet.
#5
When the wysiwyg enabled， the problem happen：
warning: Parameter 1 to theme_wysiwyg_profile_overview() expected to be a reference, value given in D:\Program\xampp\htdocs\drupal\includes\theme.inc on line 617.
#6
I tried to remove '&' in function in wysiwyg\wysiwyg.admin.inc like
function theme_wysiwyg_profile_overview(&$form) {
the problem fixed, but tinymce doesn't work.
#7
@pouvoir, the theme_wysiwyg_profile_overview problem is unrelated. It happens on the admin page when using PHP 5.3, using the editor should still work. It was fixed in 2.x-dev. See #613480: PHP 5.3 Support.
Was TinyMCE working before you applied the patch? Which version of Wysiwyg are you using? The patch is made for 6.x-2.x-dev, if you are using 6.x-2.0 you need the patch from landry.
Which browser are you using?
Do you see any JavaScript errors?
Have you tried with the Garland theme?
#8
Patch in #4 works fine for me, thanks!
#9
browser: firefox3.6
theme: Garland
I don't see any Javascript errors.
when I change to 6.x-2.x-dev, it works fine. so strange！
Thanks a lot！！
#10
Subscribing.
#11
subscribe
#12
Subscribe
#13
Subscribing i have the same problem here using TinyMCE 3.2.7 instead of 3.3b.
#14
Subscribing.
#15
Subscribing.
No problems in IE8, just in Firefox (3.5.7).
#16
@Aart, what are the problems in Firefox?
#17
Feeling a bit stupid: apparently all I had to do was delete the profile and create it again, like you suggested to Pouvoir. Now the toolbar is showing just fine in FF. Thanks!
#18
+++ wysiwyg.module 26 Jan 2010 02:28:08 -0000@@ -284,12 +284,12 @@ function wysiwyg_load_editor($profile) {
drupal_add_js(array('wysiwyg' => array(
- 'configs' => array($editor['name'] => array()),
- // @todo Move into (global) editor settings.
+ 'configs' => array($editor['name'] => array(
// If JS compression is enabled, at least TinyMCE is unable to determine
// its own base path and exec mode since it can't find the script name.
'editorBasePath' => base_path() . $editor['library path'],
'execMode' => $library,
+ )),
)), 'setting');
I'm basically ok with this change, but
1) the indentation is wrong
2) aren't the .init/.attach methods expecting the subkeys to be configurations for formats?
+++ editors/tinymce.inc 26 Jan 2010 02:28:08 -0000@@ -82,8 +100,8 @@ function wysiwyg_tinymce_editor() {
- // Version is contained in the first 80 chars.
...
+ // Version is contained in the first 80 chars before 3.3, then 100 chars.
Just change the number in the comment, please.
+++ editors/tinymce.inc 26 Jan 2010 02:28:08 -0000@@ -574,6 +592,16 @@ function wysiwyg_tinymce_plugins($editor
+ if (version_compare($editor['installed version'], '3.3', '>=')) {
+ unset($plugins['safari']);
The Safari compatibility plugin should be moved into a dedicated condition that only adds it for the actual certain version range.
+++ editors/tinymce.inc 26 Jan 2010 02:28:08 -0000@@ -574,6 +592,16 @@ function wysiwyg_tinymce_plugins($editor
+ 'extensions' => array('advlist' => t('Advanced lists')),
The title should be singular, I think.
Powered by Dreditor.
#19
#20
Subscribing
Edit:
I've noticed that on the TinyMCE download page (that Wysiwyg links to), 3.3 is the only available download.
That means users who are new to Wysiwyg may be confused right now if 3.2.x is the only working option with the recommended release of Wysiwyg.
#21
(Argh! I wrote this yesterday but just noticed I never submitted it, thankfully it was cached by the browser hehe)
1) I noticed that just after uploading the file, wanted to hear your opinion on the other changes before uploading a new one just for that.
2) Yes, at least the init method for TinyMCE does. It failed silently so I didn't notice it. As a workaround I wrapped the global settings in a special 'global' format and made the TinyMCE implementations skip it. (The TinyMCE 2.x implementation is broken for other reasons as well, just keeping them somewhat synced to.)
The CKEditor implementation is the only one which also uses the init method, but it already filters out any format keys for which don't also exist in the list of Drupal plugins, so no need for changes there (other than maybe just to indicate that the new 'global' format isn't a real one.
It''s not an optimal solution, but it makes sense to find all the editor settings below configs->editorName, global and format-specific. Another solution is to do the opposite and wrap the formatN keys in a 'formats' collection. But that would require a lot more changes and make the most commonly used settings go even deeper into the hierarchy.
3) Ok.
4) I just added a "if below 3.3" check around the safari plugin as it's already with another plugin in the "if above 3.0" check, if that's ok.
5) The full name in the plugin source is in plural. Left it as is, but I wouldn't mind if it was changed before commit.
#22
+++ editors/tinymce.inc 9 Feb 2010 02:03:10 -0000@@ -65,6 +65,24 @@ function wysiwyg_tinymce_editor() {
+ '3.3' => array(
...
+ 'libraries' => array(
+ '' => array(
+ 'title' => 'Minified',
+ 'files' => array('tiny_mce.js'),
+ ),
Since we are close to a new stable release, I'd prefer to add a @todo to the current 3.1 definition, stating that starting from 3.3, tiny_mce.js "may" work with enabled JS aggregation. ;)
+++ editors/tinymce.inc 9 Feb 2010 02:03:10 -0000@@ -566,10 +584,21 @@ function wysiwyg_tinymce_plugins($editor
+ 'extensions' => array('advlist' => t('Advanced lists')),
Singular would make more sense, because "Advanced image" and "Advanced link" are singular, too.
+++ editors/js/tinymce-3.js 9 Feb 2010 02:03:12 -0000@@ -17,13 +17,15 @@ Drupal.wysiwyg.editor.init.tinymce = fun
- // @todo Move global library settings somewhere else.
...
+ if (format == 'global') {
+ continue;
+ };
For now, I'd leave this @todo intact.
For 3.x, I think we already discussed several inheritance options...
1. Global settings
2. Format/profile settings
3. Profile/field settings (we possibly want to allow multiple profiles per format at some point)
I think we can go with this patch. One of my next best ideas (for 3.x) would be, whether something like
$.extend({}, Drupal.settings.wysiwyg.global.tinymce, settings);
or similar would work - so that we'd have a single settings variable containing everything - possibly requires a bit refactoring in a couple of places.
Powered by Dreditor.
#23
Completely noob here. I, too, am getting the "The version of TinyMCE could not be detected." error.
How and where do I apply this patch? Thanks.
#24
Updated to match sun's review.
#25
Thanks! 2 nitpicks that can hopefully be adjusted prior commit:
+++ wysiwyg.module 10 Feb 2010 16:40:10 -0000@@ -292,12 +292,12 @@ function wysiwyg_load_editor($profile) {
+ // If JS compression is enabled, at least TinyMCE is unable to determine
+ // its own base path and exec mode since it can't find the script name.
Slightly exceeds 80 chars - we can shorten + generalize this comment now.
+++ editors/tinymce.inc 10 Feb 2010 16:40:10 -0000@@ -45,6 +45,7 @@ function wysiwyg_tinymce_editor() {
+ // @todo Starting from 3.3, tiny_mce.js may support JS aggregation.
Trailing white-space here.
Powered by Dreditor.
#26
Committed to all branches.
Thank you for reporting, testing and reviewing! Support for TinyMCE 3.3 will be in all -dev snapshots within a few hours.
Slight modifications had to be done for DRUPAL-6--3 and HEAD as the settings had been moved higher up in the settings hierarchy there.
I also synced a bit with the TinyMCE 2.x implementations on all branches since the changes in wysiwyg.module affects it as well. It's still broken for other reasons tho, just trying to not add more of them.
Attaching the HEAD patch for reference.
EDIT: I just noticed sun meant the comment in wysiwyg.module, I only changed the corresponding one in the tinymce files. *sigh* Oh well, we have other long comments in there so I suggest we do a cleanup run soon to catch them all in one go.
#27
The heck. Do I need to apply for CVS account to patch it? I no nothing about patching. I got as far as to installing svntortoise on my Win7. I have this wysiwyg issue. Help!
#28
@bron, no, all you need is an up to date -dev snapshot (assuming the patch still applies to that code) and a program capable of understanding patch/diff files. TortoiseSVN will do fine, see http://drupal.org/patch/apply.
However, this patch is already committed to CVS and is thus already part of the automatically generated -dev snapshots.
#29
Still, a problem there I guess. I installed the dev snapshot and I discovered that now when I input with tinyMCE I get info that I have to provide text for that field - It doesnt happen when I disable the rich text editor.
#30
Sounds like what you typed in the editor doesn't get transferred to the original textarea before the form is submitted. If you have an "AJAXifying" module installed, please try disabling it as there might be compatibility problems.
If the editor is detected as being installed, which seems to be the case, please open a new issue about your problem (if you can't find an existing one of course).
#31
Please could this be released?
It's a critical bug that breaks a popular editor completely. Three weeks with this fix still unreleased is too long.
#32
Let's discuss over in #652410: Wysiwyg 2.1
#33
Automatically closed -- issue fixed for 2 weeks with no activity.