I think that TWiki::Form::new may be invoking handleCommonTags() with the incorrect topic/web names.
TWiki::Form::new supports:
- Retrieving values from other topics
- using a %SEARCH% to dynamically generate that list
Here's the code responsible:
if ( $store->topicExists( $fieldWeb, $fieldTopic ) ) {
my( $meta, $text ) =
$store->readTopic( $session->{user},
$fieldWeb, $fieldTopic, undef );
# Add processing of SEARCHES for Lists
$text = $this->{session}->handleCommonTags(
$text,$this->{web},$this->{topic});
@posValues = _getPossibleFieldValues( $text );
$fieldDef->{type} ||= 'select'; #FIXME keep?
}
This results in handleCommonTags() being called with the text from $fieldTopic but the the topic name of the form template. This breaks plugins like
VarCachePlugin that rely on the topic name. In the case of
VarCachePlugin, this causes the plugin to subsitute text from the incorrect topic as the list of options.
Logically, it seems like handleCommonTags should be invoked with the topic name of the text it is processing, especially since inlined SEARCHes are expanded elsewhere in TWiki::Form::_parseFormDefinition() using the template name. Was this behavior deliberate?
Potential fix:
--- Form.pm 2007-05-01 18:19:01.000000000 -0400
+++ Form.pm.field 2007-05-01 18:19:53.000000000 -0400
@@ -120,7 +120,7 @@
$fieldWeb, $fieldTopic, undef );
# Add processing of SEARCHES for Lists
$text = $this->{session}->handleCommonTags(
- $text,$this->{web},$this->{topic});
+ $text,$fieldWeb,$fieldTopic);
@posValues = _getPossibleFieldValues( $text );
$fieldDef->{type} ||= 'select'; #FIXME keep?
}
The above patch resolves my problem, but I'm not sure if it creates others.
--
TWiki:Main/LeeVerberne
- 01 May 2007
Hi Lee,
Thanks for the patch, but before we apply it we really, really need a testcase. This usually takes the form as one or more topics that demonstrate the problem in
LitterTray web, so that we can test that the "fix is in". Testcases are usually re-coded as unit tests, and thereby become part of the TWiki spec.
If you could work up a testcase, we can take this further. Thanks,
--
TWiki:Main.CrawfordCurrie
- 02 May 2007
Hi Crawford -- thanks for the quick response!
I'm not recommending the patch (yet) so much as asking for guidance. I've created the following pages in the
LitterTray to demonstrate what's happening:
When you attempt to edit the form in
Item4004Example, the %TOPIC% in the form value is evaluated differently than when it's viewed in
TopicClassification.
On the one hand, this behavior might be intentional since it makes it seem like the information contained in
TopicClassification is actually coming from
Item4004Form. On the other hand, it causes the following problem for me:
I have some rather complicated SEARCHes to generate dynamic list of options. They take quite a long time (on the order of 5 minutes), so there is a tremendous lag between when you click on the Edit link and actually receive the page to edit. My thought is that I'd use the
VarCachePlugin to cache the results of the search (which required adding an option to
VarCachePlugin to force caching semantics in the presences of url params so that my edit links would read from the cache), but this breaks because the
VarCachePlugin returns the cached data for the Form topic (e.g.
Item4004Form) rather than the cached data of the SEARCH (e.g.
TopicClassification).
I think that the behavior should match the semantics of %TOPIC% & %BASETOPIC% when used with %INCLUDE%. That is, %TOPIC% should expand to the same value it does when viewing the topic directly; this seems to better fit the TWiki paradigm. It's a bit of a grey area, though, so I'm happy to concede that there might be other reasons this behavior was chosen and try to find some other way to solve my performance problem.
So I guess my question is, is this behavior intentional or is it a bug? If the former, why was it chosen and should we change it?
Thanks,
--
TWiki:Main.LeeVerberne
- 02 May 2007
OIC - you are interpreting the use of a wikiword for the name of a list of items in a select to be an implicit INCLUDE. I am fairly confident that the behaviour you are observing is accidental rather than designed, and was the implementors best guess.
It would certainly be a good idea to clean this up. However it's also a "spec change" (yes, I know there
is no spec, but any change in behaviour like this is a spec change)
so any change would involve documenting the behaviour, and writing unit tests for the agreed behaviour.
To get a spec change in motion requires a
ChangeProposal topic in Codev web. This will give it open discussion, in case someone knows why it was done this way or has a strong opinion one way or the other. I can imagine some people might react because of the potential impact on existing TWiki applications.
I'm setting this to "no action", as it's not actually a bug, and in anticipation that you will open the discussion in Codev. Once it's agreed there, you can open this again as an enhancement
BTW discussions on Codev don't take long these days, as long as someone is pushing them.
--
TWiki:Main.CrawfordCurrie
- 02 May 2007