Even though YUI has been moved to opac-tmpl/lib/yui, the packages were installing some of the CSS assets to opac-tmpl/prog/en/yui
Some more details: The problem is that these two files are not where they are supposed to be: /opac-tmpl/lib/yui/reset-fonts-grids.css /opac-tmpl/lib/yui/skin.css
Created attachment 13720 [details] [review] Bug 9052: 2 yui files installed in the wrong place by the packages
(In reply to comment #1) > Some more details: > > The problem is that these two files are not where they are supposed to be: > > /opac-tmpl/lib/yui/reset-fonts-grids.css > /opac-tmpl/lib/yui/skin.css Please guide me, which files I have to move to which location for proper working?
rangi reports in IRC that the patch doesn't work. Moving back to in discussion.
This is a very dirty (messing in /usr/share is not good if you are not the package manager) fix for it: sudo cp -al /usr/share/koha/opac/htdocs/opac-tmpl/prog/en/css/{reset-fonts-grids,skin}.css lib/yui/
I think Bug 8623 was what exposed this bug. Why aren't we depending on Package libjs-yui anyway?
We are linking in the system yui libraries, however Koha isn't referencing them correctly (e.g. skin.css and reset-fonts-grids.css aren't in the same directory: robin@debmaker32:/usr/share/koha/opac/htdocs$ find -L -name skin.css -or -name reset-fonts-grids.css ./opac-tmpl/lib/yui/assets/skins/sam/skin.css ./opac-tmpl/lib/yui/reset-fonts-grids/reset-fonts-grids.css ./opac-tmpl/prog/en/css/skin.css ./opac-tmpl/prog/en/css/reset-fonts-grids.css The first two are symlinked from libjs-yui, the second two the ones that Koha bundles. The Koha version is quite a bit older than the debian version. This may be a problem, we I'll try it anyway. Koha is also referencing it badly: robin@zarathud:~/catalyst/koha/koha-tmpl/opac-tmpl/prog/en$ grep -r skin.css * includes/doc-head-close.inc:<link rel="stylesheet" type="text/css" href="/opac-tmpl/lib/yui/skin.css" /> modules/sco/help.tt:<link rel="stylesheet" type="text/css" href="[% yuipath %]/skin.css" /> modules/sco/sco-main.tt:<link rel="stylesheet" type="text/css" href="[% yuipath %]/skin.css" /> I think we should be referencing it from the opac.css, that way it's centralised and it can be re-written as part of the package building process.
Created attachment 14110 [details] [review] Bug 9052 - rewrite the YUI links to use the system library This uses libjs-yui to provide the skin.css and reset-fonts-grids.css files from YUI. It patches the CSS files to point to the right location for the files. To test: * Build a package with this patch included * Install it * Look at the OPAC and note that things no longer look terrible, and that there are no 404's coming from bad CSS URLs.
The patch attached hasn't had a lot of testing by me, just enough to indicate that it looks like it works. If someone else could give it a good looking at, that would be appreciated. It deliberately doesn't modify the behaviour for the staff client or the SCO-related templates. I haven't established if there are issues there or not yet, I think the OPAC is more critical.
Created attachment 14118 [details] [review] Bug 9052 - rewrite the YUI links to use the system library This uses libjs-yui to provide the skin.css and reset-fonts-grids.css files from YUI. It patches the CSS files to point to the right location for the files. To test: * Build a package with this patch included * Install it * Look at the OPAC and note that things no longer look terrible, and that there are no 404's coming from bad CSS URLs. Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net> I created a package of 3.10+9052, installed and tested. I confirm that the OPAC does not look broken anymore and I could not find 404 errors with Firebug.
I have not had time do extensive testing, I mainly checked that OPAC pages look OK and Firebug does not show any errors. Given that packages are the recommended way of installation I'd like to see this pushed soon as it makes 3.10 packages usable without manual tweaking -- even if that means less extensive testing. If somebody with more time at hand would do a second signoff that would of course be appreciated. On a sidenote, I had a problem with > libdigest-jhash-perl: 0.07-1koha1 Depends: perlapi-5.10.1 but it is not installable and had to manually install an older version of libdigest-jhash-perl in the pbuilder environment. It happened when i tried to build master+9052, which does not work anyway. I am new to the roll-your-own-packages-business so I'm not sure if it is supposed to or not, probably an error on my side. 3.10+master works and is what I based my signoff on.
(In reply to comment #11) > 3.10+master works and is what I based my signoff on. That line is supposed to read '3.10+9052' works and is what I based my signoff on'
Thanks for testing! The libjhash thing is completely unrelated, basically it's an annoyingly problematic package. We're going to fix that by removing that dependency, as it's optional anyway.
Created attachment 14159 [details] [review] Bug 9052 - rewrite the YUI links to use the system library This uses libjs-yui to provide the skin.css and reset-fonts-grids.css files from YUI. It patches the CSS files to point to the right location for the files. To test: * Build a package with this patch included * Install it * Look at the OPAC and note that things no longer look terrible, and that there are no 404's coming from bad CSS URLs. Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net> I created a package of 3.10+9052, installed and tested. I confirm that the OPAC does not look broken anymore and I could not find 404 errors with Firebug. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> I also created a package of 3.10+9052 and removed libchi-* from the debian/control file. The OPAC shows no problems using those packages, layout, pictures and colors seem to all be in place. Also the OPAC on master with patch applied still works as expected.
I didn't think to put this in the testing notes, however it would be worth checking that these changes don't break a git or tarball install, as I have changed things that shouldn't-but-might affect that, in particular how the CSS is loaded on all OPAC pages.
(In reply to comment #15) > I didn't think to put this in the testing notes, however it would be worth > checking that these changes don't break a git or tarball install, as I have > changed things that shouldn't-but-might affect that, in particular how the > CSS is loaded on all OPAC pages. I think Katrin tested this for a git install right ?
(In reply to comment #16) > I think Katrin tested this for a git install right ? I didn't read it that way initially, but on re-read you might be right.
Yes, I did a git install and took a look at the OPAC there before creating the packages from that changed branch.
Really, we are not going to lose anything, but win the "package installation is our recommended way" back. I think it is hardly possible that it's worse than before.
This patch has been pushed to master.
Pushed to 3.10.x will be in 3.10.1
The staff client needs no work: it seems to keep the .css files in the language specific directory (that said, it might be nice to remove them and uses the system ones anyway, but it's not critical.)
It _is_ an issue for the self-checkout module. I'll patch this in the same way.
Created attachment 14178 [details] [review] Bug 9052 - followup: fix the YUI CSS locations for SCO This is followup to the previous YUI-fixing patch, and simply causes the self-checkout page to load the YUI files via the sco CSS file, and that gets re-written at package build time to work.
Attached patch is a whole lot simpler, but the same testing process should apply as for the previous one.
Created attachment 14185 [details] [review] Bug 9052 - followup: fix the YUI CSS locations for SCO This is followup to the previous YUI-fixing patch, and simply causes the self-checkout page to load the YUI files via the sco CSS file, and that gets re-written at package build time to work. Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Working on this.
Created attachment 14329 [details] [review] [SIGNED-OFF] Bug 9052 - followup: fix the YUI CSS locations for SCO This is followup to the previous YUI-fixing patch, and simply causes the self-checkout page to load the YUI files via the sco CSS file, and that gets re-written at package build time to work. Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Tested on a package installation using packages built by Mirko Tietgen and a git dev installation. To test: 1) Enable WebBasedSelfCheck system preference 2) Enter login date for staff user in AutoSelfCheckAllowed... prefernces 3) Go to the self checkout page: .../cgi-bin/koha/sco/sco-main.pl 4) Verfiy all pages have the usual look, CSS, Javascript and images look normal 5) Verify the same is true for translated templates Manual: http://manual.koha-community.org/3.10/en/selfcheckout.html
Doesn't apply to 3.10.x please rebase and send a new patch
This patch is already in 3.10.x.