We shouldn't really be showing any sort of configuration for modules that don't return a search name. This means they're just interacting with indexing and not actually providing a search. This fixes an issue where weird empty entries where showing up in the list.

CommentFileSizeAuthor
opensearch_empty_admin_links.patch1.42 KBneclimdul

Comments

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Postponed (maintainer needs more info)

Hello, and thanks for the report.

May you report a list of modules that don't return a valid name? AFAIK, modules implementing hook_search() should return a valid name.

neclimdul’s picture

neclimdul
He/Him
commented
Status: Postponed (maintainer needs more info) » Needs review

The two I came across are "apachesolr_attachments" and "location." Because of the way _search_menu(the access callback for search.module defined search pages) is setup, if a module does not return a true value for the 'name' op, then it doesn't get a page but can interact with all the other operations. This is the desired situation for modules like the ones I listed. I guess this is a bit of a documentation deficiency for hook_search().

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs review » Fixed

Thanks for your reply.
I altered the code as you suggested, and added the same check in another function. The code has been committed.

Thanks for your report.

neclimdul’s picture

neclimdul
He/Him
commented

Not sure how I suggested the code be changed as I think the original patch was fine. Just a note, empty($name) is actually unnecessary any inconsistent with _opensearch_is_search_page() or the core search.module tests. Something will be returned so there is no concern for it not being set.

Anonymous’s picture

Anonymous (not verified) commented
Status: Closed (fixed) » Fixed

!$name, and empty($name) are equivalent. The difference is that the second expression allows to avoid broken, or not well implemented search modules.
As per _opensearch_is_search_page(), the function is going to be removed.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.