Insert image leads to broken links in some circumstances

I can't reproduce this issue. If you edit the image using the Image button, does its URL look correct? If you disable rich text editing or look in the editor's source mode, does the the image URL still look alright? Can you copy that URL, point your browser there and see the image?

Does this also happen with 7.x-2.1?

That's one of TinyMCE's temporary tags it uses internally. It should either get removed or replaced by the actual tag for the image you're inserting. I don't think Wysiwyg is affecting this process directly, but any errors reported by the browser could tell for sure.

I have the same problem, when i mark the broken image and click to edit the image there is no path, when I now go to the file browser select the same or a new image again it works.

I use the latest TinyMCE (version 3.4.3.1) and Wysiwyg 7.x-2.x.

@mlehew: Have you found s.th. in the TinyMCE issues or opened a ticket there?

I think I've just about tracked the bug down. If you include a button that inclues the 'font' plugin (for me this was the HTML Block Format button) in editors/tinymce.inc you'll get a span element added to the extended_valid_elements list. The current code doesn't include 'id' as an attribute for spans and it appears that TinyMCE requires this for it's initial image insert. Changing the list of extended valid elements for the font stanza to the following appears to fix the bug:

array('font[face|size|color|style],span[id|class|align|style]'), 

I'm having no problems inserting images when the HTML Block Format button is enabled. I've searched the code to see if spans are actually used when inserting img tags but that turned up nothing.

Since I can't reproduce this locally, I'm going to need access to a site where this problem exists so I can debug it, if you aren't able to do it yourselves. If anyone's willing to give me access, please use my contact form and reference this issue.

(Btw, "needs review" is for when there are .patch files.)

I just set up a test installation with minimum of modules + wysiwyg. I use TinyMCE (tested with version 3.4.3.1 or 3.3.9.4) in default/libraries/tinymce. I found that if I enable one of the following plugins: Font, Font size, Font style or HTML block format (together with Image and IMCE) i'll get the same as #2: <img id="__mce_tmp" alt="" />. The image dialog won't close. I need to hit the cancel button. With ckeditor it works fine.

@TwoD: If you want, I can send you the files of the minimal installation + database dump.

Same problem here, fix at #6 worked for me.

Fix from elliot in #6 works, thanks. I don't understand why to fix it this way but finally I created a patch.

@TwoD: Where you able to reproduce the error with the minimal installation I had sent to you? I hope so.

I found that I was using an older version than I first thought, which is why I couldn't reproduce this problem.

The TinyMCE commit ed43eebdfb34988ba7802a62115640e82b8e3706 triggered this issue in version 3.4.3. TinyMCE uses a span tag to track where the caret should be positioned after inserting content in the editor. After the change this temporary span is getting processed by the user defined valid elements filter. If extended_valid_elements overrides the span tag, it must include the id attribute or the new code won't preserve it during filtering. When TinyMCE later tries to find that span [by id], it gets null and crashes soon after. The exception causing the crash isn't seen in the main window (at least not in Chrome) because the call originated from the Image dialog. Attaching a debugger in that window did show the exception and made it possible to debug the code.

I haven't yet worked out exactly how the code paths differ before and after that commit, so I'm not 100% sure this is not something we should do something about, rather than filing a bug at TinyMCE. Filtering internal temporary elements based on user defined rules does seem like an odd thing to do though...

I guess it goes without saying, as this is caused by external libraries, but I'm getting the same problem with the latest 6.x-dev and tinymce 3.4.3.2.

The fix at #6 / #11 works perfect. Thanks.

don't know where the tags came from. my apologies

elliot
Thanks for the post. Spent hours trying to figure out what was wrong, and this fixed it in a jiffy.

elliot and osopolar,
Thank you so much for finding the fix for this and posting it. I've been trying to figure this out for a week now and the fix posted in #6/#11 worked perfectly!

Excellent! The patch in #7 works like a gem for me.

• WYSIWYG 7.x-2.1
• IMCE 7.x-1.4
• IMCE WYSIWYG 7.x-1.x-dev
• TinyMCE 3.4.3.2

Thanks!

I have the same problem ... and to solve it I disabled the "HTML Format" and "Font Size" functions of the toolbar (but I need these features).

Have you tested if these two functions also works by applying this patch ?

Thanks :)

I haven't tested with HTML Format disabled, but I do have Font Size disabled on my toolbar. I need HTML Format, of course.

The problem with this workaround is that if anyone overrides the span definition using hook_wysiwyg_editor_settings_alter(), the fix will be undone if they forget the id attribute. It'd be better if this could be fixed in TinyMCE itself, since id attributes are not required, and perhaps not desired by some, on span tags. I'll try searching their bugtracker some more and if I can't find anything I'll file a new one.

Sorry but how to I install a patch?

Using WYSIWYG with and the IMCE Bridge. The Process I run to inset an image is as follows ...

WYSIWYG –> 6.x.2.4
IMCE –> 6.x.2.2
IMCE Bridge –> 6.x.1.1
Tiny mce –> 3.4.3.2

Click image button

Click the browse button

Using the file browser I select an image and click insert file

The picture is now shown in the preview window

Click the insert button

Now this is where a broken image is now displayed in the TinyMCE editor window. When I view the source this is what I get ...

Solution :-

Get a span element added to the extended_valid_elements list. The current code doesn't include 'id' as an attribute for spans and it appears that TinyMCE requires this for it's initial image insert. Changing the list of extended valid elements for the font stanza to the following appears to fix the bug:

array('font[face|size|color|style],span[id|class|align|style]'),

Path :-

module/wysiwyg/editors/tinymce.inc

diff -Naur wysiwyg-old/editors/tinymce.inc wysiwyg/editors/tinymce.inc

--- wysiwyg-old/editors/tinymce.inc 2011-07-03 19:58:52.000000000 +0200

+++ wysiwyg/editors/tinymce.inc 2011-07-03 19:38:40.000000000 +0200

@@ -451,7 +451,7 @@

'font' => array(

'path' => $editor['library path'] . '/plugins/font',

'buttons' => array('formatselect' => t('HTML block format'), 'fontselect' => t('Font'), 'fontsizeselect' => t('Font size'), 'styleselect' => t('Font style')),

- 'extended_valid_elements' => array('font[face|size|color|style],span[class|align|style]'),

+ 'extended_valid_elements' => array('font[face|size|color|style],span[id|class|align|style]'),

'url' => 'http://wiki.moxiecode.com/index.php/TinyMCE:Plugins/font',

'internal' => TRUE,

),

Line No :- 463

'extended_valid_elements' => array('font[face|size|color|style],span[class|align|style]'),

Replace with :-

'extended_valid_elements' => array('font[face|size|color|style],span[id|class|align|style]'),

I thinks above information fixed your problem :)

Forgot the downgrade workaround...

@Abhinesh Sharma: This is the workaround described in #6 with patch in #11. Fixed does not mean that there is a solution to solve or workaround a problem, it means that the problem won't appear when you install or update your module. But nice to see that the workaround helped you.

subscribe

Added note about known workarounds.

Typo fix.

This happens with any version of Wysiwyg (and the workaround patch in #11 should work with all of them) so I'll change this to the -dev to give it focus.
#11 is a good patch, and this regression is making the latest release of the editor unusable in common situations so I'd like to commit it. I'm however sure we'll at some point forget why the id attribute is there when just looking at the code.
How about adding this comment above it?

Version 3.4.3 has a regression where ids of internal spans also get filtered.

So the latest version of TinyMCE without this bug is is 3.4.2? Or better downgrade TinyMCE to 3.3.9.4?

Yes, would be nice to fix this temporarily in the dev, with the mentioned comment and an link to this issue, with a notice here and setting it to postponed (needs to be fixed in TinyMCE). As said in http://stackoverflow.com/questions/6688527/tinymce-j-is-undefined this probably will be fixed soon.

Downgrading to TinyMCE 3.3.9.4 didn't solve it for me (still displayed a broken image). But, elliot's fix at #6 did. Sheer genius, sir. Thanks a ton.

Aha, then I think patch at #11 should really get committed asap. It's only a very little change but seems to fix a lot of problems.

I tested with 3.4.2 and 3.3.9.4 again and can confirm the problem is not showing up with those versions. (They''re available under Downloads on their Github account.)
When switching between TinyMCE versions it's very important to clear the browser cache or it may still load the previously used version.

And yes, the issue number would be prefixed to the code comment I posted above.

Good to know that TinyMCE 3.4.2 is the last "good" version. Would you commit the proposed patch (along with the mentioned comment) anyway?

absolutely, you are genius!!!

I would, but I think this needs a little bit more testing. Note that the patch only adds the id attribute to extended_valid_elements to the font plugin. Normally, Wysiwyg won't tell an editor to load a plugin unless at least one of its buttons are enabled (or it's an enabled button-less extension).

I just want to verify that this still works if none of "HTML block format", "Font", "Font size" or "Font styles" have been enabled. (Note, some of those are broken, see #970452: Fix TinyMCE plugin names and remove outdated plugin information.)
Like, what happens if just the image plugin is enabled and one tries to insert an image at the cursor position? TinyMCE would still use a temporary span, but will it be filtered out?

I'm in the middle of something else right now so I can't change my dev environment to test, anyone who can help?

This seems very easy to test. Somebody just needs to ensure all checkboxes under "Buttons and plugins" at admin/settings/wysiwyg/profile/[profile_id]/edit are unticked.

You could also help in testing it if you want to speed up development.

This patch, alas, does NOT work with WYSIWYG filter.

It works in Full HTML mode, but not in any of my filtered formats.

I am unable to add the span ID to WYSIWYG filters, because it thinks it's an invalid format (__mce_tmp - it rejects the __ at the beginning.) So in my case it is sort of a combination-bug caused by two things. My understanding was that WYSIWYG filters were only applied on save, though, so who knows what I'm doing wrong... If I switch to Full HTML, insert the image, and switch back to a filtered format, the image stays. I tried adding the __mce_temp to the filter database manually as a serialized value, and allowing span ID, but no dice.

On the other hand, the patch doesn't STOP anything else from working that wasn't working before, so :)

If the patch works around a few problems but does not introduce new ones, then there should be no reason against committing it.

I think that this might be fixed in TinyMCE 3.4.4. Bug is gone for me.

Same here! Bug is gone after upgrading TinyMCE from 3.4.3.2 to v3.4.4. So the problem only occurs with "bad" TinyMCE versions 3.4.3.x (3.4.3, 3.4.3.1 and 3.4.3.2).

Since this problem was not caused by the drupal module, I have changed the category from "bug report" to "support request". Nothing to do here except maybe documenting not to use TinyMCE 3.4.3.x.

yah! im happy to now! tinymce update fixing this.

Excellent - marking this issue as fixed.

Great! Better it was fixed in TinyMCE than here, since we'd have to guess or examine each of its plugins to know which ones could potentially use ids in span tags. (I also have a feeling this regression in TinyMCE could have caused similar issues with other attributes/tags used internally if they were not in either valid_elements or extended_valid_elements.)

@Reinette, this has nothing to do with WYSIWYG Filter, any filter/format settings or any other Drupal module. As you say, format filters only run during content rendering. This problem appears inside TinyMCE itself when it tries to find the caret location during/after content was inserted by someone or something. I'm guessing that when using the editors for the formats where you could still not insert images correctly did not have the font plugin active. As such, the extended_valid_elements override provided by our meta-data for that plugin was never sent to the editor, so the id attribute would still not be valid on spans inside the editor.

TwoD: Thanks for that explanation, I'll see if that's the issue!

I'll have to revisevmy last statement above, as it was brought to my attention that WYSIWYG Filter does indeed change how TinyMCE behaves when outputting markup. It does this by implementing hook_wysiwyg_editor_settings_alter() and overwriting the valid_elements setting with the configuration given to it. It does not touch extended_valid_elements, which may cause problems if the setttings generated by Wysiwyg or another module are incompatible. Settings for other editors are not touched.

Thanks for the clarification. Since the answer is to just upgrade, I will, but it's good to know I wasn't going completely crazy - and so I'll know where to look if I encounter something similar again :)