Download & Extend
Issues

'block_callback' in menu_router table seems to have no purpose

Posted by joachim on October 16, 2010 at 5:10pm
Project:Drupal core
Version:7.x-dev
Component:menu system
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed (fixed)

Issue Summary

The block_callback field in the {menu_router} table seems to be a curious leftover from pre-D6 at least.

It's only used in system_main_admin_page():

      if ($item['block_callback'] && function_exists($item['block_callback'])) {
        $function = $item['block_callback'];
        $block['content'] .= $function();
      }

There's not a single item in my menu_router table that has this set. On D6, there's only one, and it's not even relevant to system_main_admin_page()!

I reckon we can get rid of this.

Login or register to post comments

Comments

#1

Posted by joachim on October 16, 2010 at 5:11pm
Status:active» needs review

Let's see what the testbot makes of this ;)

AttachmentSizeStatusTest resultOperations
943558.drupal.block_callback-test.patch1015 bytesIdlePASSED: [[SimpleTest]]: [MySQL] 26,289 pass(es).View details

#2

Posted by joachim on October 17, 2010 at 4:11pm
Version:8.x-dev» 7.x-dev

Testbot status 'ignored'? Is that because this is on 8? Hmm let's try it on 7.

#3

Posted by moshe weitzman on October 18, 2010 at 12:44am

tests pass. kill it.

#4

Posted by joachim on October 19, 2010 at 3:20pm

Here's a patch that removes it all.

AttachmentSizeStatusTest resultOperations
943558.drupal.remove-menu-block_callback.patch2.54 KBIdlePASSED: [[SimpleTest]]: [MySQL] 26,368 pass(es).View details

#5

Posted by moshe weitzman on October 19, 2010 at 3:24pm

Needs an update function to drop the column, no?

#6

Posted by joachim on October 19, 2010 at 3:57pm

So we do.

AttachmentSizeStatusTest resultOperations
943558-6.drupal.remove-menu-block_callback.patch2.85 KBIdlePASSED: [[SimpleTest]]: [MySQL] 26,381 pass(es).View details

#7

Posted by moshe weitzman on October 19, 2010 at 4:22pm
Status:needs review» reviewed & tested by the community

Looks ready to me.

#8

Posted by catch on October 19, 2010 at 4:32pm

nice.

#9

Posted by Dries on October 20, 2010 at 12:48am
Status:reviewed & tested by the community» fixed

Committed to CVS HEAD. Thanks.

#10

Posted by David_Rothstein on October 20, 2010 at 10:59pm
Status:fixed» needs work

It's not all the way gone yet...

#11

Posted by David_Rothstein on October 20, 2010 at 11:02pm
Status:needs work» needs review

Now it is :)

By the way, is the original commit here an API change? It seems like it sort of is, although I'm not convinced this functionality worked as intended in D7 anymore anyway.

AttachmentSizeStatusTest resultOperations
943558-11.patch1.59 KBIdlePASSED: [[SimpleTest]]: [MySQL] 26,431 pass(es).View details

#12

Posted by moshe weitzman on October 20, 2010 at 11:52pm
Status:needs review» reviewed & tested by the community

RTBC.

#13

Posted by Dries on October 21, 2010 at 11:56am
Status:reviewed & tested by the community» fixed

Committed to CVS HEAD. Thanks.

#14

Posted by System Message on November 4, 2010 at 12:00pm
Status:fixed» closed (fixed)

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

nobody click here