Bugzilla – Bug 17373
playing or adding "All Songs" after searching with a search term including a period is broken
Last modified: 2016-03-10 08:05:11 UTC
Using the Web Classic Interface, search for a Song with a full-stop (period) in the name. This is a common requirement for classical songs. The list of songs returned is correct, but any attempt to select and play one produces a random result. See also the forum - http://forums.slimdevices.com/showthread.php?s=a2042b1a47a6b121dd2f0434fbf25a4a&p=645129#post645129 where this has been confirmed by others. A work-around is to replace the full stop with a space. I do not know if this fault existed in earlier versions of the server, but it is definitely in 7.6 on Windows 7 (64 bit).
== Auto-comment from SVN commit #33242 to the slim repo by mherger == == http://svn.slimdevices.com/slim?view=revision&revision=33242 == Fixed Bug: 17373 Description: remove period from search expression, as it breaks our url index. It is ignored during the search anyway.
Unfortunately this fix prevents the xmlbrowser search mechanism being used for passing more general strings such as urls to browsing code. It looks like it breaks my use of this function to allow http://open.spotify.com urls to be searched for instance. Could we look for a more generic way of encoding the index and search so we don't need to break this?
> Could we look for a more generic way of encoding the index and search so we > don't need to break this? Definitely. I think I even mentioned this to Alan when I did that change. Do you want to take a stab?
The current fix for this is causing problems for multple plugins as they use the search field to enter urls and other text which should not be modified. Can you confirm where the original bug applied - is it only for playing the results of searches with periods in it and from which web interfaces (Classic or Default?) Which specific search box are you using - the one on the top level of the web interface?
Michael - potential alternative fix, what do you think? Index: Slim/Web/XMLBrowser.pm =================================================================== --- Slim/Web/XMLBrowser.pm (revision 33764) +++ Slim/Web/XMLBrowser.pm (working copy) @@ -277,9 +277,6 @@ my $searchQuery; if ( $subFeed->{'type'} && $subFeed->{'type'} eq 'search' && defined $stash->{'q'} ) { - # bug 17373 - remove period from search expression, as it breaks our index (and is ignored during the search anyway) - $stash->{q} =~ s/\./ /g; - $crumbText .= '_' . uri_escape_utf8( $stash->{q}, "^A-Za-z0-9" ); $searchQuery = $stash->{'q'}; } @@ -1186,8 +1183,14 @@ return; } + # Bug 17373: rewrite the index arg from raw uri if present as it will already be unescaped which breaks search encoding of index + # (this is the same issue as bug 17181) + my ($itemId) = ($response->request->uri =~ m%index=(.*?)&%); + if ($itemId) { + $args->{'index'} = $itemId; + } + my ($index, $quantity) = (($args->{'start'} || 0), ($args->{'itemsPerPage'} || $prefs->get('itemsPerPage'))); - my $itemId = $args->{'index'}; if (defined $itemId) { my $i = $itemId; $i =~ s/^(?:[a-f0-9]{8})?\.?//; # strip sessionid if present
Alan - do you want to review this as Michael is out? The current "fix" breaks functionality in several of my plugins so I am keen to fix in another way! (Plugins from me and others use the search dialog to allow urls to be entered so removing periods from the string is a bad idea!)
Apologies - I received an email about this a while ago, and replied, but it seems the reply didn't get delivered. I was asked a couple of questions, to which I attempted to reply: It was Classic web interface, top level. I imagine it applied to Default as well butI don't think I tested it there. As the original report said, the list returned was initially correct but then playing one of the songs gave completely the wrong result. Patching the full stop to a space should have been only a temporary fix until a fix for the underlying problem was found. Tim
Adrian - feel free to commit. Thanks!
== Auto-comment from SVN commit #33811 to the slim repo by adrian == == http://svn.slimdevices.com/slim?view=revision&revision=33811 == Bug: 17373 Description: fix bug in an alternative way which does not corrupt the search string
== Auto-comment from SVN commit #33821 to the slim repo by adrian == == http://svn.slimdevices.com/slim?view=revision&revision=33821 == Bug: 17373 Description: revert change 33811 as it breaks other search results
*** This bug has been marked as a duplicate of bug 16370 ***