The current code checks for '://' in the 856, and adds http:// or ftp://, depending on first indicator, if it isn't present. The marc standard, however, lists more access methods that are not tested for or covered. mailto: won't meet the test for '://' and should be handled
Created attachment 145999 [details] [review] Bug 32794: Expand tests for access method in XLST files This patch expands the checks for 856u to check for mailto, and adds a few conditiona checks of the first indicator to cover more methods Reference: https://www.oclc.org/bibformats/en/8xx/856.html To test: 1 - Edit a record and add a few 856 fields: 856 0 0 $umailto:test@example.org 856 0 0 $utest@example.org 856 2 0 $utelnet://pucc.princeton.edu 856 2 0 $upucc.princeton.edu 856 3 0 $u8885555555 2 - View staff details and results and opac details and results 3 - Note all links have been prepended with http:// 4 - Apply patch 5 - Refresh all views, ensure links are correctly formed
Created attachment 147745 [details] [review] Bug 32794: Expand tests for access method in XLST files This patch expands the checks for 856u to check for mailto, and adds a few conditiona checks of the first indicator to cover more methods Reference: https://www.oclc.org/bibformats/en/8xx/856.html To test: 1 - Edit a record and add a few 856 fields: 856 0 0 $umailto:test@example.org 856 0 0 $utest@example.org 856 2 0 $utelnet://pucc.princeton.edu 856 2 0 $upucc.princeton.edu 856 3 0 $u8885555555 2 - View staff details and results and opac details and results 3 - Note all links have been prepended with http:// 4 - Apply patch 5 - Refresh all views, ensure links are correctly formed Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
The "dial-up" one was particularly horrifying, since once I clicked through all the browser and OS warnings about "Dude, are you really clicking on a tel: link?" the only available handler app was Facetime. Thanks for choosing a number that wouldn't have gone through :)
This is a bit of teasing: <xsl:when test="@ind1=7"> <xsl:value-of select="marc:subfield[@code='2']"/><xsl:text>://</xsl:text> But when you would put mailto inside $2, this goes wrong again ;) Not really a blocker, just noting a fictive improvement.
From LOC 7 - Method specified in subfield $2 856 7#$3b&w film copy neg.$ddag$f3d01926$2file Too bad that this example does not contain the full prefix in $2. Would have been easy to just remove the suffix :// from our xslt.
Looking here / might add follow-up
Created attachment 148565 [details] [review] Bug 32794: Expand tests for access method in XLST files This patch expands the checks for 856u to check for mailto, and adds a few conditiona checks of the first indicator to cover more methods Reference: https://www.oclc.org/bibformats/en/8xx/856.html To test: 1 - Edit a record and add a few 856 fields: 856 0 0 $umailto:test@example.org 856 0 0 $utest@example.org 856 2 0 $utelnet://pucc.princeton.edu 856 2 0 $upucc.princeton.edu 856 3 0 $u8885555555 2 - View staff details and results and opac details and results 3 - Note all links have been prepended with http:// 4 - Apply patch 5 - Refresh all views, ensure links are correctly formed Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 148566 [details] [review] Bug 32794: (follow-up) Consolidate shared code into one function (:template) Few extra lines for the $2 exception as noted on Bugzilla. Note: We should probably check why the utils xslt on opac and intranet have so much differences currently. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 148567 [details] [review] Bug 32794: (follow-up) Replace code by new function (template) Test plan: Same as first patch. Bonus: Remove protocol from $u. Set first ind1 to 7. Put mailto in $2. Check link. Change $2 to tel. Check link. Change $2 to https. Check link. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(In reply to Marcel de Rooy from comment #4) > This is a bit of teasing: > > <xsl:when test="@ind1=7"> > <xsl:value-of > select="marc:subfield[@code='2']"/><xsl:text>://</xsl:text> > > But when you would put mailto inside $2, this goes wrong again ;) > Not really a blocker, just noting a fictive improvement. Ended up fixing it with three more lines.
Pushed to master for 23.05. Nice work everyone, thanks!
Nice work everyone! Pushed to stable for 22.11.x