Bug: skin is lost through edit replace form

Testcase

1. Edit a topic with a skin (add ?skin=tomato to the url)
2. Hit add form & add a form
3. Back to edit, but this time no tomato skin

This has a major impact on me because I am using a skin to select the Kupu WYSIWYG editor, and if you change the form you get kicked back to the non-kupu skin, which is rather inelegant (though survivable in the medium term).

-- CrawfordCurrie - 13 May 2005

Crawford, just to make sure I understand...

You set a skin via a URL parameter to the edit script. Then you are trying to add a form, and when you come back from that detour the skin URL parameter is not there any more.

I think this is the intended behavior, e.g., the same happens when you go into preview and back (through cancel). However, I see how this can be annoying. I think we will need to do some heavy parameter passing for the skin variable to be preserved, though...

One could also argue, if you edit in a skin more, why shouldn't the various dialogs, such as oops screens or the add/change dialog, not appear in the same skin? But how far should this go?

-- ThomasWeigert - 13 May 2005

Some more experimentation shows that the situation is even worse. If you edit a topic with a skin being applied though the Url parameter, and you checkpoint that topic, it will loose the skin parameter.

I think a reasonable interpretation could be that as long as one is performing conceptually the same activity (e.g., view or edit) the skin parameter applied will be maintained. This would include

• for the view mode
  • switching to viewauth (e.g., editing a table using EditTablePlugin)
  • going to more screen
  • looking at earlier revisions
  • printable version
  • raw text
  • attachments
  • and similar
• for the edit mode
  • checkpoint
  • preview
  • adding form
  • changing form

However, this appears to be quite some work as one would have to systematically pass the skin parameter around.

Note that this scheme may also have disadvantages, unless one have your SkinSearchPath feature. In cairo, if one adds some skin that is specific to the view mode, for example, by providing view.potato.tmpl, but which is not available for any of the other templates, then it would end up falling back on the ClassicSkin when you transition to the other screens.

All that said, I do not know what the spec is for these situations.

-- ThomasWeigert - 14 May 2005

I think a key issue is that the user perception of what is happening here is not what is really going on. As you describe above, the user thinks of "coming back to the previuos edit screen after changing the form" where in reality that is a new edit screen.

-- ThomasWeigert - 14 May 2005

Correct. Meeting user expectations is paramount, IMHO.

This problem is a killer. The way I have done WYSIWYG editing is to select an alternate skin for the WYSIWYG editor, as against the standard skin which still uses a textarea edit. I want to show the form fields on the WYSIWYG editor screen, but I am forced to have the replaceform and addform buttons as well - they are hard coded into the form display. If I lose the skin, it means that any navigation away from the WYSIWYG pane will switch back to the standard textarea. I don't have an opportunity in the plugin to patch the links, even if I could trust them to be there and be recognisable, because they are never passed to the plugin. I can't override the template for displaying the form because there is no template. Passing on the skin is the only solution I can see (apart from patching Form.pm, which is rather extreme).

The edit screen uses a submit to save the topic when redirecting to change form. But change form uses view, which in turn uses %EDITURL% to redirect back to. EDITURL is set statically in TWiki.pm, without the skin parameter, even though it is present in the query.

Checkpoint is a different problem. In View.pm, it composes the new edit url and redirects to it.

Proposed fix for DakarRelease:

• Change Form.pm to use a skin-specific template for the "furniture" on the form display
• Include the skin parameter in EDITURL
• Pass the skin parameter through Checkpoint save

BTW further to what you were saying above, I have been wondering for some time if the 'skin=' URL parameter shouldn't just be a temporary extra addition to the skin path, rather than replacing it completely. What do you think?

-- CrawfordCurrie - 14 May 2005

Crawford, to your first point. I think the real problem is the way the form is hardwired. I have been staring at that code for some time now hoping to figure out a good way of cleaning it up. I have been wondering along the following lines

• provide a template for the rendering of the form that is included in the view and edit template
• move the form into a plugin (of course this requires metadata access from the plugin but that is really a big requirement anyways)

From your other suggestions I infer that you are not concerned about transitioning to a different skin when, e.g., moving to other temporary states.

To your final point, I think that your great SkinSearchPath idea offers many new opportunities and we are just starting to discover the possible applications. Your idea of treating the Url skin parameter as an addon rather than a replacement sounds good, but I really need to experiment to get my head around this new capability...

-- ThomasWeigert - 14 May 2005

I had though there should be specific required defs in the master template for the skin viz something like: %TMPL:DEF{FORM:header:has_form% ...Change form... %TMPL:END% and %TMPL:DEF{FORM:header:no_form% ...Add form... %TMPL:END% these can then simply be TMPL:P'd into the form.

A logical extension to this is to use the type naming from the form definition to define formats for each type: %TMPL:DEF{FORM:row:checkbox+buttons%<tr><td>$title</td><td>$value</td> ... %TMPL:END% See the way attachment tables are done in Attach.pm.

I want to stack the skin type onto the other skin types temporarily. This is to allow special screens defined by plugins to inherit the L&F while adding new functionality. Besides the WYSIWYG editor, another example is the 'edit action' screen on the action editor. This approach not only allows us to re-use other skin work done, but also lets us eliminate scripts in plugins, as work that was previously done in scripts can be done in the handlers instead.

I must keep reminding myself - this requires context information to be passed to the plugin, so it knows if it is being asked to render a template, the body text or an include.

-- CrawfordCurrie - 14 May 2005

Update; while working on DakarMergeModel I implemented an "alternative form" of Oops redirect that retains all the URL parameters from the current query and adds them, as hidden params, to the Oops page (it expands extralog=- caching topic in the oops template). If you specify "keep=>1" on the OopsException it will use this form of pseudo-redirect instead of an HTTP redirect (which loses all the params)

-- CrawfordCurrie - 01 Jun 2005

Reported as a bug, so it can be tracked - Bugs:Item46

-- CrawfordCurrie - 21 Jun 2005

Fixed in SVN 4442 - CC