Summary: | OPACMobileUserCSS being overriden by OPACUserCSS | ||
---|---|---|---|
Product: | Koha | Reporter: | Ed Veal <edward.m.veal> |
Component: | OPAC | Assignee: | Mark Tompsett <mtompset> |
Status: | CLOSED INVALID | QA Contact: | |
Severity: | minor | ||
Priority: | P5 - low | CC: | bgkriegel, chris, gmcharlt, kyle, mtompset, nengard |
Version: | 3.16 | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | Trivial patch | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: | Circulation function: | ||
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
Created attachment 15780 [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 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. Created attachment 16087 [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 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. 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. 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? (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. Created attachment 27297 [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 NOTE: This patch version is bidi logic first. ccsr is unchanged, prog changes to put bidi first. Created attachment 27298 [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 NOTE: This version puts the OPACMobileUserCSS before the bidi logic. This means ccsr changes, but prog does not. 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. The OPACMobileUserCSS has been removed with the prog/CCSR OPAC theme removal. 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. 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 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 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? 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. 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. |