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

Jason Judge wrote:
> "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).
> 

Yeah it took me a few minutes to figure out what was going on as it work 
fine on my Mac OSX machine, but wasn't working on one of my RedHat machines.


> 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
> 
>