01407: "Save and Edit" option ignores Replace On Edit ($ROEPatterns) (Open)

Summary: "Save and Edit" option ignores Replace On Edit ($ROEPatterns)
Created: 2017-05-03 11:36
Status: Open
Category: Bug
From: Peter Kay
Assigned:
Priority: 3
Version: 2.2.97
OS:

Description:

Problem: If you select "Save and Edit" from the edit form, Replace On Edit doesn't happen, and the user might suddenly see strange markup in the edit form.

Background: You can have a pair of ReplaceOnSave and ReplaceOnEdit patterns that write additional data to disk but then hide this data from users (to make it transparent to the user, for example).

Example: Set $ROSPatterns['/XXX/']='YYY'; $ROEPatterns['/YYY/']='XXX'; If you write a page with XXX in it and press "Save and Edit" suddenly you have "YYY".

Proposed solution: Add the following lines to pmwiki.php at line 1999:

        UpdatePage($pagename, $page, $new);
        Lock(0);
        if ($IsPagePosted && !@$_POST['postedit'])
          { Redirect(FmtPageName($EditRedirectFmt, $pagename)); return; }
 insert if ($IsPagePosted) {
 insert   global $ROEPatterns;
 insert   // restore ReplaceOnEdit after page was saved in UpdatePage
 insert   $new['text']=PPRA((array)@$ROEPatterns, $new['text']);
 insert }

Peter Kay May 03, 2017, at 11:37 AM

"Save and Edit" option doesn't ignore Replace On Edit ($ROEPatterns), it processes it before $ROSPatterns. So in your case where you want the same pattern to flip back and forward, unfortunately for you the order is not good. The simplest way to fix this is to first process $ROSPatterns, then $ROEPatterns in ReplaceOnSave(). I don't think there would be any negative side effect -- let me think about it. --Petko May 03, 2017, at 12:59 PM

Ah, for your precise case, if flipped, there will be no way to actually save the replacement in $ROSPatterns, as the other one will revert it. :-) --Petko May 03, 2017, at 01:04 PM

The order cannot be simply reversed, because templates need the ROE to happen before the ROS. Suppose we edit a new file from a template:
  • New file is editing from a template; HandleEdit is called
  • HandleEdit calls UpdatePage
    • EditTemplate loads template file
    • ReplaceOnSave processes ROEPatterns: any patterns in the template get shifted into "edit land", as if the user entered them by hand
    • ReplaceOnSave would process ROSPatterns if the file is being written (for example, a recipe that is copying a template file to a new file)
  • HandleEdit passes the page from UpdatePage to the user for editing.
In the case of Save and Edit:
  • HandleEdit is called with the user-edited page
    • ReplaceOnSave handles any ROEPatterns - an unnecessary step in this specific case because we're getting user data for the page?
    • ReplaceOnSave handles ROSPatterns because the page is being saved - the page data is now in "saved land", not "edit land"
  • HandleEdit gets the new page text back - it's still exactly what was written to disk.
  • Proposed change: if HandleEdit is going to continue editing (which it is in this case), ROEPatterns need to be processed again.
  • HandleEdit passes the text to the user to edit. Currently, it's still in "saved land."
It might be better to change the way ROEPatterns are handled entirely, but the fact that ReplaceOnSave() processes both ROEPatterns and then ROSPatterns is terribly useful for anyone writing template-type recipes.
Peter Kay May 18, 2017, at 07:57 PM

I'll think about adding this to the core, meanwhile in your recipe you can add an $EditFunctions[] entry to re-process the $ROEPatterns once again. As of today in subversion or from 2.2.98, there will be a refactored ReplaceOnSave function so you can call ProcessROESPatterns($new['text'], $ROEPatterns); to save you a few lines of code:

if (@$_POST['postedit']>'')
  $EditFunctions[] = 'ReplaceOnPostEdit';
if(! function_exists('ReplaceOnPostEdit')) {
  function ReplaceOnPostEdit($pagename,&$page,&$new) {
    global $ROEPatterns;
    $new['=preview'] = $new['text'] = ProcessROESPatterns($new['text'], $ROEPatterns);
    PCache($pagename, $new);
  }
}

If such a function is added to the core, it will probably be called 'ReplaceOnPostEdit', hence the conditional. --Petko May 21, 2017, at 01:45 AM