I make this topic so it is not forgotten and has a place holder.

If Kupu strips out all HTML comments when saving in IE (which 99% of the typical business users use and 80-90% of normal users use) it will mean that at huge percentage of topics will be destroyed by edited and saved in WYSIWYG mode

The reason is that hiding setting TWikiVariables within HTML comments is very common. Many TWiki Applications take advantage of doing smart things hidden within comments. Plugins such as HolidaylistPlugin often has the bulleted list setting the actual holidays within HTML comments.

Access rights (ALLOWTOPICCHANGE or ALLOWTOPICVIEW) for example are almost always put inside HTML comments. So after an edit and a save with WYSIWYG the access rights are gone and you won't even notice.

The examples are endless.

There is no way that Dakar can be released with WYSIWYG bundled with it. Not even as a beta. Not even as an alpha. As it works now it is not only useless. It is dangerous.

It does not help trying to write your way out of it in the plugin topic. If people enable the plugin in configure, the WYSIWYG button is there and people will click on it and save no matter what you write.

It has previous worked in IE. I raised a bug Item1259 and in SVN 8091 a fix was implemented. Since then something has been changed which completely has broken handling of comments in IE.

If you are not able to fix this issue - do not bundle WysiwygPlugin with Dakar! It should not even be on Twiki.org in its present state.

KJL

It is stated numerous times that WYSIWYG is not for HTML, only for TML. Make global %HTMLCOMMENTSTART/STOP% variables that will survive editing, make variables for any <div> tricks you might use, etc etc and enable automatic disable of the wysiwyg for any pages with HTML (including comments).

And then: File the bug reports that are left for TML, so it'll work close to 100% for that - I think we are getting close.

It does not matter what you call the plugin and what you write about it. If there is a WYSIWYG button people can click on they will and deleteting standard comments in existing topics is a disaster. Just look at some of the latest comments on TWiki:Plugins.WysiwygPluginDev and you can see the first reactions from users. KJL

-- SP

Note that the plugin is actually innocent of the charges. It is Kupu that strips the comments. I know it is difficult to differentiate, but the difference is actually very important when it comes to finding fixes. I do not modify the Kupu 1.3.2 release shipped with the plugin, only override those parts of the functionality that can be overridden.

In fact, the detail is (I think) that the Kupu XHTML validator strips comments.

I thought I had a solution for this, which is to disable the validator, but it appears that IE depends on the validator to generate serialisable output.

I will continue to study this problem and try to identify a fix. I am hoping it is not intractable. If the consensus is that this is serious enough to prevent release with Dakar, then that would be a shame.

CC

I sure hope other agree with with me that a plugin that presents people with a WYSIWYG button which will delete all HTML comments and destroy hundreds of topics is not something you want to jeopardize the overall impression of TWiki with. As I just wrote in Steffens comments above - you already have the first negative reactions from your users on TWiki:Plugins.WysiwygPluginDev. It is serious business to ship something that broken - even if you call it beta. My guess is that 90% of the TWiki Dakars that get installed the next weeks will be upgrades from Cairo or earlier and they will have 100s or 1000s or 10000s of topics with important TWiki Variables such as access control settings between HTML comments and one little WYSIWIG save cycle - even without typing anything will wipe out this important information. People do not write "Hello Mom" between HTML comments. It is important stuff that is put there. And writing a warning in the plugin topic is no good. The admin that installs the upgrade will be so busy reading hundreds of pages of information that he may very well not see this. And 99% of the users never will. They don't even know the plugin topic exists.

Now that all this is said - there may be light at the end of the tunnel.

There is a posting done on the 29th of January on TWiki:Plugins.WysiwygPlugin

Someone has patched a javascript file called sarissa.js. I assume this is part of kupu.

I have tested the patch very briefly by manually installing the 26 Jan 2006 version of the plugin and overwriting it with the patched sarissa.js

And it works. It no longer makes the undefinedundefined error and it preserves the comments.

I propose we release the beta plugin with this patched kupu file.

KJL

See relevant patch in TWiki:Support.WYSIWYGPluginDeletesAllTopicContentWhenSaving

-- PTh

It is the same fix by the same person that I write about just above.

I have made a patch (attached) which...

• Reverts CC's SVN 8572 changes (the ones that makes IE not preserve comments)
• Implements Mike Moretti's patch to the Kupu javascript file sarissa.js which caused IE to delete the entire topic and replacing it with "undefinedundefined".

Crawford did not commit additional Wysiwyg changes after 8572 so the patch takes us back to the last good (except for the undefinedundefined problem in IE). And Mike Moretti's fix takes care of the undefined problem so everything works again.

With the attached fix committed y oppinion is that Wysiwyg Plugin can be included in TWiki 4.0.0

KJL

With CC on vacation, we'll have to take your word for it - Don't like changing kupu though, but perhaps it is the only way.

SVN 8590.

-- SP

I contributed a patch on TWiki:Support.WYSIWYGPluginDeletesAllTopicContentWhenSaving, exclude-comments-pre.patch to exclude page with comments from kupu editing. I do not know if it is a better solution (I think no), just to contribute possible options.

-- CN

Colas. I do not think it is a good solution to disable the plugin when there are comments in a topic. That means that the plugin does not work at all on 50-80% of our topics.

The patch that Mike Moretti did maintains the comments - even with TWiki variables hidden inside the comments - so I think it is not a bad solution.

Naturally it calls for thoughts to change kupu - but when there is an obvious bug in kupu - it is the right thing to do. We just have to make sure the patch is fed back to the kupu team so that the fix will be there when next version of kupu is released.

After SVN 8590 (thanks Steffen) I can open a quite complexe ISO9000 document in WYSIWYG and save it and the advanced features inside HTML comments were preserved.

KJL

I agree it is not a good solution. It is a stopgap measure useful for people like us who are going to make the plugin available to users shortly (removing comments would be extremely serious, as most access control has been put by users inside html comments...)

-- CN

What Gap are you stopping? SVN8590 fixes the comment problem. Comments are preserved with the SVN8590 fix. I am just worried that someone commits this stop gap solution now which would make the plugin close to useless. Update your TWiki from SVN and give it a try. it is not bad now.

KJL

Colas' suggestion is a purely optional setting (which is disabled by default). As far as I've understood things, other browsers we haven't had feedback on might have problems (Safari, etc); no harm done by allowing a conservative setting for those who prefer it that way. I'll be needing the pre setting myself in my environment, so I'm happy with this patch.

KJL, who is following up on the kupu roundtrip for the patch?

SVN 8592.

-- SP

Sorry. I had not understood that Colas patch was a configurable setting that you had to enable in the plugin topic. I had understood it as a hardcoded feature that would prevent the plugin from working on all topics with comments.

Then it all makes very good sense to include it.

I will see if I can feed the kupu patch back to the kupu team.

KJL

I don't think this is adequate, so I'm re-opening it. When testing on IE I discovered that comments would disappear when I flicked between wysiwyg and source modes, or if I modified a comment in source mode. Did anyone think of checking if there was a more recent version of Sarissa available? It doesn't come from the Kupu team, it's from somewhere else.

Otherwise, the alternative fix which does not involve changing any Kupu code (which I strongly prefer), and I know works in all cases, is to change comments to SPANs. This results in comments being visible in Wysiwyg mode as italic text with a blue background. To re-enable comment translation to spans, do this:

Revert the changes since my last checkin

Modify the comment hoisting code in TML2HTML to this:

    $text =~ s/<!--(.*?)-->/$this->_liftOut(CGI::span({class=>'TMLcomment'},
       HTML::Entities::encode_entities($1)))/ges;

Modify HTML2TML/Node.pm method _handleSPAN, TMLcomment handling, to this:

    } elsif( defined( $this->{attrs}->{class} ) &&
               $this->{attrs}->{class} =~ /\bTMLcomment\b/ ) {
        my( $flags, $text ) = $this->_flatKids( $options | $WC::NO_BLOCK_TML );
        return (0, '<!--'.HTML::Entities::decode_entities($text).'-->' );

Comments will now appear in text, and will be editable. Note that entity-conversion of \n may also be necessary.

Remove the remark about the handling of comments in IE from the doc.

CC

Actually, I just checked the latest code at sarissa:

http://cvs.sourceforge.net/viewcvs.py/sarissa/sarissa/sarissa.js?view=markup (http://sourceforge.net/projects/sarissa)

It appears they did pretty much the same thing I did to fix this problem. It was marked as a TODO in the code we have in TWiki, and it looks like they "did" it in the latest version which we don't have. I don't know how hard/easy it would be to plug a new version of sarissa into kupu and I don't have the resources right now to try that kind of thing...

Also, I tried out your "comment" problem, switching between source mode and wysiwyg on IE 6 and Firefox with just my patch in (and no other changes) and I didn't see any comments disappearing, they just got moved up and jammed onto the previous line. There's weird sorts of paragraphing/line moving going on but that's it.

-- TWiki:Main.MikeMoretti

We are too late in the game to upgrade to the latest Kupu.

Could anyone confirm if the latest Plugin version works as expected?

-- PTh

CC. The patch I attached and that was committed did revert you latest changes so that part is already covered.

What is there now is Mike's patched sarissa and the version of the plugin just before your last commit.

I also noticed that the plugin snips off the newline before a comment but only if the comment comes right after a line of text. If there is an empty line it does not. In most cases this does not mean anything. And at least everything within the comments is preserved. When it means something it can be fixed by adding a new line in normal mode. No information is lost.

I would recommend that the plugin stays as it is when we release in only few hours from now.

Remember that this is a plugin. We can update the plugin topic on twiki.org updating the plugin zip file in a few days when you are back CC and we have had the chance to both test the latest sarissa.js and maybe also your suggested change.

KJL

As a side note, the latest kupu doesn't include the latest sarissa; it still has the problem. That's the first place I looked before making the fix. When I found out that kupu still had the problem, I fixed it in TWiki alone. Also, when I did that, I didn't know sarissa was a separate toolkit altogether. Eventually, in order to not use a special hacked-up patch by me we'd have to somehow integrate the latest sarissa into the latest kupu and then the latest kupu into TWiki. It may be quite some amount of work to do this and get stuff submitted to the kupu folks, etc and wait for the turnaround and then reintegrate back into TWiki.

-MM

So can we close this item?

-- PTh

I expended a huge amount of effort to try and make sure that we didn't ship with anything other than an off-the-shelf Kupu. It is frustrating that this one small change to Sarissa invalidates all that hard work. However it is pragmatic, and if it is established to work, and followed up with the Kupu team, I can hardly argue. We need to push the Kupu team to integrate the latest Sarissa.

CC