Re: 1 cset was pushed to modules/xarpages: Minor bug fix. Now using the themes osdi...

"Brian McGilligan" <brian-4Y3+cfMDcQXD0D/[email protected]> wrote in message
news:[email protected]...
> Jason Judge wrote:
>
> > Nope. If xarTplSetThemeName() is not setting the theme by its *name*
then it
> > is that which needs fixing. Bug 3876 is tracking this problem. We should
fix
> > it from that end.
> >
> > -- JJ
>
>
> So
>
> function xarTplSetThemeName($themeName)
> {
>      assert('$themeName != "" && $themeName{0} != "/"');
>      if (!file_exists($GLOBALS['xarTpl_themesBaseDir'].'/'.$themeName)) {
>          return false;
>      }
>
>      xarTpl__SetThemeNameAndDir($themeName);
>      return true;
> }
>
>
> should be
>
> function xarTplSetThemeName($themeName)
> {
>      $themeInfo = xarModInfo(xarModGetIDFromName($themeName, 'theme'),
> 'theme');
>      $themeDir = $themeInfo['osdirectory'];
>      assert('$themeDir != "" && $themeDir{0} != "/"');
>      if (!file_exists($GLOBALS['xarTpl_themesBaseDir'].'/'.$themeDir)) {
>          return false;
>      }
>
>      xarTpl__SetThemeNameAndDir($themeName);
>      return true;
> }
>
> or something like it, right?
>
> I'll go ahead and undo my commit to xarPages. Sorry about messing with
> xarPages.

No problem - it just highlights problems in the core :-) I didn't even
realise that didn't work until you made that change (though strangely, the
print theme did work with xarpages for me, on Linux too).

The suggested fix you've gven is just about right, IMO. The function should
check the module name first, and then fall back to the OS directory if there
is no match. However, even just checking the directory is probably wrong,
because it does not take into account whether the theme is enabled or not.

-- JJ