Old Whizzywig instance not removed on detach().

Hi, you must be the person I tried to reply to on IRC, sorry I was too late.
Yes, the toolbar image must be placed in the root as Whizzywig is hard-coded to look for it there.

I've noticed the other error you mentioned and I'll try to find the cause ASAP.

The latest version of Whizzywig (56) accepts different parameters to the tidyH function (and also generates XHTML without xhtml.js) which makes the Wysiwyg script break.
The attached patch should fix it while being backwards compatible with older versions of Whizzywig.

The patch was rolled agains Wysiwyg HEAD but could also be tested using Wysiwyg 2.0.
It's a simple patch but let's just make sure we don't have a second cause for this problem.

Btw, you can use the custom button set available from the official Whizzywig site if you extract the images into "libraries/whizzywig/btn".
I'm considering if maybe we should change the logic related to that so if the "btn" folder doesn't exist it uses "sites/*/libraries/whizzywig/WhizzywigToolbar.png" if that exist, then falls back to text-only buttons. Now it just looks for the "btn" folder and then falls back to Whizzywig's default (WhizzywigToolbar.png in the root folder).

+++ editors/js/whizzywig.js	26 Aug 2009 19:11:15 -0000
@@ -79,8 +79,8 @@ Drupal.wysiwyg.editor.detach.whizzywig =
+    var ver = parseInt(whizzywig_version.substr(11), 10);
+    body.innerHTML = (ver >= 56) ? tidyH(instance.contentWindow.document) : tidyH(body.innerHTML);

Can we squeeze a comment above this, explaining the WTF? :)

Otherwise, looks ready to fly.

This review is powered by Dreditor.

Actually, this raises a very interesting question. Originally, the intention was to support multiple PHP as well as JS and CSS integration files, as new editor versions appear.

So this patch adds version-specific logic to the existing JS file... is that the way we want to move forward?

Because, if it is, then we may want to re-consider how much sense the built-in version support for different integration functions and files makes in the long run...

Good point! I didn't think of it that way but now that you mention it it makes sense. This is an "API change" in the editor, even if it's a small one.

This patch creates a new JS implementation for versions 56 and above. I changed the call to tidyH() as above, and included a fix for #659680: WhizzywigToolbar.png not found (details in that issue) as the fixes both belong in this new file and the relevant problems appeared in the same release. Other than that, its the same as before.
No changes were made to the old JS file.

Thanks for reporting, reviewing, and testing! Committed to all branches.

A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.

+++ editors/whizzywig.inc	2 Jan 2010 01:05:55 -0000
@@ -27,6 +27,9 @@ function wysiwyg_whizzywig_editor() {
+      '56' => array(
+        'js files' => array('whizzywig56.js'),

Our current convention is to use a hyphen/dash between the name and version.

+++ editors/whizzywig.inc	2 Jan 2010 01:05:55 -0000
@@ -73,6 +76,9 @@ function wysiwyg_whizzywig_settings($edi
+  if (is_file($editor['library path'] . '/WhizzywigToolbar.png')) {

is_file() can have strange implications on certain PHP versions and platforms, reading in the entire file to figure out whether it is a file. Changed to file_exists().

+++ editors/js/whizzywig56.js	2 Jan 2010 01:05:55 -0000
@@ -0,0 +1,117 @@
+  // Previos versions used per-button images found in this location,
+  // now they're only used by custom buttons; none under Wysiwyg.

Fixed that typo and improved the added comments in general.

Powered by Dreditor.