This is a fantastic module, and has saved me a ton of hassle.
I've run into an issue where it seems that the active trail on the search results page is being set based on the search results. This doesn't happen for the same search using the standard Drupal site search, and it doesn't happen for all search terms, but some terms cause the primary nav to register the wrong path as the active trail. It appears that it might be the path to the last displayed search result (i.e., result 10 when results 1 through 10 are shown), but I'm not 100% sure.
Any thoughts about what might be causing this, and how it can be fixed?
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | 1040002-5.patch | 8.76 KB | jhodgdon |
| #4 | 1040002-4.patch | 8.57 KB | jhodgdon |
| #2 | 1040002.patch | 1.32 KB | jhodgdon |
Comments
Comment #1
jhodgdonSounds like a bug indeed. What happens is that in many cases, Search by Page has to render each result's page in order to display an excerpt from the page, and in many cases, the rendering causes all sorts of side effects (resetting the page title, resetting bookmarks, etc.). I tried to take those side effects into account in the module, but I must have missed the active trail. I'll see what I can do. Thanks for reporting it!
Comment #2
jhodgdonThat was it. The module was preserving pager information, page title, and breadcrumb, but not the active trail. I've added the active trail to the list. Here's the patch for the D6 version that should work. I am going to make a test for this and then commit it to the development branch for both D6 and a similar fix for D7 (along with a fix for another issue).
But meanwhile, if you could try this patch, that would be helpful.
Comment #4
jhodgdonI'm still working on this along with
#1018764: Panels page renders page header twice when it's a search result with excerpt
The patch above passes all of my tests on my test box, so I'm going to see if I can figure out why drupal's test box has so many failures. Here's a new patch that includes a fix to the other issue, a test for both issues, and some other test fixes that should erase at least some of the failures that the Drupal test bot is seeing.
Comment #5
jhodgdonOne more slightly different patch, and changing the version on this issue so I make sure the testing bot runs against the development version.
Comment #6
jhodgdonI've just checked a fix for this into the DEV branch for 6.x. I'll also be porting it to 7.x. It's basically the patch above in #5, with a few more PHP warning fixes added.
If you can test and see if this fixes your problem, that would be helpful. You should be able to download the dev version of the module within 12 hours or so.
Comment #7
jhodgdonThis fix is also done in the Drupal 7 dev version now.
I'll go ahead and mark this issue as fixed, until I hear otherwise, as I"m pretty sure this change will fix the problem.
Comment #8
noah commentedSorry for not responding sooner. I don't have a dev version of the module running anywhere right now, but I'll try to get one installed and report back what I find.
Comment #10
thugsb commentedThis is still giving active-trail for me, on Drupal 7 with the dev version. I've cleared cache, disabled and re-enabled, etc., and it's still active-trail. Any ideas?
Comment #11
jhodgdonWhat do you mean "this is still giving active-trail for me"?
Comment #12
thugsb commentedMy menu items still get the class "active-trail" when the respective page is in the results.
Comment #13
jhodgdonYou said you are using Drupal 7, right? In which case, please leave the version at 7.x-1.x-dev. If you are using Drupal 6, let me know. I will wait to hear which one you are using.
Comment #14
thugsb commentedI am using Drupal 7, and the 7.x-1.x-dev.
Comment #15
thugsb commentedSorry about how I keep accidentally switching the version!
Comment #16
jhodgdonPlease send me the information requested in the last email and I'll see if I can figure this out...
Comment #17
kirill.volkov commentedI applied patch from #5 but still the issue of menu trail being set to the last item in search results. I'm using search_by_page 6.x-1.14 and also menutrails module. I had a look at how menutrails sets the active menu item and it uses
menu_set_item(NULL, $item);So in_search_by_page_do_search()I added$tmp_item = menu_get_item();to save current item andmenu_set_item(NULL, $tmp_item);to change it back. Seems to work.Comment #18
jhodgdonAh. You didn't mention before (or I didn't notice) that you are using the Menu Trails module.
The proposed solution seems to be a reasonable idea though, even if it is only a solution for Menu Trails users. I will add that to the code as soon as possible. Thanks!
Comment #19
jhodgdonComment #20
Anonymous (not verified) commentedThis bug still appears to be an issue, for me at least. The problem occurs when setting the main search module to use Search By Page as the primary, and displaying snippets rather than teasers. Using teasers seems to not set active trail incorrectly. Active trail seems to be set for every returned result. If three results are returned all three of those items are set to active in the menu system, resulting in three active trails. I was using 7.x-1.1 and switched to 7.x-1.x-dev in an effort to fix, but no go.
Comment #21
jhodgdonI haven't added the patch to the Dev version yet, sorry! You can try what was suggested in #17. I will mark the issue "fixed" once it's added to the Dev version.
Comment #22
Anonymous (not verified) commentedWhoops, though I'd read up there that it was in the dev branch. Sorry for the confusion!
Comment #23
jhodgdonI have just added this change from #17 to the Drupal 6 and 7 versions of Search by Page (development version only so far, and if you are downloading the -dev archives, you might need to wait 12 hours to make sure drupal.org has built a new zip file). If you could test and verify that it fixes your problems, I would be grateful!