Item8723: Broken (?) Inclusion order of Skin Template Topics

Priority: Urgent
Current State: Closed
Released In: 1.1.0
Target Release: minor
Applies To: Engine
Reported By: DiabJerius
Waiting For:
Last Change By: KennethLavrsen
I'm converting a TmWiki web over to Foswiki. That web has it's own skin, which slightly modified the Pattern skin. I implemented it entirely in Topics, and set SKIN = myskin,pattern in WebPreferences. All was well.

I've moved over to Foswiki 1.0.9 with the default TemplatePath. Something has changed. Here's a very short example which illustrates the problem.

I've created a topic TestSkinViewTemplate:




and Set SKIN = test, pattern in WebPreferences. I expect that the page will be rendered without a sidebar and that HELLO THERE, 1 will appear somewhere on the page. Instead the sidebar is still there, and I see the text HELLO THERE, 1. That means that the topic is indeed being parsed, but that the %TMPL:DEF% command is being overridden.

To debug this, I added some code to Foswiki::Templates::_readTemplateFile to prefix each template line with the filename or topic from which it was read, and then printed out the final recursively generated template text. Here are the interesting results of that log, looking just for the definitions of sidebar:

TestWeb.TestSkinViewTemplate: %TMPL:DEF{"sidebar"}%%TMPL:END%
viewsidebar.pattern.tmpl: %TMPL:DEF{"sidebar"}%<div id="patternSideBar"><div id="patternClearHeaderLeft"></div>
What's obvious is that the definition from TestSkinViewTemplate is showing up first and is getting overridden. I expected it to show up last.

More debug output showing the order in which the templates are read:
retrieveTopic( Foswiki::Store=HASH(0xbab4f0), TestWeb, TestSkinViewTemplate )

Here's what I think is happening. The first template file read is view.pattern.tmpl. It starts out like this:
[... lots of definitions ...]
The template inclusion code essentially replaces the %TMPL:INCLUDE{...}% with the contents of the templates. So, when the first INCLUDE is processed, it picks up TestWeb.TestSkinViewTemplate and inserts that before the inclusion of viewsidebar.pattern.tmpl. When viewsidebar.pattern.tmpl is eventually included it overrides the definition of sidebar from the topic.

I believe that the problem is that view.pattern.tmpl is being processed first. If the topic template was processed first, then I believe all would work as before.

Thanks, Diab

-- DiabJerius - 17 Mar 2010

This may mean nothing, but if I set VIEW_TEMPLATE to TestSkinView it works as expected.

-- DiabJerius - 18 Mar 2010

wow - that's actually a very interesting observation.

-- SvenDowideit - 30 Apr 2010

Any hope of reverting to the old behavior? Being able to create templates without having to muck about on the server is really really useful. It makes users happy, and it's so much easier to upgrade.

-- DiabJerius - 07 May 2010

I wasn't involved in the template path changes, so I'm not sure why that search order was chosen; I think maybe Kenneth and Arthur were? But what you describe certainly sounds like a bug.

-- CrawfordCurrie - 14 May 2010

I was not involved either.

-- ArthurClemens - 14 May 2010

Sven made the change under the auspices of Item4463.

If you change it back, then don't forget to change the doc in SkinTemplates as well Item2558.

-- CrawfordCurrie - 07 Jun 2010

My feeling is that we are dammed no matter what we do.

This template order was also changed a few times in the old project.

There is always one use case where one order is the best and another usecase where another order is the best.

I doubt there is a path that can be defined as the ultimate right path.

I guess this is why the configure setting was introduced in the first place.

Maybe instead of keeping on changing it, configure help text should suggest 2 or 3 alternative paths for some typical use cases, and then let the current default stay what it is.

-- KennethLavrsen - 18 Jul 2010

I agree, though I think a page in Support web (and a link to that page in configure) is the appropriate action. And nothing is going to happen if we wait for Sven to provide feedback on this, so I'm moving it back to confirmed. Anyone can pick it up for action.

-- CrawfordCurrie - 22 Jul 2010


-- CrawfordCurrie - 26 Jul 2010


ItemTemplate edit

Summary Broken (?) Inclusion order of Skin Template Topics
ReportedBy DiabJerius
Codebase 1.0.9, trunk
SVN Range
AppliesTo Engine
Priority Urgent
CurrentState Closed
Checkins distro:c5ebf7ea9ce9
TargetRelease minor
ReleasedIn 1.1.0
Topic revision: r12 - 04 Oct 2010, KennethLavrsen - This page was cached on 17 Oct 2018 - 19:56.

The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. See Copyright Statement. Creative Commons License    Legal Imprint    Privacy Policy