#2069023: Fix reaction to disabled modules introduced search_api_system_info_alter().
Because of a call to l() it can make drupal_theme_initialize() run too early in the progress (before the full bootrap).
The hook is in my case invoked by features, but since side-wide content is generated with a theme specific function I think this is a bug in search_api.

Stripped version of the debug backtrace:

menu_get_custom_theme() <-- $initialize is FALSE by default, so this function will return NULL
drupal_theme_initialize()
l()
search_api_system_info_alter()
...
system_rebuild_module_data()
features_get_info()
...
feature_load()
...
drupal_path_initialize()
_drupal_bootstrap_full()
...

I attached a patch that fixes the problem for me. My proposed solution is to solve this the same way as usually done in hook_requirements() where the link is not created with l(), but by creating the a element directly.
I tried to create a test to reproduce the problem, but could not get it working. It looks like there is some other variable like a hook_preprocess_link() or something that is defined in one of the other modules I have enabled, and is not enabled on a clean testing environment.

Comments

Yaron Tal’s picture

Status: Active » Needs review
drunken monkey’s picture

Status: Needs review » Needs work

Thanks for posting this!
I didn't know half of this, but it makes sense and we should of course fix it. Hard-coding some links on an admin page doesn't seem too bad, I'd say.

However, the variables you include in the HTML strings should definitely be passed through check_plain().

Yaron Tal’s picture

Status: Needs work » Needs review
StatusFileSize
new1.01 KB

Oops, somehow forgot to copy that to the patch. Attached a new version with check_plain added.

Also besides displaying this on an admin page, this output could be shown on other pages with listings of modules or where info of a module is shown. This could be in every theme, while the output is created once and then cached. This could thus lead to a hook_preprocess_link of one theme running, while the output is shown in another theme.

drunken monkey’s picture

Status: Needs review » Needs work

With "variables" I meant both $name and $url. While rather improbably perhaps, it's still possible that the URL would contain special HTML characters. And better safe than sorry. (See also the last line of l().)

Yaron Tal’s picture

StatusFileSize
new1.03 KB

Added an extra check_plain() for $url.

Yaron Tal’s picture

Status: Needs work » Needs review
JvE’s picture

Ah, so this is why I kept getting the main theme in stead of the admin theme on my features pages.
Patch looks sane and solves the problem on one of my sites.

drunken monkey’s picture

Status: Needs review » Fixed

Excellent, thanks for testing, and thanks again for your work on this, Yaron Tal!

Committed.

drunken monkey’s picture

Including a follow-up fix of a bug I just spotted in your patch, i.e., that you use …/index/… for the server links, too. Of course I spotted that only after committing.

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

Issue summary: View changes

I think this is a bug in search_api, not in features.