Bug 8622 - Koha themes broken
Summary: Koha themes broken
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: 3.10
Hardware: All All
: P3 major (vote)
Assignee: Jared Camins-Esakov
QA Contact: Mason James
URL:
Keywords:
Depends on: 8623
Blocks: 8597
  Show dependency treegraph
 
Reported: 2012-08-12 23:55 UTC by Jared Camins-Esakov
Modified: 2013-12-05 20:01 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 8622: Fix theme fallback (12.42 KB, patch)
2012-08-12 23:56 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 8622: Fix theme fallback (12.44 KB, patch)
2012-08-13 15:39 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 8622: Fix theme fallback (12.55 KB, patch)
2012-08-20 12:39 UTC, Mason James
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jared Camins-Esakov 2012-08-12 23:55:24 UTC
Due to regressions in the codebase, Koha's built in theming support (e.g. prog) no longer functions properly: all of prog must be duplicated even for a relatively small change due to errors in fallback for TT includes and XSLT files. Moreover, in places where fallback *was* working, incorrect CSS, javascript, and image files were being displayed because [% themelang %] in the templates was being set to the fallback theme/language, rather than the current theme/language. CSS, Koha-specific javascript, and image files should always be pulled from the preferred theme, never from the fallback theme.
Comment 1 Jared Camins-Esakov 2012-08-12 23:56:03 UTC Comment hidden (obsolete)
Comment 2 Jared Camins-Esakov 2012-08-13 00:58:31 UTC
To test:
1) Ensure that images, CSS, and Javascript load normally with the prog theme.
2) View an XSLT-generated block in English and another language, and ensure that the XSLT is rendered in both.
3) [optional] Either create a new theme or add the theme created by bug 8597, switch to the new theme, and ensure that images, CSS and Javascript load normally.
4) [optional] In this other theme, view an XSLT-generated block in English and another language, and ensure that they render.

If there are any problems with this patch, it will be readily apparent because most assets will not load.
Comment 3 Kyle M Hall 2012-08-13 15:39:04 UTC Comment hidden (obsolete)
Comment 4 Kyle M Hall 2012-08-13 16:05:56 UTC
SO'd. Leaving as "Needs Signoff" for more testing. Tested cases 1 & 2.
Comment 5 Mason James 2012-08-20 12:37:42 UTC
(In reply to comment #4)
> SO'd. Leaving as "Needs Signoff" for more testing. Tested cases 1 & 2.


i'll add a 2nd sign-off for cases 3 and 4. 

they work OK too :)
Comment 6 Mason James 2012-08-20 12:39:33 UTC
Created attachment 11701 [details] [review]
Bug 8622: Fix theme fallback

This patch does the following:
1) Enables fallback for includes between different themes and different
   languages (with the exact same precedence as for .tt files)
2) Enable fallback for XSLT files between different themes and different
   languages (again, same precedence)
3) Change the semantics of the TT [% themelang %] variable so that it always
   refers to the preferred theme and language, rather than the fallback
   theme/language. As a result, all themes must include all javascript,
   css and image resources they use.

Note that these changes actually have no impact whatsoever on an
installation where the default (prog) themes are in use.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>
Comment 7 Chris Cormack 2012-08-20 20:15:53 UTC
All cases have now been tested, switching to signed off.
Comment 8 Mason James 2012-08-31 04:01:14 UTC
(In reply to comment #7)
> All cases have now been tested, switching to signed off.

passing QA on this patch

$ koha-qa.pl -c 1

testing 1 commit(s) (applied to commit ba98cb9)
 * 4b071b1 Bug 8622: Fix theme fallback
      C4/Output.pm
      C4/Reports/Guided.pm
      C4/Templates.pm
      C4/XSLT.pm
      catalogue/moredetail.pl
      edithelp.pl
      opac/opac-main.pl

* C4/Output.pm                                                             OK
* C4/Reports/Guided.pm                                                     OK
* C4/Templates.pm                                                          OK
* C4/XSLT.pm                                                               OK
* catalogue/moredetail.pl                                                  OK
* edithelp.pl                                                              OK
* opac/opac-main.pl                                                        OK
Comment 9 Paul Poulain 2012-09-05 10:02:13 UTC
Patch pushed to master

Could not see any difference between before and after the patch, which what we expect when we use prog theme (what most libraries does)
Comment 10 Jared Camins-Esakov 2012-12-03 13:07:15 UTC
Theme fallback is now working on master.