Item9269: Standard escapes are applied too early in FORMFIELD

Found when testing FORMFIELD:

You type

%FORMFIELD{"ReportedBy" format="$dollarvalue = $value"}%

you get:

$value = MichaelDaum

should be

$value = MichaelDaum

The $dollar must be processed after the current formatting loop has finished, not before. There might be other occurences in the code where this is wrong.

Can somebody confirm that expandStandardEscapes is wrong in Search.pm? Should be testable with a $dollarntopics=$ntopics . There are soem calls to expandStandardEscapes in Macros/SEARCH.pm as well. IMHO, all of these should be removed and placed at the very end of SEARCH.pm just before returning the result.

There's strange looking code in FOREACH.pm not using the vars $format, $header, $footer at all.

-- MichaelDaum - 07 Jul 2010

I'm a bit worried that you've commited a change to a really subtle area of foswiki rendering without any unit tests. I hope you've tested on 1.0 - and wonder what changed in the FORMFIELD code.

Search.pm does IIRC have unit tests that revealed corner cases wrt expandStandardEscapes - and I had to write those unit tests on 1.0, and then port them, to ensure we retained some of the more surprising ones - so I'd suggest the same approach should be done here.

I wrote unit tests for both your change and FOREACH (SEARCH uses the same macro) - the FOREACH code does the right thing already..

fixes only made for 1.1 - I have no idea if 1.0.9 is broken - though your report suggests it might.

-- SvenDowideit - 08 Jul 2010

At least FORMFIELD is broken on 1.0.9 as you can see in the example above. Reading the code around expandStandardEscapes I got the impression that some of it in SEARCH is wrong as it calls expandStandardEscapes before expanding things like ntopics and the like. Not sure if your unit tests cover these. But it should be rather easy to do something along the lines of $dollarntopics=$ntopics ...

-- MichaelDaum - 09 Jul 2010

fixes and unit tests made for 1.1 - it'll be alot easier for you if you click on the Foswikirev links below to see what was actually done.

-- SvenDowideit - 13 Jul 2010

Sorry, but the thing is not fixed as below tests indicate. This is what was to be expected looking at a rather limitted range of code where expandStandardEscapes() is used.

Error in zerroresults (trunk only)

%SEARCH{"does not matc[h]" 
  type="regex" 
  zeroresults="$dollarweb=$web" 
  format="dummy" 
}%

Error in header and footer (trunk and stable)

%SEARCH{".*" 
  limit="1" 
  type="regex" 
  nonoise="on"
  header="header: $dollarweb=$web%BR%" 
  format="format: $dollarweb=$web%BR%" 
  footer="footer: $dollarweb=$web" 
}%

Error in pagerformat (trunk only)

%SEARCH{".*"
  type="regex"
  nonoise="on"
  format="   1 $topic"
  pager="on"
  pagesize="10"
  pagerformat="pagerformat: $dollarweb=$web"
}%

Each of these tests shows that $dollarweb expands to the web of the hit as well as $web does ... which is wrong. In general it should only require a single call to expandStandardEscapes() at the very end of the code just before returning the result to the parser. Theoretically, the parser should have been designed to expand standard escapes and not require the caller to do the job.

-- MichaelDaum - 19 Jul 2010

This might be why trunk Item9464 is broken and stable Item9464 is not.

-- PaulHarvey - 13 Aug 2010

changes commited, and they don't seem to help trunk Item9464, who's crash we should treat as a final release blocker for 1.1

-- SvenDowideit - 19 Aug 2010