http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11826 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|gmcharlt@gmail.com |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11826 --- Comment #1 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 25573 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=25573&action=edit Bug 11826: Add XSLT handler object to Koha namespace In achieving the goals of umbrella report 6536 (Z3950 improvements), this report adds a XSLT handler object to the Koha namespace. A follow-up adds unit tests for this module. This patch does not yet incorporate the new object into Z3950 search. So it does not change any behavior and is therefore harder to test. Test plan: Run the unit tests of the follow-up patch on report 11826. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> I tested the new module in two additional ways: [1] Made a simple script (four lines as below) that ran a larger xml file with 1500 records through a test XSLT file modifying the title. my $eng= Koha::XSLT_Handler->new; open my $fh1, '<', '_example_file'; my @xml= <$fh1>; print $eng->transform( join('', @xml), '_example_xsl_file'); [2] Incorporated the object into my local changes of Breeding.pm (based on older work, not ready for submittal now). Ran transformation on Z3950 search results. Worked well :) -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11826 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=6536 -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11826 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |11915 -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11826 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff --- Comment #4 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Magnus Enger from comment #3)

Kudos for splitting code into smaller patches! But I don't think it makes sense to add code to Koha before it is used somewhere - the bigger project might get abandoned and we are left with dead code. I'm setting this to "In discussion" and we can set it back to "Needs signoff" when the functionality that builds on this also needs signoff.

Back to Needs Signoff. Please look at the dependency. (Bug 11915.) You are most welcome to sign off :) -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11826 --- Comment #6 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 27230 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=27230&action=edit Bug 11826: Use xslt handler object in showmarc, Record.pm Modifies showmarc and opac-showmarc to use new xslt handler. Removes cardview.pl as obsolete script. Modifies C4/Record.pm and a typo in the test Record.t. Test plan: [1] catalogue/showmarc: Go to Cataloging. Search. Click Card. [2] opac-showmarc: Go to opac detail, MARC view. Open URL for plain view in new tab. Change URL: Change viewas=html to viewas=card [3] Verify that there are no references in the codebase to cardview.pl [4] C4/Record.pm: Run the Record.t test in db_dependent. This test uses marc2modsxml, triggering the change. Additional: export to MODS from opac-detail. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11826 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|11915 | -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11826 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|Small patch |Medium patch -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11826 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #25573|0 |1 is obsolete| | --- Comment #8 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Created attachment 27869 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=27869&action=edit [SIGNED-OFF] Bug 11826: Add XSLT handler object to Koha namespace In achieving the goals of umbrella report 6536 (Z3950 improvements), this report adds a XSLT handler object to the Koha namespace. A follow-up adds unit tests for this module. This patch does not yet incorporate the new object into Z3950 search. So it does not change any behavior and is therefore harder to test. Test plan: Run the unit tests of the follow-up patch on report 11826. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> I tested the new module in two additional ways: [1] Made a simple script (four lines as below) that ran a larger xml file with 1500 records through a test XSLT file modifying the title. my $eng= Koha::XSLT_Handler->new; open my $fh1, '<', '_example_file'; my @xml= <$fh1>; print $eng->transform( join('', @xml), '_example_xsl_file'); [2] Incorporated the object into my local changes of Breeding.pm (based on older work, not ready for submittal now). Ran transformation on Z3950 search results. Worked well :) Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Test runs Ok. No koha-qa errors. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11826 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #27229|0 |1 is obsolete| | --- Comment #10 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Created attachment 27871 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=27871&action=edit [SIGNED-OFF] Bug 11826: Using the XSLT handler in C4/XSLT module Incorporate the new object into C4/XSLT module. The handler object needed one adjustment to allow for passing URLs. Removed the GetURI function; it is no longer needed. Added some documentation lines. Moved a file-level lexical upwards in the code for visibility. Removed some tabs (on two lines). Note: The handler could perhaps be moved up to the Context module, or be saved in a global ('our') variable. But I would rather not do that now, making testing not too complex at this moment. Test plan: Enable XSLT preferences for opac and intranet. Check result and detail pages. Check individual list display (virtual shelves). Force an XSLT error by editing one of your xsl files. Check the corresponding display. Put an URL in one the XSLT prefs (could just be to your own server). Check the display. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Running XSLTParse4Display in a loop showed no significant performance change. One pass cost me around 0.012 sec (except for the first pass). Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Results, detail, url and list view Ok No koha-qa errors -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11826 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |bgkriegel@gmail.com -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11826 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_3_16_candidate | --- Comment #12 from Galen Charlton <gmcharlt@gmail.com> --- This looks promising for 3.18, but upon reflection, this has no urgent reason to bypass QA and go into 3.16.0, as there's no user-visible behavior change. Consequently, I'm removing the rel_3_16_candidate keyword. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11826 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=12172 -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11826 --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Ok, also looks like the pref is still relative to root in the overall filetree and not the root htdocs location, and so this is not a regression. This should be slightly better documented somewhere me thinks. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11826 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #27870|0 |1 is obsolete| | --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 28094 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=28094&action=edit [PASSED QA] Bug 11826: Add unit tests for Koha::XSLT_Handler Test plan: Verify if XSLT_Handler.t passes. You could also sabotage the test by removing one of the test xsl files. Or you could 'repair' the bad xsl file (test02). Remove the second line redefining the xsl variable. In all those cases the unit test should fail now.. Discard your changes :) Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Test pass. No koha-qa errors. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11826 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #27872|0 |1 is obsolete| | --- Comment #19 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 28096 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=28096&action=edit [PASSED QA] Bug 11826: Use xslt handler object in showmarc, Record.pm Modifies showmarc and opac-showmarc to use new xslt handler. Removes cardview.pl as obsolete script. Modifies C4/Record.pm and a typo in the test Record.t. Test plan: [1] catalogue/showmarc: Go to Cataloging. Search. Click Card. [2] opac-showmarc: Go to opac detail, MARC view. Open URL for plain view in new tab. Change URL: Change viewas=html to viewas=card [3] Verify that there are no references in the codebase to cardview.pl [4] C4/Record.pm: Run the Record.t test in db_dependent. This test uses marc2modsxml, triggering the change. Additional: export to MODS from opac-detail. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Views Ok. Test pass. No more cardview. No koha-qa errors Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11826 --- Comment #21 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- *** Bug 12172 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11826 --- Comment #23 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Martin Renvoize from comment #20)

3. I believe this enforces a more strict behaviour when using remote stylesheets. (I found that pre patch, you could use the directory path in the Utils import within your remote sheet and it would be relative to root on the local system. However, after the patch, I had to also host the Utils file remotely and link import that one in the remote stylesheet)

A relative path in the remote stylesheet is resolved within the remote current working directory. The old behavior of reading the text with GetURI and resolving the import path against the root of the local system was a side-effect that we should not want [to rely on].

4. It would be 'nice' to have some warning added to enable tracking through errors when a sheets fails to load. however, handling here is preferable to existing behaviour of falling back to an unhandled error in the browser.

I am attaching a QA follow-up that adds printing warns (via STDERR to the logfile) by default. Actually, that was former implicit behavior. You can now also disable that by saying $engine->print_warns(0). -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11826 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #25 from Galen Charlton <gmcharlt@gmail.com> --- Pushed to master. Thanks, Marcel! -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11826 --- Comment #27 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to David Cook from comment #26)

I wonder if it would be useful to add a method for registering Perl functions with the XML::LibXSLT object.

I suppose we wouldn't want staff clients users to inject Perl code, so it would only be useful if we wanted to provide certain functions. I've used this before in the XSLTs to encode URIs and stuff like that.

I suppose if we need to register functions, we can just do it on a case by case basis with whatever xslt handle is returned.

Interesting idea. I did not use these extensions functions, but I can imagine that some things are better done in Perl. Why not open a new report Register functions in XSLT_Handler? You can do it :) -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11826 --- Comment #29 from David Cook <dcook@prosentient.com.au> --- (In reply to M. de Rooy from comment #27)

(In reply to David Cook from comment #26)

...

I wonder if it would be useful to add a method for registering Perl functions with the XML::LibXSLT object.

I suppose we wouldn't want staff clients users to inject Perl code, so it would only be useful if we wanted to provide certain functions. I've used this before in the XSLTs to encode URIs and stuff like that.

I suppose if we need to register functions, we can just do it on a case by case basis with whatever xslt handle is returned.

Interesting idea. I did not use these extensions functions, but I can imagine that some things are better done in Perl. Why not open a new report Register functions in XSLT_Handler? You can do it :)

After reading through the XML::LibXSLT documentation again (and some of the libxslt documentation), I've realized that it isn't necessary to register extension functions via the XML::LibXSLT object itself (and thus the XSLT_Handler object). "XML::LibXSLT->register_function($uri, $name, $subref)" registers extension functions "globally", which can be done at the class level rather than the object level. That is, after you load the XML::LibXSLT class module, you can register functions regardless of the XSLTs that you need to parse or the objects you use to do so. If I understand correctly, it's because the XML::LibXSLT is just an interface to the libxslt C library. So you're not registering the extension function with an object but rather the actual XSLT engine itself. It might be worth noting that the documentation says this might not be thread or fork safe. I don't know what consequences that will have for folks running with Plack. I am guessing that every extension function you register will stick around until you restart Plack. I don't know if this will have any performance consequences. Extension functions are pretty common (they get used all the time in DSpace with the Xalan engine). So I don't know if there will be any negative consequences, but perhaps something to keep in mind. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11826 --- Comment #31 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to David Cook from comment #30)

I have however opened a bug to add parameters to the XSLT_Handler transform method.

This allows you to pass a hash of key => value pairs to use in a XSLT as parameters. I've used this in my OAI Harvesting code to pass in the unique record identifier from the OAI-PMH response to the XSLT so I can add it to the MARCXML that gets added to Koha.

Bug 13262 - Add parameters to XSLT Handler transform method http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13262

In SO now. -- You are receiving this mail because: You are watching all bug changes.