Item13962: HolidaylistPlugin breaks CommentPlugin if used on same page

We use HolidaylistPlugin in many departments.

And the way we use it is to show the calendar

And below we have the list that people enter but instead of having to know the exact syntax we have a COMMENT tag with a user defined set of fields so you can easily enter your start and end date and a comment.

Even if I reduce it all down to a default HOLIDAYLIST macro followed by a default COMMENT tag like this

%HOLIDAYLIST%

That is it. All you need to do to reproduce it is install HolidaylistPlugin and make a test page with the two macros above

And you will see the COMMENT tag does not save anything.

If you swap the two macros so COMMENT comes before HOLIDAYLIST then the comments get saved. I have tried to find the root cause for 2 hours now. I cannot figure out what it is.

I have tried to disable AJAX comments. That makes no difference

-- KennethLavrsen - 12 Feb 2016

See the IRC Logs for discussion, but the issue comes down to a serious "reuse" issue within the core design of Foswiki, and the implementation of the HOLIDAYLIST macro, and the CommentPlugin, and SpreadSheetPlugin, and anything else that uses a variable to store data between multiple macro expansions. In this case, each time a COMMENT macro is expanded, it increments an "index" of the form in the page, so that the post of the form can be attributed back to the specific macro.

The HOLIDAYLIST macro finds the lists of holidays by building a string of INCLUDE macros, one per each place identified as holding holidays. That includes the topic containing the HOLIDAYLIST macro that is currently being expanded.

 %INCLUDE{"web.topic"}% %INCLUDE{"web.anothertopic"}% ..." 

That text is then expanded by calling expandCommonVariables, the results scanned for holiday definitions, and the the results are discarded. But because the text contains COMMENT macros, they are also expanded, discarded, but increment the form index resulting in a mismatch.

The problem is that macros like %COMMENT{}% or %CALCULATE{}% save state. COMMENT stores the relative COMMENT position and CALCULATE can save calculations results into $SET variables. Unfortunately we have no mechanism to "disarm" these, when expanded in this context. so it throws off the COMMENT position, and can cause false results in calculations. And yet, this whole flow is critical because holiday lists might be generated by SEARCH macros, or calculated with SSP, for ex.

Possible solutions:

• Change HolidayListPlugin from using INCLUDE to manually retrieve each topic using readTopic(), disarm any COMMENT macros, and then call expandCommonVariables. But that still leaves CALC, which might be needed for the holiday calculations, and there could also be nested INCLUDEs which also contain COMMENT or CALCULATE macros.
• Implement another one-off hack to turn off COMMENT rendering as was done for EditTablePlugin:

    # Item1491: horrific hack to stop the EditTablePlugin eating it's own
    # feet.
    local $Foswiki::Plugins::EditTablePlugin::recursionBlock = 1;

• Invent a new core context that can be used to tell plugins not to save any state. if (context=trashit) { disable any incrementing counters, saving state, etc }. (Formalizing the EditTablePlugin hack into core)
• Add a state flag to the _RegisteredMacro handler, that is set if expansion is done by some other plugin call to expandCommon, vs. the main expansion code. CommentPlugin would then disable itself, and SpreadSheetPlugin would have to disable the $SET

All of these are rather sub-optimal, in that they address one-off extensions, or need changed to every extension that might be effected. I think the preferred solution would be:

• Change HOLIDAYLIST macro to use a named INCLUDE section instead of whole topic includes. %INCLUDE{"web.topic" section="holidaylist"}%.
• This would involve finding and adding Sections to any topic intended to render a holiday list.
• This could probably be a disabled by configuration to allow graceful migration. HOLIDAYLIST_INCLUDESECTION = (defaults to holidaylist)
• There would still be risk if someone placed a %COMMENT macro or %CALCULATE{$SET macro into the included section, so this needs to be documented as a limitation. but exposure would be much more controlled.

I can't think of any other change that would protect any/all plugins, and yet still allow macro expansion from within the actual holiday list.

-- GeorgeClark - 12 Feb 2016

And we should probably document in general, that using expandCommonVariables on large blocks of text or includes of topics can result in unexpected behavior when macros are expanded in an unexpected order. Ideal use is to expand well defined subsets of macros.

-- GeorgeClark - 12 Feb 2016

To get my plugin to work reasonably I have repeated the hack that was already there.

    # Item1491: horrific hack to stop the EditTablePlugin eating it's own
    # feet.
    local $Foswiki::Plugins::EditTablePlugin::recursionBlock = 1;
    local $Foswiki::Plugins::CommentPlugin::commentIndex = 0;

That seems to work OK for us

-- KennethLavrsen - 15 Feb 2016

Commenting is disabled while not logged in