The solution to #497206: Avoid search conflicts with other forms, use menu API instead of search_get_keys() gets search keys via the menu API rather than search_get_keys() -- however, this broke searches that use a slash in them, which is a regression. E.g., http://drupal.org/search/apachesolr_search/05/11/2010
I suspect that %menu_tail in search_menu() is not working as excepted.
BTW, I discovered this via the tests for #74673: Search does not correctly handle large numbers (> maxint) so you cannot search by UPC code for example so if that gets in this will be covered.
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | fix_searches_with_slash.patch | 713 bytes | berdir |
Comments
Comment #1
mcarbone commentedAh, to recreate, simply search for something with a slash, and you'll see that Drupal cuts off everything past the first slash. E.g., search/node/12/34/56789
Comment #2
mcarbone commentedRelated:
#298561: menu_tail should automatically act as a load function as well to allow slashes in search
#93854: Allow autocompletion requests to include slashes
#600424: search.module pulls arguments directly from $_GET rather than using the menu system
Comment #3
berdirThe attached patch fixes this. Not sure if it's what we want, but it works :)
Basically, the menu api passes strings separated with a / as separate arguments to the function. What this patch does is using func_get_args() to get all arguments and merges them together again.
I don't think this has anything to do with %menu_trail, that's just a _to_arg function that provides a default value for the menu callback. It doesn't do anything when loading/executing the router entry.
Comment #4
drunken monkeyPatch seems to work, except when searching for something like "foo/". In that case, the trailing slashes are stripped.
But since this is a very improbable case and everything else seems to work fine, I'd vote +1.
Comment #5
mcarbone commentedI think this is going to have to be fixed by fixing menu_tail, as the whole purpose of #497206: Avoid search conflicts with other forms, use menu API instead of search_get_keys() was to move search keys into the menu API. It would make more sense to roll that back, I think, then to commit this solution, which is essentially bringing back search_get_keys directly. I'd like to hear the input of some of those folks who were involved with that patch.
Comment #6
chx commentedI am working on adding menu_tail_load. Stay tuned.
Comment #7
moshe weitzman commentedhttp://drupal.org/node/45111
Comment #8
mcarbone commentedDo we really intentionally ship with regressions?
Comment #9
moshe weitzman commentedWe ship with bugs. Sometimes new ones, sometimes old friends. Again, read the priority documentation.
Comment #10
mcarbone commentedI have; I'm not suggesting reverting your change. In a perfect world, this would've failed a test and so would be critical, but of course that applies to all bugs. (Though I can see the headline in the New York Post now: "Drupal 7 ships with the anti-American inability to search for 9/11!!!")
Comment #11
chx commentedI think #298561-3: menu_tail should automatically act as a load function as well to allow slashes in search fixes it.
Comment #12
chx commentedGiven the test i wrote for that, i am closing this a a duplicate.