Item13484: Changing form to none - causing lost topic content

How to repo

• go to http://trunk.foswiki.org/Sandbox/Úňíčôďě/UniTestFormTopic1
• click edit AT THE FORM (e.g. not topic edit, nor edit wiki text)
• change the form to "none"
• the edit will show NO TEXT

Don't save the topic, just cancel to not lost the test.

-- JozefMojzis - 29 Jun 2015

Actually fails on any task in the task web. Just edit form and remove it, and the topic text gets removed as well.

Doesn't fail on 1.1.9. A new blocker. RC is a no-go.

-- GeorgeClark - 29 Jun 2015

Here is what is going on.

Foswiki 1.1.9

• (edit form)
  • Get Item13484?t=1234123
• (change form)
  • POST bin/save/Tasks/Item13484
  • URL param text contains "the raw topic text"
• (edit)
  • POST bin/edit/Tasks/Item13484
  • URL param text contains "the raw topic text"

Foswiki 1.2

• (edit form)
  • Get Item13484?t=1234123
• (change form)
  • POST bin/save/ (no web/Topic in URL)
  • text url param is not provided
• (edit)
  • POST bin/edit/Tasks/Item13484
  • text url param is empty.

-- GeorgeClark - 29 Jun 2015

Confirmed, This appears to be a NatEdit issue. Changed the SKIN setting for my user to famfamfam,pattern and the problem is resolved. MichaelDaum ... over to you.

-- GeorgeClark - 29 Jun 2015

I've fixed it but definitely needs validation. When UI::ChangeForm runs, it can set a alternate action, if it knows that it's a form edit and not a topic edit. Nothing was setting the "editaction" url param, so it always bounces the user into edit. Setting an editaction=form when editing a form seems to avoid the text loss. It's not an issue for the standard editor, because it always posts the topic text in a hidden field.

-- GeorgeClark - 29 Jun 2015

Analysis

This is quite a mess here. In the templates and UI code we have action but also editaction. Both are trying to address the same thing. One should go.

(1) lib/Foswiki/UI/ChangeForm.pm has got an undocumented pseudo macro called %EDITACTION% which expands to

<input type="hidden" name="action" value="$editaction" />

The perl value of $editaction depends on the call to generate()

Input value and macro are called "editaction" however form field is called "action" ... Mismatch!

(2) lib/Foswiki/UI/Save.pm reads a URL parameter called editaction which is then used to call generate() (see above) ... which in turn generates a hidden form field called with name action ... Mismatch!

The same value of the URL parameter editaction is used to initialize the checkpoint code path ... and generates a url containing an "action" parameter Mismatch!

(3) lib/Foswiki/UI/Edit.pm reads an url parameter called action and stores it into a perl variable called $editaction Mismatch!

The code in Edit.pm is fine besides the variable being named in a way adding to the confusion.

(4) templates/viewtopicactionbuttons.pm tests for a macro %EDITACTION% expanding to something non-empty ... and then sets an url parameter called action Mismatch!

The code to do so looks like this:

%IF{"defined EDITACTION" then=";action=%EDITACTION" else=""}%

This is always returning an empty string as %EDITACTION% isn't a proper macro. It is inserted to the templates using text-substitution in ChangeForm.pm

(5) templates/changeform.tmpl uses %EDITACTION% ... no obvious error here. This is actually the only template where %EDITACTION% as a "macro" makes sense. Actually %EDITACTION% should be removed all together.

(6) templates/edittext.tmpl sets a hidden form field "editaction" to "text" ... correct in principle. Should be renamed to "action" however.

(7) templates/edit.tmpl and templates/edit.nat.tmpl forward the URL parameter action into a form field of the same name ... This is how it is supposed to be.

Clicking on "Change Form" will send this action form field to the change-form dialog. However changeform.tmpl does not insert this value back into the html form it creates. Instead it expects yet another URL parameter called editaction which is then processed by the backend ChangeForm.pm as part of this %EDITACTION% pseudo-macro. This is where the real error reported here happens. No big surprise given all the mismatches in the code and the templates.

As a consequence, after selecting a new form, will the original action URL parameter posted when clicking on "Change Form" be lost ... and the editor is not set back into "Edit Form" mode thus nullifying the text value of the topic.

(8) templates/formtables.tmpl creates an edit URL to edit the data form only using the URL parameter action ... This is how it is supposed to be.

Bottom line: The rename, manage and register scripts (and probably some more) use an URL parameter action as well. So standardizing on action makes sense while editaction and %EDITACTION should simply go away in the save script and its code paths. We need to remove editaction and %EDITACTION% and replaced with action. In most cases templates are already using the action URL parameter and properly forward it initializing a hidden form field with it. Better not adding yet another hidden form field called editaction holding the same value.

-- MichaelDaum - 30 Jun 2015

I just read CommandAndCGIScripts and I'm not convinced (1), (2) and (6) are correct.

According to CommandAndCGIScripts, editaction is "passed on to the edit script in the action parameter", so it's not really an action for the Save script, it's an action that is passed on to the edit script. save doesn't have a documented action paramater but I see one in the code:

    # the 'action' parameter has been deprecated, though is still available
    # for compatibility with old templates.
    if ( !$saveaction && $query->param('action') ) {

save supports a bunch of action_* parameters - I vaguely recall adding these to avoid conflicts with the existing action.

My take on it:

1. (1), (2) and (6) analysis is wrong
2. (3), (5), (7) and (8) I agree, though have not analysed in detail
3. (4) definitely needs careful analysis
4. action ought to be re-instated and the action_* parameters deprecated, though it's debatable this is needed for 1.2.0
5. editaction should be more clearly documented, as it's function is unclear

-- CrawfordCurrie - 30 Jun 2015

How could (1,2, 6) be wrong? This is what the current code says reading it. The mismatch between action and editaction is there. It should not as this is redundancy and mismatching while being read from the URL, entered into the form and then being processed in the core, this all hopping between calling it action or editaction. There is no further details required to analyze for (3). This is simply a renaming of one variable from $editaction to $action just to remove the mismatch.

-- MichaelDaum - 30 Jun 2015

Commenting is disabled while not logged in