TPAC: Address some search syntax leaks in links
authorDan Scott <dscott@laurentian.ca>
Thu, 11 Oct 2012 04:56:40 +0000 (00:56 -0400)
committerBill Erickson <berick@esilibrary.com>
Tue, 30 Oct 2012 16:27:32 +0000 (12:27 -0400)
Expand the list of filtered characters to cover all of the special
characters documented for the Evergreen search grammar
(http://evergreen-ils.org/dokuwiki/doku.php?id=documentation:technical:search_grammar)
when generating links in the TPAC so as to avoid inadvertently launching
filtered searches when a user clicks on something that should just be a
display value.

For example, if a title includes "Presenting a subject: tips for
consultants", it should _not_ launch a search for "subject" containing
"tips for consultants".

This commit addresses most of the link problems in the record
display, as well as the author links in the search results table.

Still problematic are the facets (which seem to rely on exact matching,
such that filtering out the problematic characters is itself
problematic) and autocomplete (which requires modifying the Autocomplete
Dojo widget).

In addition, this commit makes the series code actually display, as it
was using a non-standard method to attempt to return the results from
the BLOCK (and failing). Also, it makes the links for authors in the
record details match the MODS32 definition for personal name parts and
only use the "acdq" subfields. This enables a click on the link to
actually return results; previously, in the case where the author field
included (for example) a subfield "g" value, that value would be
included in the generated link and would likely lead to 0 hits.

For authors, we substitute with a space rather than just eliding the
substituted value. Authors are particularly likely to have dates like
1899-1978; "1899 1978" matches, but "18991978" will not.

Perhaps we should take the same approach with the others, or break down
the search/replace logic a little further (for example, we could remove
the "-" only if it is preceded by a space or is at the start of the
string and is followed immediately by a character, and preserve it if it
is surrounded by digits). But this seems to take us pretty far down the
road of less negatively surprising results.

Signed-off-by: Dan Scott <dscott@laurentian.ca>
Signed-off-by: Bill Erickson <berick@esilibrary.com>
Open-ILS/src/templates/opac/parts/record/authors.tt2
Open-ILS/src/templates/opac/parts/record/series.tt2
Open-ILS/src/templates/opac/parts/record/subjects.tt2
Open-ILS/src/templates/opac/parts/result/table.tt2

index bc61e48..448fdf8 100644 (file)
@@ -36,10 +36,12 @@ BLOCK build_author_links;
                 tlabel = relators.$relcode || label;
             END;
             NEXT UNLESS code.match('[a-z]');
-            sf_raw = subfield.textContent;
             sf = subfield.textContent | html;
             term = term _ ' ' _ sf;
-            qterm = qterm _ ' ' _ sf_raw;
+            IF code.match('[acdq]');
+                sf_raw = subfield.textContent.replace('[#"^$\+\-,\.:;&|\[\]()]', ' ');
+                qterm = qterm _ ' ' _ sf_raw;
+            END;
         END;
         url = mkurl(ctx.opac_root _ '/results', {query => qterm, qtype => 'author'}, ['page', 'expand']);
         author_type = (tlabel || label) | html;
index 4695768..aa96243 100644 (file)
@@ -7,7 +7,7 @@
     results = [];
     FOR tag IN series_tags;
         FOR node IN ctx.marc_xml.findnodes('//*[@tag="' _ tag _ '"]/*');
-            node_uri = node.textContent | uri;
+            node_uri = node.textContent.replace('[#"^$\+\-,\.:;&|\[\]()]', '') | uri;
             node_html = node.textContent | html;
             IF !loop.first;
                 results.last = result.last _ '<span>&mdash;</span>';
             );
         END;
     END; 
+    FOR entry IN results;
+    -%]
+    <li class='rdetail_series_value'>[% entry %]</li>
+    [%- END;
 END;
 %]
 
@@ -25,8 +29,6 @@ END;
     IF series_anchors.length > 0; %]
 <h2 class='rdetail_related_series'>[% l('Search for related items by series') %]</h2>
 <ul>
-    [%- FOR entry IN series_anchors %]
-    <li class='rdetail_series_value'>[% entry %]</li>
-    [% END %]
+    [% series_anchors %]
 </ul>
 [%- END %]
index 52d9ac8..e92644d 100644 (file)
@@ -38,7 +38,7 @@
                 IF code.match('[vxyz]'); " &gt; "; END;
                 # at this point, we actually have a partial term to use.
                 single_term = subfield.textContent | html;
-                all_terms.push(subfield.textContent);
+                all_terms.push(subfield.textContent.replace('[#"^$\+\-,\.:;&|\[\]()]', ''));
                 total_term = all_terms.join(" ").replace('\s+$', '');
             %]
 <a href="[% mkurl(ctx.opac_root _ '/results', {qtype=>'subject', query=>total_term}, stop_parms); %]">[% single_term %]</a>
index 48f8be0..b63ec88 100644 (file)
@@ -59,7 +59,7 @@
                                                         <em><a title="[% l("Perform an Author Search") %]"
                                                                 name='item_author'
                                                                 href="[%- 
-                                                                    authorquery = attrs.author | replace('[,\.:;]', '');
+                                                                    authorquery = attrs.author | replace('[#"^$\+\-,\.:;&|\[\]()]', '');
                                                                     mkurl(ctx.opac_root _ '/results', {qtype => 'author', query => authorquery}, ['page'])
                                                                     -%]">[% attrs.author | html %]</a></em>
                                                                     [%- UNLESS CGI.param('detail_record_view')