[Bug 13170] New: Remove of prog theme broke the OPAC's "view plain" option for MARC details
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13170 Bug ID: 13170 Summary: Remove of prog theme broke the OPAC's "view plain" option for MARC details Change sponsored?: --- Product: Koha Version: master Hardware: All URL: /cgi-bin/koha/opac-showmarc.pl?id=XXX&viewas=html OS: All Status: NEW Severity: major Priority: P5 - low Component: Templates Assignee: tomascohen@gmail.com Reporter: oleonard@myacpl.org QA Contact: testopia@bugs.koha-community.org In the OPAC if you view the MARC details for a title (and have OPACXSLTDetailsDisplay enabled) there is a "view plain" link which displays the output of opac-showmarc.pl. This is broken in master: opac-showmarc.pl: XSLT file not found. at kohaclone/Koha/XSLT_Handler.pm git bisect blames Bug 12233: Remove the prog and ccsr themes files. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13170 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de, | |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13170 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13170 --- Comment #1 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 33128 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33128&action=edit Bug 13170 Remove of prog theme broke the OPAC's "view plain" option for MARC details In the OPAC if you view the MARC details for a title (and have OPACXSLTDetailsDisplay enabled) there is a "view plain" link which displays the output of opac-showmarc.pl. This is broken in master: fixed by this patch. Test plan: (1) Set OPACXSLTDetailsDisplay to default (2) Do a search on OPAC, then display a specific biblio record (3) Click on MARC view tab. Then click on 'view plain' link. Nothing is displayed. (4) Apply the patch. And refresh the MARC detail page. (5) Click on 'view plain' link. Check that a plain text MARC record is displayed. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13170 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |frederic@tamil.fr Assignee|tomascohen@gmail.com |frederic@tamil.fr -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13170 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Frederic, I'm on the phone right now, so cannot test. But could you make the patch use C4::Templates::themelanguage instead so we use the fallback logic from only one place? It does exactly what you want. Thanks! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13170 --- Comment #3 from Frédéric Demians <frederic@tamil.fr> --- (In reply to Tomás Cohen Arazi from comment #2)
Frederic, I'm on the phone right now, so cannot test. But could you make the patch use C4::Templates::themelanguage instead so we use the fallback logic from only one place? It does exactly what you want. Thanks!
Yes, of course. Thanks for pointing to the right direction! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13170 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33128|0 |1 is obsolete| | --- Comment #4 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 33129 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33129&action=edit Bug 13170 Remove of prog theme broke the OPAC's "view plain" option for MARC details In the OPAC if you view the MARC details for a title (and have OPACXSLTDetailsDisplay enabled) there is a "view plain" link which displays the output of opac-showmarc.pl. This is broken in master: fixed by this patch. Note: Implemented with C4::XSLT::_get_best_default_xslt_filename() to keep code orthogonal. Test plan: (1) Set OPACXSLTDetailsDisplay to default (2) Do a search on OPAC, then display a specific biblio record (3) Click on MARC view tab. Then click on 'view plain' link. Nothing is displayed. (4) Apply the patch. And refresh the MARC detail page. (5) Click on 'view plain' link. Check that a plain text MARC record is displayed. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13170 --- Comment #5 from Chris Cormack <chris@bigballofwax.co.nz> --- Comment on attachment 33129 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33129 Bug 13170 Remove of prog theme broke the OPAC's "view plain" option for MARC details Review of attachment 33129: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=13170&attachment=33129) ----------------------------------------------------------------- ::: opac/opac-showmarc.pl @@ +57,5 @@
+ C4::Context->config('opachtdocs'), + C4::Context->preference('opacthemes'), + C4::Languages::getlanguage($input), + $view eq 'card' ? 'compact.xsl' : 'plainMARC.xsl' + );
Why are we doing this? Instead of using C4::Template::themelanguage ? Now we still have different behaviour for xslt and for normal templates themelanguage will never fallback to prog for the opac. however _get_best_defautl_xslt_filename will. So if have a new theme chrisc theme. And I make all the stuff, but no new xslt file. It will now fall back to prog, instead of bootstrap. This is a problem as prog no longer exists in the OPAC. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13170 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |chris@bigballofwax.co.nz -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13170 --- Comment #6 from Frédéric Demians <frederic@tamil.fr> --- (In reply to Chris Cormack from comment #5)
Why are we doing this? Instead of using C4::Template::themelanguage ?
Now we still have different behaviour for xslt and for normal templates
themelanguage will never fallback to prog for the opac. however _get_best_defautl_xslt_filename will.
So if have a new theme chrisc theme. And I make all the stuff, but no new xslt file. It will now fall back to prog, instead of bootstrap.
This is a problem as prog no longer exists in the OPAC.
Ok. I see. I forgot to modify _get_best_default_xslt_filename to fall back to 'bootstrap' theme rather than 'prog' for OPAC interface. There is a difference between C4::Template::themelanguage & C4::XSLT::_get_best_default_xslt_filename. 'themelanguage' search tempalates in subdirectories of directory named 'modules'. '_get_best_default_xslt_filename' search XSL file in a subdirectory named 'xslt'. Would you suggest to factorize both functions in one? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13170 --- Comment #7 from Chris Cormack <chris@bigballofwax.co.nz> --- (In reply to Frédéric Demians from comment #6)
(In reply to Chris Cormack from comment #5)
Why are we doing this? Instead of using C4::Template::themelanguage ?
Now we still have different behaviour for xslt and for normal templates
themelanguage will never fallback to prog for the opac. however _get_best_defautl_xslt_filename will.
So if have a new theme chrisc theme. And I make all the stuff, but no new xslt file. It will now fall back to prog, instead of bootstrap.
This is a problem as prog no longer exists in the OPAC.
Ok. I see. I forgot to modify _get_best_default_xslt_filename to fall back to 'bootstrap' theme rather than 'prog' for OPAC interface.
It should fall back to whateverr OPACFallback is set to. Currently thats bootstrap, but we dont want to be in this same position when we change default theme again sometime in the future of having hardcoded theme names.
There is a difference between C4::Template::themelanguage & C4::XSLT::_get_best_default_xslt_filename. 'themelanguage' search tempalates in subdirectories of directory named 'modules'. '_get_best_default_xslt_filename' search XSL file in a subdirectory named 'xslt'.
Would you suggest to factorize both functions in one?
Something like that, a single place where we pick the theme and language seems to be a lot less error prone and easier to maintain. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13170 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13170 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33129|0 |1 is obsolete| | --- Comment #8 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 33179 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33179&action=edit Bug 13170 Remove of prog theme broke the OPAC's "view plain" option for MARC details In the OPAC if you view the MARC details for a title (and have OPACXSLTDetailsDisplay enabled) there is a "view plain" link which displays the output of opac-showmarc.pl. This is broken in master: fixed by this patch. Note: Implemented with C4::XSLT::_get_best_default_xslt_filename() to keep code orthogonal. Test plan: (1) Set OPACXSLTDetailsDisplay to default (2) Do a search on OPAC, then display a specific biblio record (3) Click on MARC view tab. Then click on 'view plain' link. Nothing is displayed. (4) Apply the patch. And refresh the MARC detail page. (5) Click on 'view plain' link. Check that a plain text MARC record is displayed. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13170 --- Comment #9 from Chris Cormack <chris@bigballofwax.co.nz> --- Comment on attachment 33179 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33179 Bug 13170 Remove of prog theme broke the OPAC's "view plain" option for MARC details Review of attachment 33179: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=13170&attachment=33179) ----------------------------------------------------------------- ::: C4/XSLT.pm @@ +145,4 @@
my @candidates = ( "$htdocs/$theme/$lang/xslt/${base_xslfile}", # exact match "$htdocs/$theme/en/xslt/${base_xslfile}", # if not, preferred theme in English + "$htdocs/bootstrap/$lang/xslt/${base_xslfile}", # if not, 'bootstrap' theme in preferred language
Hmm nope, still not the same behaviour in themelanguage we do my $fallback_syspref = ($interface eq 'intranet') ? 'template' : 'OPACFallback'; C4::Context->preference( $fallback_syspref ) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13170 --- Comment #10 from Frédéric Demians <frederic@tamil.fr> ---
It should fall back to whateverr OPACFallback is set to. Currently thats bootstrap, but we dont want to be in this same position when we change default theme again sometime in the future of having hardcoded theme names.
Sorry. I wasn't even aware of this syspref.
Something like that, a single place where we pick the theme and language seems to be a lot less error prone and easier to maintain.
Will try. Thanks.
Hmm nope, still not the same behaviour
in themelanguage we do
my $fallback_syspref = ($interface eq 'intranet') ? 'template' : 'OPACFallback'; C4::Context->preference( $fallback_syspref )
I couldn't agree with you more. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13170 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33179|0 |1 is obsolete| | --- Comment #11 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 33180 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33180&action=edit Bug 13170 Remove of prog theme broke the OPAC's "view plain" option for MARC details In the OPAC if you view the MARC details for a title (and have OPACXSLTDetailsDisplay enabled) there is a "view plain" link which displays the output of opac-showmarc.pl. This is broken in master: fixed by this patch. Note: Implemented with C4::XSLT::_get_best_default_xslt_filename() to keep code orthogonal. Test plan: (1) Set OPACXSLTDetailsDisplay to default (2) Do a search on OPAC, then display a specific biblio record (3) Click on MARC view tab. Then click on 'view plain' link. Nothing is displayed. (4) Apply the patch. And refresh the MARC detail page. (5) Click on 'view plain' link. Check that a plain text MARC record is displayed. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13170 --- Comment #12 from Chris Cormack <chris@bigballofwax.co.nz> --- Excellent this looks good, Ill do a proper test tomorrow (i might edit the commit message to remove mention of the sub we aren't using now). But from a code review it looks good. Just to do some regression testing now. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13170 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13170 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33180|0 |1 is obsolete| | --- Comment #13 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 33181 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33181&action=edit Bug 13170 Remove of prog theme broke the OPAC's "view plain" option for MARC details In the OPAC if you view the MARC details for a title (and have OPACXSLTDetailsDisplay enabled) there is a "view plain" link which displays the output of opac-showmarc.pl. This is broken in master: fixed by this patch. Test plan: (1) Set OPACXSLTDetailsDisplay to default (2) Do a search on OPAC, then display a specific biblio record (3) Click on MARC view tab. Then click on 'view plain' link. Nothing is displayed. (4) Apply the patch. And refresh the MARC detail page. (5) Click on 'view plain' link. Check that a plain text MARC record is displayed. Signed-off-by: Chris <chris@bigballofwax.co.nz> Note: This makes a small change to C4::Templates::themelanguage so that it works with .xsl files too (They live in the xslt dir) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13170 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13170 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33181|0 |1 is obsolete| | --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 33182 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33182&action=edit [PASSED QA] Bug 13170 Remove of prog theme broke the OPAC's "view plain" option for MARC details In the OPAC if you view the MARC details for a title (and have OPACXSLTDetailsDisplay enabled) there is a "view plain" link which displays the output of opac-showmarc.pl. This is broken in master: fixed by this patch. Test plan: (1) Set OPACXSLTDetailsDisplay to default (2) Do a search on OPAC, then display a specific biblio record (3) Click on MARC view tab. Then click on 'view plain' link. Nothing is displayed. (4) Apply the patch. And refresh the MARC detail page. (5) Click on 'view plain' link. Check that a plain text MARC record is displayed. Signed-off-by: Chris <chris@bigballofwax.co.nz> Note: This makes a small change to C4::Templates::themelanguage so that it works with .xsl files too (They live in the xslt dir) Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Works as described, passes tests and QA script. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13170 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #15 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patch pushed to master. Thanks Frédéric! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org