Bug 10052 - Make intranetstylesheet and intranetcolorstylesheet behave exactly like their opac counterparts
Summary: Make intranetstylesheet and intranetcolorstylesheet behave exactly like their...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Marcel de Rooy
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-15 08:57 UTC by Marcel de Rooy
Modified: 2014-12-07 20:02 UTC (History)
2 users (show)

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


Attachments
Bug 10052: Make intranetstylesheet and intranetcolorstylesheet behave exactly like their opac counterparts (5.27 KB, patch)
2013-04-15 10:29 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10052: Db revision for intranetstylesheet (2.43 KB, patch)
2013-04-15 10:29 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10052: Make intranetstylesheet and intranetcolorstylesheet behave exactly like their opac counterparts (5.27 KB, patch)
2013-04-15 10:29 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10052: Db revision for intranetstylesheet (2.43 KB, patch)
2013-04-15 10:42 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10052: Make intranetstylesheet and intranetcolorstylesheet behave exactly like their opac counterparts (5.27 KB, patch)
2013-04-15 10:42 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10052 Followup for edithelp.tt and help-top.inc (6.37 KB, patch)
2013-04-15 11:32 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10052 Followup for edithelp.tt and help-top.inc (6.34 KB, patch)
2013-04-15 16:33 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10052: Db revision for intranetstylesheet (2.43 KB, patch)
2013-05-03 09:06 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 10052: Make intranetstylesheet and intranetcolorstylesheet behave exactly like their opac counterparts (5.30 KB, patch)
2013-05-03 09:13 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 10052 Followup for edithelp.tt and help-top.inc (6.37 KB, patch)
2013-05-03 09:14 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 10052: Db revision for intranetstylesheet (2.55 KB, patch)
2013-05-13 07:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10052: Make intranetstylesheet and intranetcolorstylesheet behave exactly like their opac counterparts (5.33 KB, patch)
2013-05-13 07:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10052 Followup for edithelp.tt and help-top.inc (6.39 KB, patch)
2013-05-13 07:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10052: QA Followup (1.33 KB, patch)
2013-05-13 07:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review
[PASSED QA] Bug 10052: Db revision for intranetstylesheet (2.59 KB, patch)
2013-05-14 06:29 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 10052: Make intranetstylesheet and intranetcolorstylesheet behave exactly like their opac counterparts (5.35 KB, patch)
2013-05-14 06:29 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 10052 Followup for edithelp.tt and help-top.inc (6.49 KB, patch)
2013-05-14 06:29 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 10052: QA Followup (1.42 KB, patch)
2013-05-14 06:29 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2013-04-15 08:57:12 UTC
This is actually a followup on bug 8263: Make OPAC stylesheet preferences more consistent.
The changes for intranet smaller.

Pref intranetstylesheet may be blank: meaning use staff-global.css (no change)
if it contains a full http or https prefixed string, use that (no change)
if it contains a root symbol / at the start, just follow the pref (no change)
if it contains something else, it should be interpreted as relative and prefixed with [themelanguage]/css (this is NEW behavior and just like OPAC)

Pref intranetcolorstylesheet now is always prefixed with theme/css.
For consistency with opac and intranetstylesheet, we should add the above logic for an absolute path (root symbol) or http(s) prefixed path.
This means that we add new behavior: allow css for colors outside the themedir and even remote.

Does this change need a db revision?
If intranetstylesheet now contains say $relpath (not blank and not starting with http or root symbol), and we must assume that it worked :), we cannot simply prefix it with theme. What to do?
Most safe approach is: prefix $relpath with / here.
So: Yes, we do need a db rev for this unusual case.
There is no change needed for the intranetcolorstylesheet.

This report changes include doc-head-close.inc, and updates the descriptions for the prefs in staff_client.pref, with a dbrev in updatedatabase.pl.
Comment 1 Marcel de Rooy 2013-04-15 10:29:20 UTC Comment hidden (obsolete)
Comment 2 Marcel de Rooy 2013-04-15 10:29:37 UTC Comment hidden (obsolete)
Comment 3 Marcel de Rooy 2013-04-15 10:29:43 UTC Comment hidden (obsolete)
Comment 4 Marcel de Rooy 2013-04-15 10:42:10 UTC
Wait: I will move the Print Version statement of the dbrev outside the if.
Comment 5 Marcel de Rooy 2013-04-15 10:42:27 UTC Comment hidden (obsolete)
Comment 6 Marcel de Rooy 2013-04-15 10:42:30 UTC Comment hidden (obsolete)
Comment 7 Marcel de Rooy 2013-04-15 10:56:45 UTC
Still some work to do:
koha-tmpl/intranet-tmpl/prog/en/modules/help/edithelp.tt
koha-tmpl/intranet-tmpl/prog/en/includes/help-top.inc

Will move the code to an include?
Comment 8 Marcel de Rooy 2013-04-15 11:32:13 UTC Comment hidden (obsolete)
Comment 9 Marcel de Rooy 2013-04-15 16:33:01 UTC Comment hidden (obsolete)
Comment 10 Chris Cormack 2013-05-03 09:06:53 UTC Comment hidden (obsolete)
Comment 11 Chris Cormack 2013-05-03 09:13:18 UTC Comment hidden (obsolete)
Comment 12 Chris Cormack 2013-05-03 09:14:06 UTC Comment hidden (obsolete)
Comment 13 Katrin Fischer 2013-05-12 14:11:16 UTC
Hi Marcel,

I am trying to figure out how this works and entered a http://... into my intranetstylesheet preference. I can see that it works for the staff pages, but the help files (http://localhost:8080/cgi-bin/koha/help.pl) still refer to staff-global.css. Can you take a look please? All 3 patches were applied and the databaes update left my http:// URL alone.


<title>Online Help</title>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<link rel="shortcut icon" href="/intranet-tmpl/prog/en/includes/favicon.ico" type="image/x-icon" />
<link rel="stylesheet" type="text/css" href="/intranet-tmpl/lib/jquery/jquery-ui.css" />
<link rel="stylesheet" type="text/css" media="print" href="/intranet-tmpl/prog/en/css/print.css" />


    <link rel="stylesheet" type="text/css" href="/intranet-tmpl/prog/en/css/staff-global.css" />


<!-- local colors -->
Comment 14 Marcel de Rooy 2013-05-13 07:16:05 UTC Comment hidden (obsolete)
Comment 15 Marcel de Rooy 2013-05-13 07:16:09 UTC Comment hidden (obsolete)
Comment 16 Marcel de Rooy 2013-05-13 07:16:12 UTC Comment hidden (obsolete)
Comment 17 Marcel de Rooy 2013-05-13 07:16:15 UTC Comment hidden (obsolete)
Comment 18 Marcel de Rooy 2013-05-13 07:21:20 UTC
(In reply to comment #13)
> I am trying to figure out how this works and entered a http://... into my
> intranetstylesheet preference. I can see that it works for the staff pages,
> but the help files (http://localhost:8080/cgi-bin/koha/help.pl) still refer
> to staff-global.css. Can you take a look please? All 3 patches were applied
> and the databaes update left my http:// URL alone.

Good testing, Katrin! Gründlich :) Thanks.
You found something that must have been there for some time already..
The template var was not filled because it is not loaded via get_template_and_user but just gettemplate.
Added a QA followup to fill them now. Resolved merge conflict on updatedb.
Comment 19 Katrin Fischer 2013-05-14 06:29:33 UTC
Created attachment 18124 [details] [review]
[PASSED QA] Bug 10052: Db revision for intranetstylesheet

Report 10052 makes the intranetstylesheet prefs behave like the opac ones.
This patch contains an optional db revision: If the pref intranetstylesheet is
not blank, and does not start with http, https or /, then prefix it with a
slash to make it an absolute path.

Test plan:
Before applying this patch:
1) Put intranet-tmpl/prog/en/css/staff-global.css into intranetstylesheet.
This works in the current staff client, but will be interpreted as relative as
soon as both patches for this report have been applied.
Apply the patch. And run: /cgi-bin/koha/installer/install.pl?step=3&op=updatestructure
Check the webinstaller warning and the pref value. There should be a slash
in front now. The css should still work.

Continue after applying this patch with:
2) Clear the intranetstylesheet pref. Run updatestructure again. No message: OK
3) Put /intranet-tmpl/prog/en/css/staff-global.css in intranetstylesheet.
Run updatestructure again. No message: OK
4) Prefix intranetstylesheet with http://[your-server-here]. Test again.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comments on last patch.
Comment 20 Katrin Fischer 2013-05-14 06:29:44 UTC
Created attachment 18125 [details] [review]
[PASSED QA] Bug 10052: Make intranetstylesheet and intranetcolorstylesheet behave exactly like their opac counterparts

Allow a relative path (within theme/css) in intranetstylesheet too.
Allow a full path, local or remote with http(s), in intranetcolorstylesheet.
This restores consistency between opac and staff.

Test plan:
First: the patch with the db revision should have been applied. And also the
updatestructure step from the web installer should have been run.

Clear intranetstylesheet. Check included stylesheet with page source in browser.
Enter a full path with http or / in intranetstylesheet. Check again.
Enter a relative path (such as staff-global.css). Check again.

Clear intranetcolorstylesheet. Check if there is no reference with page source.
Enter a full path with http or / in intranetcolorstylesheet. Check reference.
Enter a relative path (such as blue.css). Check again.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 21 Katrin Fischer 2013-05-14 06:29:50 UTC
Created attachment 18126 [details] [review]
[PASSED QA] Bug 10052 Followup for edithelp.tt and help-top.inc

A grep on the stylesheet names revealed that similar code was used in those
template files.
This patch moves the code from doc-head-close.inc to a new include file.
Now, doc-head-close and the help templates refer to that one file.

Test plan:
If you tested the preceding two patches, now you only need to:
1) Check included css via page source in browser from staff main page.
2) Check it also from some other location.
3) Check again from Help: e.g. /cgi-bin/koha/help.pl
4) Check again from Edit Help: /cgi-bin/koha/edithelp.pl

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Template only, no string changes.
Tested according to test plan.
Comment 22 Katrin Fischer 2013-05-14 06:29:57 UTC
Created attachment 18127 [details] [review]
[PASSED QA] Bug 10052: QA Followup

As Katrin pointed out, the intranetstylesheet setting was not respected in the
help pages. The script help.pl does not use get_template_and_user which sets
these variables via Auth.pm but calls gettemplate.
This makes it necessary to 'manually' set them. This patch does that.
Evidently, this could be the case for some other templates vars too..

Test plan:
Apply all patches for 10052.
Change intranetstylesheet. Check a help page.
Change intranetcolorstylesheet. Check a help page.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Looks all good to me now.
Passes tests and still no string changes.
Comment 23 Jared Camins-Esakov 2013-05-14 12:21:20 UTC
String changes on the patch "Bug 10052: Make intranetstylesheet and intranetcolorstylesheet behave exactly like their opac counterparts." Holding for 3.14.
Comment 24 Katrin Fischer 2013-05-14 12:24:23 UTC
Oh sorry, missed that :(
Comment 25 Marcel de Rooy 2013-05-23 08:28:48 UTC
Version back to master.
Comment 26 Galen Charlton 2013-05-23 16:05:51 UTC
Pushed to master.  Thanks, Marcel!