Closed (outdated)
Project:
Drupal core
Version:
7.x-dev
Component:
base system
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
3 Sep 2015 at 19:03 UTC
Updated:
13 Oct 2015 at 08:19 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
chx commentedComment #3
fabianx commentedMissing '&' before $text.
Comment #4
chx commentedAlso added a note about how hook_url_outbound_alter is better for $path .
Comment #5
chx commentedFixing credit.
Comment #6
fabianx commentedRTBC, I don't think we need tests for this little change.
Comment #7
David_Rothstein commentedMy understanding is that this kind of use case is one of the reasons l() was made themable... Via a theme preprocess function or similar, you should be able to alter anything you want to here.
Of course that's an expensive thing to do performance-wise, but the expense comes only for the sites that need it, whereas a drupal_alter() call isn't totally free and impacts the majority of sites that won't ever use it. Plus it adds some extra complexity to what is already a complex area of the code...
I know there are also concerns with l() getting called too early in the bootstrap and triggering the theme system too early, but that should be possible to fix in #2086335: Bootstrap issue with theme initialization causes hook_custom_theme() never to be invoked.
Overall I think this change needs more review; I'm not totally sure that it's justified.
Comment #8
fabianx commentedWe maybe could make hook_link_alter() optionally enabled via a $settings parameter?
Theme Link via preprocess is not the right thing here as it bundles the fast link system to the (in comparison) very slow theme system.