Bug 8623 - Javascript libraries should be outside theme directories
Summary: Javascript libraries should be outside theme directories
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: 3.10
Hardware: All All
: P5 - low normal (vote)
Assignee: Jared Camins-Esakov
QA Contact: Mason James
URL:
Keywords:
: 4048 (view as bug list)
Depends on:
Blocks: 8597 8622
  Show dependency treegraph
 
Reported: 2012-08-12 23:57 UTC by Jared Camins-Esakov
Modified: 2013-12-05 20:06 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 8623: Do not accept "lib" as a valid theme (947 bytes, patch)
2012-08-13 00:14 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 8623: Move YUI out of prog (8.51 KB, patch)
2012-08-13 01:22 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 8623: Do not accept "lib" as a valid theme (961 bytes, patch)
2012-08-13 16:15 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 8623: Move YUI out of prog (8.52 KB, patch)
2012-08-13 16:16 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 8623 - Fix YUI path for OPAC in debian/rules (2.89 KB, patch)
2012-09-17 19:07 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 8623 - Fix YUI path for OPAC in debian/rules (3.13 KB, patch)
2012-09-27 13:13 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 8623 follow-up: make sure CCSR theme loads YUI (2.45 KB, patch)
2012-09-27 13:13 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8623 - Fix YUI path for OPAC in debian/rules (3.22 KB, patch)
2012-09-28 12:55 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8623 follow-up: make sure CCSR theme loads YUI (2.54 KB, patch)
2012-09-28 12:56 UTC, Kyle M Hall
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:57:46 UTC
Similar to bug 4048, which seeks to move Koha's non-translatable assets outside of language-specific directories, third-party javascript libraries should be stored outside of the theme hierarchy entirely, in koha-tmpl/opac-tmpl/lib/ and koha-tmpl/intranet-tmpl/lib/. This will prevent future regressions in Koha's theming functionality.
Comment 1 Jared Camins-Esakov 2012-08-13 00:14:51 UTC Comment hidden (obsolete)
Comment 2 Jared Camins-Esakov 2012-08-13 01:22:11 UTC Comment hidden (obsolete)
Comment 3 Owen Leonard 2012-08-13 10:03:55 UTC
The second patch doesn't apply:

Applying: Bug 8623: Move YUI out of prog
fatal: sha1 information is lacking or useless (C4/Templates.pm).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Comment 4 Chris Cormack 2012-08-13 10:05:09 UTC
Owen, it depends on the patch for bug 8622 being applied first.
Comment 5 Owen Leonard 2012-08-13 10:09:43 UTC
*** Bug 4048 has been marked as a duplicate of this bug. ***
Comment 6 Kyle M Hall 2012-08-13 16:15:23 UTC Comment hidden (obsolete)
Comment 7 Kyle M Hall 2012-08-13 16:16:16 UTC Comment hidden (obsolete)
Comment 8 Mason James 2012-08-17 23:47:09 UTC
(In reply to comment #7)
> Created attachment 11582 [details] [review]
> Bug 8623: Move YUI out of prog
> 
> This patch moves the YUI assets from koha-tmpl/opac-tmpl/prog/en/lib/yui
> to koha-tmpl/opac-tmpl/lib/yui.
> 
> NOTE: This was tested on Chrome, FF, and Safari on a Mac, and IE and FF
> on Windows.
> 
> To test:
> 1) View a smattering of pages on the OPAC and intranet. If the move
>    did not work flawlessly, layout will be way off.
> 
> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

QA comments
patch wont apply against master '322a9c7fbbcf456084d1937f48345b3cdec058ce'

please resend a patch that applies against master


-------------------------------------------
mason@xen1:~/git/head-clean$ gl |head -1
commit 322a9c7fbbcf456084d1937f48345b3cdec058ce

mason@xen1:~/git/head-clean$ git bz apply 8623
Bug 8623 - Javascript libraries should be outside theme directories

Bug 8623: Do not accept "lib" as a valid theme
Apply? [yn] y

Applying: Bug 8623: Do not accept "lib" as a valid theme
Bug 8623: Move YUI out of prog
Apply? [yn] y

Applying: Bug 8623: Move YUI out of prog
fatal: sha1 information is lacking or useless (C4/Templates.pm).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 8623: Move YUI out of prog
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
Patch left in /tmp/Bug-8623-Move-YUI-out-of-prog-DVU7vK.patch

-------------------------------------------
Comment 9 Mason James 2012-08-17 23:50:47 UTC
> please resend a patch that applies against master
> 

... or explain a suitable method to apply the patch successfully
(which i might be missing)
Comment 10 Mason James 2012-08-20 03:26:11 UTC
> 
> QA comments
> patch wont apply against master '322a9c7fbbcf456084d1937f48345b3cdec058ce'
> 
> please resend a patch that applies against master
> 

all good :) this patch applies OK, after patch 8622

ready for QA again...
Comment 11 Mason James 2012-08-31 03:58:23 UTC
> all good :) this patch applies OK, after patch 8622
> 
> ready for QA again...


passing QA on this

$ koha-qa.pl -c 2

testing 2 commit(s) (applied to commit 6ca9b8f)
 * ba98cb9 Bug 8623: Move YUI out of prog
      C4/Templates.pm
      koha-tmpl/opac-tmpl/lib/yui/button.css
      koha-tmpl/opac-tmpl/lib/yui/container.css
      koha-tmpl/opac-tmpl/lib/yui/container/container-min.js
      koha-tmpl/opac-tmpl/lib/yui/container/container_core-min.js
      koha-tmpl/opac-tmpl/lib/yui/menu.css
      koha-tmpl/opac-tmpl/lib/yui/menu/menu-min.js
      koha-tmpl/opac-tmpl/lib/yui/reset-fonts-grids.css
      koha-tmpl/opac-tmpl/lib/yui/skin.css
      koha-tmpl/opac-tmpl/lib/yui/utilities/utilities.js
      koha-tmpl/opac-tmpl/lib/yui/yui-sprite.png
      koha-tmpl/opac-tmpl/prog/en/css/opac.css
      koha-tmpl/opac-tmpl/prog/en/css/sco.css
      koha-tmpl/opac-tmpl/prog/en/includes/doc-head-close.inc
      koha-tmpl/opac-tmpl/prog/en/lib/yui/button.css
      koha-tmpl/opac-tmpl/prog/en/lib/yui/container.css
      koha-tmpl/opac-tmpl/prog/en/lib/yui/container/container-min.js
      koha-tmpl/opac-tmpl/prog/en/lib/yui/container/container_core-min.js
      koha-tmpl/opac-tmpl/prog/en/lib/yui/menu.css
      koha-tmpl/opac-tmpl/prog/en/lib/yui/menu/menu-min.js
      koha-tmpl/opac-tmpl/prog/en/lib/yui/reset-fonts-grids.css
      koha-tmpl/opac-tmpl/prog/en/lib/yui/skin.css
      koha-tmpl/opac-tmpl/prog/en/lib/yui/utilities/utilities.js
      koha-tmpl/opac-tmpl/prog/en/lib/yui/yui-sprite.png
      koha-tmpl/opac-tmpl/prog/en/modules/sco/help.tt
      koha-tmpl/opac-tmpl/prog/en/modules/sco/sco-main.tt

 * 2de8f5f Bug 8623: Do not accept "lib" as a valid theme
      C4/Koha.pm

* C4/Templates.pm                                                          OK
* C4/Koha.pm                                                               OK
* koha-tmpl/opac-tmpl/prog/en/includes/doc-head-close.inc                  OK
* koha-tmpl/opac-tmpl/prog/en/modules/sco/help.tt                          OK
* koha-tmpl/opac-tmpl/prog/en/modules/sco/sco-main.tt                      OK
Comment 12 Paul Poulain 2012-09-05 10:09:25 UTC
I could not find any difference between display before and after the patch, which is what I was expecting.
However, I think it's more an ENH than a bugfix, changing status. But, as usual, chris_c, it's your final decision.
Comment 13 Tomás Cohen Arazi 2012-09-17 19:07:32 UTC Comment hidden (obsolete)
Comment 14 Jared Camins-Esakov 2012-09-27 13:13:01 UTC Comment hidden (obsolete)
Comment 15 Jared Camins-Esakov 2012-09-27 13:13:20 UTC Comment hidden (obsolete)
Comment 16 Jared Camins-Esakov 2012-09-27 13:15:26 UTC
Follow-ups fix bugs.
Comment 17 Kyle M Hall 2012-09-28 12:55:40 UTC
Created attachment 12574 [details] [review]
[SIGNED-OFF] Bug 8623 - Fix YUI path for OPAC in debian/rules

After the changes to YUI path the build process is broken. For testing this I had
to set the build process to ommit the tests as there are some db_dependent test being
called (which failed).

To do this I ran the build script like this:

sudo DEB_BUILD_OPTIONS=nocheck ./debian/build-git-snapshot -distribution precise -D precise -r ~/ubuntu -v 3.9.0-046~git -d

Note: Even when this is fixed, there are still issues with some files missing

dh_install: etc/koha/searchengine/solr/config.yaml exists in debian/tmp but is not installed to anywhere
dh_install: etc/koha/searchengine/solr/indexes.yaml exists in debian/tmp but is not installed to anywhere
dh_install: etc/koha/solr/indexes.yaml exists in debian/tmp but is not installed to anywhere
dh_install: missing files, aborting
make: *** [binary] Error 2
dpkg-buildpackage: error: fakeroot debian/rules binary gave error exit status 2

Sponsored-by: Universidad Nacional de Córdoba

Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Please note that debian/rules *must* use tabs. If you have a git hook
that automatically replaces tabs with spaces, please undo those changes.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 18 Kyle M Hall 2012-09-28 12:56:00 UTC
Created attachment 12575 [details] [review]
[SIGNED-OFF] Bug 8623 follow-up: make sure CCSR theme loads YUI

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 19 Mason James 2012-10-03 00:05:03 UTC
(In reply to comment #18)
> Created attachment 12575 [details] [review]
> [SIGNED-OFF] Bug 8623 follow-up: make sure CCSR theme loads YUI
> 
> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

passing QA on 2 patches, tested OK

$ koha-qa.pl -c 2

testing 2 commit(s) (applied to commit 0a35b26)
 * fabf5cf Bug 8623 follow-up: make sure CCSR theme loads YUI
      koha-tmpl/opac-tmpl/ccsr/en/css/opac.css
      koha-tmpl/opac-tmpl/ccsr/en/css/sco.css
      koha-tmpl/opac-tmpl/ccsr/en/includes/doc-head-close.inc

 * 399f7cd Bug 8623 - Fix YUI path for OPAC in debian/rules
      debian/rules

* koha-tmpl/opac-tmpl/ccsr/en/includes/doc-head-close.inc                  OK
Comment 20 Paul Poulain 2012-10-05 09:49:49 UTC
Patch pushed to master
(1st patch pushed to bug_8623 branch, 2nd patch on master only, does not apply on bug_8623)
Comment 21 Chris Cormack 2013-04-20 23:16:49 UTC
Released in 3.10.0