Shortcuts are built unconditionally for anonymous users

The whole shortcut_page_build() function definitely needs some rewriting - I guess we were going to mostly take care of that as a followup in the main shortcut issue, but yeah, this is a problem.

I've looked at this function several times and I'm not really sure I even understand how it works... it puts renderable stuff in $page['toolbar_drawer'] which the toolbar module then grabs, but since it's in the $page array, it's not obvious to me why it never gets displayed in the case where the toolbar module never grabs it... if it did, this performance bug would have been very visibly obvious :)

In any case, the shortcut_page_build() function needs some rethinking.

.

Subby.

Well, I guess this part of the code was just copied from toolbar, which assumed shortcuts are built when there is a place to display them (in the toolbar). If the toolbar was disabled, it did not render shortcuts. So, now that they are separate (and shortcut does not display the shortcuts in itself if toolbar is disabled), we should only generate these links if there will be someone to pick up the links. When this "drawer API" was designed, I've suggested adding a hook instead, but got opposition to add such a very special case hook, something like hook_toolbar_drawer(). That would however serve as a trigger for shortcut module to build the links, and the performance hit would not be there otherwise.

BTW, if it really takes 6% to build the shortcut bar, then we probably have performance fixes to do elsewhere too.

subscribble

I guess we were going to mostly take care of that as a followup in the main shortcut issue, but yeah, this is a problem

out of curiosity ... what is "the main shortcut issue?"

Here is a patch which takes the approach of having the shortcut module use hook_page_alter() to add a pre-render function to the toolbar. That way, the shortcuts are only built when they need to be rendered.

By the way, @bleen18, the main shortcut issue is this one: #511286: D7UX: Implement customizable shortcuts (the issue where the shortcut module was first added)

This fixes the problem at hand. We've made quite a dance for ourselves between the drawer and its contents. Not sure the complexity is helpful enough.

Hm. This patch effectively makes toolbar a requirement of shortcut? Should we spell this out in the .info file explicitly? Or is the idea that we're using a generic array index here that could be implemented by Super Toolbar or whatever?

@webchick: Well, the shortcut module is by design separated, so that it can be put under/with admin_menu module for example which can be a toolbar module replacement. And vice versa. So cross-dependencies should be optional at best.

Thank you for the benchmarks, catch!

Committed to HEAD.

Thanks!

Regarding the shortcut-toolbar coupling, this patch really didn't change much from the way it was before. In addition to other toolbar modules being able to use the same array structure (and automatically get the shortcuts inserted), we also have shortcut_renderable_links() which they can call to explicitly render the shortcuts wherever they want, plus the shortcut module provides its own block which works reasonably well even in the case where no toolbar module is installed at all.

Oops.