(r30) AddNatEditToCore < Development

You are here: Foswiki>Development Web>ReleaseProcess>FeatureRequest>AddNatEditToCore (revision 30) (raw view)
---+!! Feature Proposal: Let's add !NatEdit to the core

%TOC%



---++ Motivation

[[Extensions.NatEditPlugin][NatEdit]] is:
   * pretty cool
   * simple
   * popular with the mostly-normal-but-very-slightly-geeky person
   * attractively packaged (it looks nice)
   * has a proven track record of usage in many sites
   * integrates well with pattern skin (has a consistent look and feel)
   * integrates well with tinymce (well, doesn't stomp on it anyway)
   * is a natural bridge between the raw textarea and WYSIWYG
We have been resistant to previous proposals to incorporate !NatEdit in the core in the past because of a perception that it carried a lot of baggage. A look at the dependencies list is still quite scary:
| Name  |  Version |  Description |
| Foswiki::Plugins::JQueryPlugin |  &gt;=3740 |  Required. |
| Foswiki::Plugins::SetVariablePlugin |  &gt;=4287 |  Required. |
| Foswiki::Plugins::UploadPlugin |  &gt;=1340 |  Required. |
| Foswiki::Plugins::RenderPlugin |  &gt;=3644 |  Required. |
| Foswiki::Plugins::DBCachePlugin |  &gt;=4358 |  Required. |
| Foswiki::Plugins::ZonePlugin |  &gt;=1.0 |  Required. |
I have a site I want !NatEdit on, but I didn't want all the baggage; so I thought I'd cut it down to just the core dependencies (i.e. just JQueryPlugin in 1.1) to see what happened. The answer? Not a lot. Fifteen minutes editing templates later, I had an (almost) fully functional !NatEdit without the extra dependencies. Also, my small edits *do not compromise the functionality* of !NatEdit. All the extra features come back *when the extra plugins are installed*. I just switched off certain features in the templates when they are absent.

So, I'm thinking, this is a really low hanging fruit. I know Michael has plans to replace natedit with a better code editor, but TBH, I'm doubtful of the value of that. NatEdit is there, it works, it's proven. We should just bring it in to the core. It even has some tests, for goodness sakes!

Even if we don't add it to the core, I still think the patch should be applied, as it makes it much easier for people to adopt.

---++ Description and Documentation
   * Apply the attached patch to NatEditPlugin
   * perltidy it
   * add it to the core dependencies
   * change the default skin to natedit,pattern
   * clean up the documentation (it's a bit germanic)
*NOTE* the patch is bigger than it needs to be because I had to remove the inline HTML comments from the templates. This is because they violate the 1.1 "no content after /html" constraint.

---++ Screenshots

%IMAGE{"Support/Faq23/NatEditSnap5.png" size="250"}%
%IMAGE{"Development/MergeWYSIWYGEditors/NatEditSnap1.png" size="250"}%
%IMAGE{"Development/AccessControlQuery/NatEditSnap1.png" size="250"}%
%IMAGE{"WireframesEditScreenInspirations/NatEditSnap1.png" size="250"}%
%IMAGE{"WireframesEditScreenInspirations/NatEditSnap4.png" size="250"}%
%IMAGE{"WireframesEditScreenInspirations/NatEditSnap6.jpeg" size="250"}%
%IMAGE{"WireframesEditScreenInspirations/NatEditSnap7.jpeg" size="250"}%
%IMAGE{"WireframesEditScreenInspirations/NatEditSnap8.jpeg" size="250"}%
%IMAGE{"WireframesEditScreenInspirations/NatEditSnap9.jpeg" size="250"}%
%IMAGE{"WireframesEditScreenInspirations/NatEditSnap10.jpeg" size="250"}%
%IMAGE{"WireframesEditScreenInspirations/NatEditSnap11.jpeg" size="250"}%
%IMAGE{"WireframesEditScreenInspirations/NatEditSnap12.jpeg" size="250"}%
%IMAGE{"WireframesEditScreenInspirations/NatEditSnap13.jpeg" size="250"}%
%CLEAR%

-- __Contributors:__ Main.CrawfordCurrie, Main.MichaelDaum - 28 Jun 2010

---++ Discussion

Hard to see which part of the patch is adding conditionals and which is removign html comments. Most of the comments that you removed are there for good reasons: to prevent empty lines that are converted th &lt;p />s later on. These better should stay in.

The attachment upload plugin is bound to change quite a lot as the new UploadPlugin will be based on plupload. I am still about to rework the upload workflow (a) from within natedit as well as (b)  managing attachments outside the edit interface. Carlo did some rather
cool wireframes that I still have to digest more closely. 

-- Main.MichaelDaum - 28 Jun 2010

I put the comments back so you can see the changes more clearly (though they still have to be removed for it to work in 1.1)

The points about the upload are fair enough - and I imagine this won't be the last time it changes, either. But the pace of change is incredibly slow, so having this plugin in core shouldn't make that much difference.

-- Main.CrawfordCurrie - 28 Jun 2010

True, pace is low. I am all for adding it to the core, not only because I am using it every day, installing it on every client project. The most important point for me is that it standardizes the interface of wiki apps implementing custom EDIT_TEMPLATEs.

The latest patch w/o the removed html comments looks good. Will add it to the next NatEditPlugin release.

-- Main.MichaelDaum - 28 Jun 2010

I re-patched with the excess comments replaced with %{ }% comments, which is a better way of eliminating newlines and keeps 1.1 happy.

-- Main.CrawfordCurrie - 28 Jun 2010

Main question I have wrt moving it into the core for foswiki 2.0 - is can we spend some time coalessing the API's with the TinyMCEPlugin (not by moving Nat _to_ use Tiny's but in some logical way, so that we can share some functionalities?

-- Main.SvenDowideit - 28 Jun 2010

Yes, I have some ideas on this. There's a task somewhere I wrote ages ago to improve the way NatEdit + TMCE co-exist; it's a bit messy right now - you will notice as natedit fires up the wysiwyg editor jumps all over the place.

In that task I said we needed a plugin inside TMCE to provide some event hooks for natedit to latch on to, and I think I'm in a position to do that.

As for attach dialogues and so-on - shall investigate.

-- Main.PaulHarvey - 29 Jun 2010

I assume you are talking 2.0

It would be a major risk and setback for 1.1. Such a large UI change is ALWAYS connected with a lot of surprising problems and bug fixing.

I like the idea for 2.0.

-- Main.KennethLavrsen - 29 Jun 2010

I am a bit confused there.

Is this proposal about deciding that 2.0 would ship without a WYSIWYG editor activated by default when you press the "Edit" button in a vanilla Foswiki installation ?

-- Main.RaulFRodriguez - 29 Jun 2010

This proposal is about replacing the current raw edit with natedit in %FORMFIELD{"PlannedFor"}%.

-- Main.MichaelDaum - 29 Jun 2010

OK, thanks for clarifying. That's fine with me then.

-- Main.RaulFRodriguez - 29 Jun 2010

My first thought was that this would be for 2.0, but after bouncing up and down on it (quite vigorously) for the last couple of days, I haven't found a single problem. I'm starting to think we should consider this for 1.1. Yes, I know the risks, but I will point out that it's been in mainstream use for a long, long time already.

-- Main.CrawfordCurrie - 29 Jun 2010

The !NatEdit stuff may have evolved since I last looked at it.

But I remember several issues

The good thing first: Yes the !NatEdit as such is agreat help for the users as it provides the most common formatting when you edit the raw wiki markup.

But I remember these issues

   * The design of the edit page gave huge problems on netbooks. On my 1024x600 point machine I did not have many lines to edit the content. It was so bad that I had to constantly copy the content to my editor and paste it back. The editor had the issue that it did not allow resizing the edit window to most of the screen height and then scroll the whole window down. You can do this with the normal raw editor. Is this fixed now?
   * The editor I remember did not allow viewing the DataForm and the topic content in the same screen. You had to click tabs to move between editing the topic and editing the form. I have several applications now where I have a special view and edit template and where the usability when editing requires that the form is visible simultaneous with the edit screen (often with the form at the top)

It is not a trivial change. It is not just to change over and all is fine. We will run into both problems and discussions. I do not support such a dramatic change now. It needs time to get it implemented.

Are my issues from more than a year ago even addressed now?

I have no doubt the issues can be resolved. My worry is that is will take time and it will take away the attention needed to close the remaining 150+ bugs plus all the testing we have only just started doing. We are already 2 months delayed and have not even reached the stability where we can form the release branch.

-- Main.KennethLavrsen - 29 Jun 2010

If the plugin is enabled but the skin pref is not set, will the raw edit function works as usual? If so, I think it would be a good idea to patch NatEditPlugin, tidy it, add it to the core and document the fact that you can change the skin pref to have access to it.

That way we can raise admins  awareness on the plugin, perhaps bringing valuable feedback for things in 2.0. I think that would be a smooth migration path.

-- Main.RafaelAlvarez - 30 Jun 2010

I am against rushing it into 1.1 that we are close to finish. 1.1 is in feature freeze. That says it all.

Kenneth, give it a try again. Textareas won't shrink beyond a configurable min height. Putting the form on a separate tab like Preferences and Settings as well is a design decision to eliminate the need for vertical scrolling. People have repeatedly reported it to be a lot easier to use.

See this [[http://screencast.com/t/HGHdb9JK][screencast]]

Btw. here are previous discussions to add natedit to the core: PreInstallNatEditContrib, MergeWYSIWYGEditors

I've added a few screenshots above. Some of them show customizations for specific !TopicTypes to give you an impression of what kind of wiki apps have been done based on !NatEdit.

One strong reason against !NatEdit is that plans for a new skin as outlined in WireframesEditScreen significantly differ from what !NatEdit looks like today. 

-- Main.MichaelDaum - 30 Jun 2010

Caveat: some of Micha's example screens make use of the dependencies that I deliberately defused in the patch, so they are not WYSIWYG.

I don't see the point in including it disabled. Though I could see the point in making it easy to *dis* able if people found it didn't work for their local apps.

It seems a shame not to leverage this opportunity just because a few people might have problems, especially as those problems are so easy to resolve. If you've customised your view templates, you are expert enough to turn off natedit.

I'm not holding my breath regarding the WireframesEditScreen, as the development seems to have stalled. This is a low-hanging fruit that is ripe for picking. Dismissing it, even at this late stage, feels churlish.

-- Main.CrawfordCurrie - 30 Jun 2010

Michael. Your work is fantastic and I am all for making this a core feature. The issues that have to be resolved is a matter of adding some preferences so that an application that depends on the form and text field being visible on the same page can still be implented without loosing the nat edit features can still be implemented.

But it does not end there. By looking at the screenshots I am both impressed AND forming new questions.

I have seen enough to know that it will take some significant time to get this integrated and the issues ironed out. It will take months. I have seen much smaller features added to the pattern skin delaying a release for months.

We are on a feature freeze. We are supposed to iron out bugs, not to add features.

We cannot rush this into 1.1. And if we postpone 1.1 another 3-6 months I think I will personally will start loosing my interest in the whole thing. 1.0 is 1.5 years old now. We need a new release and get all the work already done out and used.

Please let is continue this with a 1.2 or 2.0 scope. Maybe it is time to branch now so this can happen in trunk while we finish 1.1 in a !Release01x01 branch.

Then we can all take the time to install the thing and get a kick-ass product out of it. I support this feature 100% but I want us all to have the time it takes to get it right so we ALL are happy with it.

-- Main.KennethLavrsen - 30 Jun 2010

I agree that we shouldn't do this in 1.1. I already have users on my "production trunk" site because I have built wiki apps depending on trunk features. And at least one IRC user that has migrated from TWiki 5 to the trunk nightlies. 

I use NatEditPlugin on my site - mostly for the settings/permissions tab functionality. Agree that NatEditPlugin would be a nice stop-gap until we get momentum going again on the 2.0 skin stuff.

Is there a reason why we can't do a 1.2 soon after getting 1.1 out the door?

-- Main.PaulHarvey - 01 Jul 2010

Actually, that's the best idea yet. Let's get 1.1 out the way, then do a 1.2 feature-focused on !NatEdit. There's no reason to get hung up on 2.0 being the next release.

-- Main.CrawfordCurrie - 01 Jul 2010

Good compromise. Let us let the 14-day window run. With the scope agreed as 1.2 I remove my concern. And if noone else objects to this it can get accepted by concensus by Monday 12 July 2010.

I look forward to helping out as a tester on this task.

-- Main.KennethLavrsen - 01 Jul 2010

The latest version of the NatEditPlugin has made the RenderPlugin an unconditional dependency.

The RenderPlugin is pretty simple; however there are a number of issues that discourage it's inclusion in the core:
   1 The plugin publishes rest handlers with no authentication of permissions constraints. They are therefore callable by any client, logged in or not (unless =rest= is in =AuthScripts=, of course, which is not recommended practice)
   1 Some parameters are not validated, opening the door to using it for XSS
   1 There are no unit tests
I have reviewed the code quite carefully, and I *think* the plugin is harmless, as it doesn't do anything that you can't do using a standard view request. I'm slightly concerned about the lack of validation, especially of the template name. Also, because the =tag=  and =expand= handlers can be used to expand an arbitrary tag, it can be trivially used to mount a DoS attack against the server.

I am currently looking at the code with a view to correcting these problems. The NatEditPlugin *only* uses the =template= handler from the RenderPlugin, so the best approach may be to copy that handler into the NatEditPlugin and leave the RenderPlugin outside the core.

Later: so this is how far I got:
   1 Copy the restTemplate handler into NatEditPlugin.pm
   1 Add parameter validation to the handler
   1 Modify the templates to invoke the handler
   1 perltidy it all
   1 Set the skin to natedit,pattern

and .... it doesn't work. The edit window is a mess: no Nat controls, buttons all on separate lines. No javascript errors, no errors in logs. All on trunk. Giving up for tonight.

-- Main.CrawfordCurrie - 13 Mar 2012

What are you doing here?

Please don't move the rest handlers into the NatEditPlugin. That's totally wrong.

The current way dialogs are loaded are in the process of being rewritten replacing jquery.simplemodal with jquery.ui. So these parts will look completely different anyway.

-- Main.MichaelDaum - 13 Mar 2012

We cannot have the current RenderPlugin in the core; I am absolutely opposed to that, as the -tag= and =expand= handlers are far too vulnerable. If you can switch these handlers off under configuration control I might be persuaded, but as it is they are far too dangerous. Copying the template handler - a few lines of code - into NatEditPlugin is the lesser of the two evils, though I agree, code duplication is nasty.

-- Main.CrawfordCurrie - 14 Mar 2012

NatEditPlugin will have to remain outside of the core distribution as long as it depends on RenderPlugin. I am by no means planning to also have RenderPlugin in the core, though I'd appreciate any improvements to it. 

-- Main.MichaelDaum - 14 Mar 2012

Right, which is why I moved that one (safe) REST handler into NatEditPlugin. I don't see that as a big deal, as it's a pretty trivial handler. If we copy the handler we can go ahead with putting natEdit into core. However to do that, we need to solve the rendering problems (I tried again, but I don't know the code well enough to work out what is going wrong)

-- Main.CrawfordCurrie - 15 Mar 2012

You most probably f*ed up css and js files in pub/System/NatEditPlugin.

No, I don't want the rest handler to be copy-pasted from RenderPlugin into NatEditPlugin. One possible solution to that would be to move the dialogs into topic space (maybe calling TMPL:P from there) and then use good old =view= using via ajax. That's more complicated than it should, but better than having YARH (yet another rest handler).

In any case all modal dialogs will be ported from jquery.simplemodal to jquery.ui.dialog.

-- Main.MichaelDaum - 15 Mar 2012

Possibly; I had to change the order of the includes in =edit.natedit.tmpl=, because they were including HTML after =</html>= which fails validation and won't run with FOSWIKI_ASSERTS enabled. That was the only signficant change, however (apart from the rest handler paths)

OK, so by reverting back to trunk and removing the unconditional dependency on RenderPlugin, I find that NatEdit now works. So the dependency is *not* unconditional :-/ On a bit more research I find that the dialogs are loaded in the DOM and the rest handler is never called.

-- Main.CrawfordCurrie - 15 Mar 2012
ChangeProposalForm edit

TopicSummary	Let's add NatEdit to the core
CurrentState	AcceptedProposal
CommittedDeveloper	CrawfordCurrie, MichaelDaum
ReasonForDecision	ConsensusReached
DateOfCommitment	28 Jul 2010
ConcernRaisedBy	MichaelDaum
BugTracking	Item11648
RelatedTopics	WysiwygTaskTeam
PlannedFor	1.2
Topic attachments
I	Attachment	Action	Size	Date	Who
png	NatEditSnap1.png	manage	77.5 K	30 Jun 2010 - 08:01	MichaelDaum
png	NatEditSnap4.png	manage	42.8 K	30 Jun 2010 - 08:02	MichaelDaum
diff	patch.diff	manage	7.5 K	28 Jun 2010 - 16:38	CrawfordCurrie
Topic revision: r30 - 15 Mar 2012, CrawfordCurrie
Development
Quick Links

Tools

The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. see CopyrightStatement.