Posted by Alan D. on

Just wondering if this is actually needed, and it also causes integration issues with the menu position module (and probably others). Maybe special_menu_items_page_alter() instead? Something late rather than early.

Files: 
CommentFileSizeAuthor
#1 special_menu_items-hook_init.patch848 bytesDamienMcKenna

Comments

DamienMcKenna’s picture

DamienMcKenna CreditAttribution: DamienMcKenna commented
Title:Is hook_init() needed?» Replace hook_init() code
Category:Support request» Feature request
Issue tags:+Performance
StatusFileSize
new special_menu_items-hook_init.patch848 bytes

For one site we just removed the breadcrumb trail integration entirely, because breadcrumbs weren't being used.

DamienMcKenna’s picture

DamienMcKenna CreditAttribution: DamienMcKenna commented

FYI Dane Powell wrote the patch, I'm just uploading it.

Alan D.’s picture

Alan D. CreditAttribution: Alan D. commented

lol, so three things

1) Why is this needed?

2) Change to a latter hook, page alter, preprocess page or something as per the reason given to 1

3) Make configurable with a toggle variable_get('special_menu_item_set_breadcrumbs', 1) + uninstall hook. I recon that no admin UI required, rather let users edit settings.php or exec php to set this. i.e. in settings.php use $conf['special_menu_item_set_breadcrumbs'] = 1; to disable breadcrumb integration or something like this in the read me file.

DamienMcKenna’s picture

DamienMcKenna CreditAttribution: DamienMcKenna commented
Status:Active» Needs work

The appropriate status.

I'll ask Dane to respond, he'd know the details on why the code needed to be removed, beyond the blanket statement of "performance!"

Dane Powell’s picture

Dane Powell CreditAttribution: Dane Powell commented

"Because performance" is part of it- I agree with this comment made when the hook_init was added: #573034-13: Breadcrumbs still show links.

hook_init() is way too soon to do this. I'd move this to hook_process_breadcrumb() which is as late as possible.

But mostly we saw some reports that it might cause trouble (in the OP here and in #489964: Breaks the block admin page when separate admin theme is used), and we didn't want to take any chances.

Teenage’s picture

Teenage CreditAttribution: Teenage commented

Hi,
I'm quite interested about the performance issue.
I get slow response time on a web service path and I notice more than half the bootstrap time was spend on special_menu_items_init which is strange for a "MENU_CALLBACK" hook_menu type.

I think this code should be moved for later execution and triggered only if needed ?

Drupal 6
Duration : 2,5s total, 2.5s for bootstrap (including 1s for special_menu_items_init) and 1ms for my hook_menu

pc-wurm’s picture

pc-wurm CreditAttribution: pc-wurm commented
Title:Replace hook_init() code» Replace or remove hook_init() code
Status:Needs work» Needs review

I am also in favor of removing this hook_init implementation, since it takes more than 800ms on Drupal 7.36 and it doesn't even bring much, if ever. After removing this hook, I don't see any changes on how the breadcrumbs displayed, but didn't test it on a clean Drupal. So I guess it is just unnecessary, and impacting the performance in an unacceptable way.