Bug 9551

Summary: OPACMobileUserCSS being overriden by OPACUserCSS
Product: Koha Reporter: Ed Veal <eveal>
Component: OPACAssignee: Mark Tompsett <mtompset>
Status: CLOSED INVALID QA Contact:
Severity: minor    
Priority: P5 - low CC: bgkriegel, chris, gmcharlt, katrin.fischer, kyle, mtompset, nengard
Version: 3.16   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 9551 - OPACMobileUserCSS being overriden by OPACUserCSS
Bug 9551 - OPACMobileUserCSS being overriden by OPACUserCSS
Bug 9551 - OPACMobileUserCSS being overriden by OPACUserCSS
Bug 9551 - OPACMobileUserCSS being overriden by OPACUserCSS
Bug 9551 - OPACMobileUserCSS being overriden by OPACUserCSS

Description Ed Veal 2013-02-05 22:24:59 UTC
I have been playing with the OPACMobileUserCSS sys pref. I am trying to have separate information in the opaccredits sys pref for the mobile and the desktop views. I am doing this by giving the content unique id's and then using the appropriate css sys pref to hide or display the appropriate information. 

However, when I do this whatever is the OpacUserCSS overrides what is in the OPACMobileUserCSS. 

Ed
Comment 1 Kyle M Hall 2013-02-28 16:21:39 UTC Comment hidden (obsolete)
Comment 2 Galen Charlton 2013-03-12 22:51:47 UTC
I notice that the patch is for the CCSR theme, but the prog theme is affected as well.

I'm a bit concerned about this sort of divergence, and I suggest that any patch that affects display functionality like this *must* be done for prog, not just CCSR.
Comment 3 Kyle M Hall 2013-03-13 11:39:03 UTC Comment hidden (obsolete)
Comment 4 Kyle M Hall 2013-03-13 11:40:17 UTC
I completely agree. I think unless we require all changes to be implemented in both template sets, we are going to see a feature divergence ( in fact, I think it's already happening ).

(In reply to comment #2)
> I notice that the patch is for the CCSR theme, but the prog theme is
> affected as well.
> 
> I'm a bit concerned about this sort of divergence, and I suggest that any
> patch that affects display functionality like this *must* be done for prog,
> not just CCSR.
Comment 5 Bernardo Gonzalez Kriegel 2013-09-15 14:09:31 UTC
This patch does not apply, not in 3.10 as indicated, nor 3.12 or master.
Anyway, the problem is real and affects master, 3.12 and 3.10
If you can, please provide different patches for each version.
Comment 6 Mark Tompsett 2014-04-17 01:33:41 UTC
I noticed this patch doesn't apply still. However, the idea of the patch is pretty obvious: put the OPACMobileUserCSS before OPACUserCSS.

I noticed that the reason it doesn't apply is because of the bidi logic. I also noticed that the prog and ccsr themes handle the ordering of the bidi logic differently.

Karam Qubsi in bug 10017 fixed right-to-left in the ccsr theme.
Jared Camins-Esakov in bug 10405 added ids for the ccsr theme.
D Ruth Bavousett in bug 8061 applies a patch similar to 10017's to the prog theme.
Jared Camins-Esakov in bug 10405 added ids for the prog theme too.

mtompset@ubuntu:~/kohaclone$ git blame -L25,35 koha-tmpl/opac-tmpl/ccsr/en/includes/doc-head-close.inc
3229d8b1 (Frédérick Capovilla 2012-08-12 18:32:45 -0400 25) <meta
3229d8b1 (Frédérick Capovilla 2012-08-12 18:32:45 -0400 26) <link
2beeedaf (Karam Qubsi         2013-04-08 15:17:30 +0300 27) [% IF ( bidi ) %]
2beeedaf (Karam Qubsi         2013-04-08 15:17:30 +0300 28)     <link
2beeedaf (Karam Qubsi         2013-04-08 15:17:30 +0300 29) [% END %]
6b5960cc (Jared Camins-Esakov 2013-06-08 16:16:07 -0400 30) ...[% OPAC
6b5960cc (Jared Camins-Esakov 2013-06-08 16:16:07 -0400 31) ...[% OPACUserCSS
47ca7f8f (Chris Cormack       2013-02-25 15:39:02 +1300 32) [% IF persona %]
47ca7f8f (Chris Cormack       2013-02-25 15:39:02 +1300 33)  <link
47ca7f8f (Chris Cormack       2013-02-25 15:39:02 +1300 34) [% END %]
47ca7f8f (Chris Cormack       2013-02-25 15:39:02 +1300 35)

mtompset@ubuntu:~/kohaclone$ git blame -L25,35 koha-tmpl/opac-tmpl/prog/en/includes/doc-head-close.inc
493dcede (Chris Cormack       2013-02-11 22:34:20 +1300 25) [% END %]
6b5960cc (Jared Camins-Esakov 2013-06-08 16:16:07 -0400 26) ...[% OPAC
2880e76e (D Ruth Bavousett    2013-04-05 17:58:33 +0300 27) [% IF ( bidi ) %]
2880e76e (D Ruth Bavousett    2013-04-05 17:58:33 +0300 28)     <link
2880e76e (D Ruth Bavousett    2013-04-05 17:58:33 +0300 29) [% END %]
6b5960cc (Jared Camins-Esakov 2013-06-08 16:16:07 -0400 30) ...[% OPACUserCSS
c2fca223 (Owen Leonard        2008-04-15 13:49:48 -0500 31) <!-- yui js -->
5884fb10 (Chris Cormack       2011-04-10 20:38:30 +1200 32) <script
5884fb10 (Chris Cormack       2011-04-10 20:38:30 +1200 33) <script
5884fb10 (Chris Cormack       2011-04-10 20:38:30 +1200 34) <script
5884fb10 (Chris Cormack       2011-04-10 20:38:30 +1200 35) <script

As you can see, ccsr has bidi, OPACMobileUserCSS, and then OPACUserCSS.
The prog theme has OPACMobileUserCSS, bidi, and then OPACUserCSS.
Sorry the OPACMobileUserCSS is chopped off, but formatting nicely is a pain.

This bug as reported may no longer exist. However, there is most definitely something wrong. PROBLEM 1: What is the correct order of bidi and OPACMobileUserCSS?

PROBLEM 2: Bootstrap no longer has OPACMobileUserCSS. Is this correct?
Comment 7 Owen Leonard 2014-04-18 18:22:23 UTC
(In reply to M. Tompsett from comment #6)

> PROBLEM 2: Bootstrap no longer has OPACMobileUserCSS. Is this correct?

This is correct. The Bootstrap theme has one stylesheet which applies across screen sizes. There is no hard division between mobile and non-mobile.
Comment 8 Mark Tompsett 2014-04-19 05:42:44 UTC Comment hidden (obsolete)
Comment 9 Mark Tompsett 2014-04-19 05:46:48 UTC Comment hidden (obsolete)
Comment 10 Mark Tompsett 2014-04-19 05:49:05 UTC
I have left the original, and it does not apply. I have also attach two possibilities of solutions. Since I don't know which order they should be in, I will leave it to someone wiser to make the decision. I would think someone affected by the bidi logic should test and decide. I'm setting the status to In Discussion, since either patch 2 and patch 3 do apply.
Comment 11 Katrin Fischer 2014-11-02 11:32:36 UTC
The OPACMobileUserCSS has been removed with the prog/CCSR OPAC theme removal.
Comment 12 Mark Tompsett 2014-11-02 16:06:51 UTC
Given that prog is still in 3.14 and 3.16 (which are not obsolete yet), and that 3.18 has not yet been released, I do not see why this couldn't be left open for back-porting. I'm not sure RESOLVED INVALID is an appropriate state, Katrin.
Comment 13 Katrin Fischer 2014-11-02 16:08:41 UTC
This patch hasn't seen any attention since April and was in discussion, if my attempt to close this brings it back to life this is fine with me :) - if you want to provide patches for the older versions that's of course possible
Comment 14 Mark Tompsett 2014-11-02 16:43:24 UTC
Created attachment 33131 [details] [review]
Bug 9551 - OPACMobileUserCSS being overriden by OPACUserCSS

Test Plan:
1) Set a rule in OpacUserCSS that should be overidden by
   a rule set in OPACMobileUserCSS
2) Load OPAC, check for OpacUserCSS taking precedence
3) Apply patch
4) Reload OPAC, check for OPACMobileUserCSS taking precedence
Comment 15 Mark Tompsett 2014-11-02 17:23:11 UTC
This should apply cleanly to 3.16.x. However, I'm not certain how the ordering of the mobile before or after makes any difference, because when I use:
.silly { color:#FF0000; } and .silly { color:#00FF00; } in the two system preferences, I always seem to get non-mobile version when hunting for .silly in firebug. Can anyone confirm this is working or not?
Comment 16 Chris Cormack 2014-12-28 04:52:48 UTC
I think that since the theme has been deprecated since 3.16.x and been deleted since 3.18.x that there is very little point in working on this. Im going to set to resolved invalid.

If people want to patch their old system instead of upgrading, the code will still be here. But there's no point having it in the queue.
Comment 17 Mark Tompsett 2014-12-28 07:52:20 UTC
Well, I did change it to be for 3.16 and not master. I was thinking RESOLVED WONTFIX might be better, since it won't be changed in master. However, I'll leave it unchanged.