Make WordPress Core

Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#34334 closed enhancement (fixed)

Add a dynamic version of the admin_print_footer_scripts hook

Reported by: tfrommen's profile tfrommen Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 4.6 Priority: normal
Severity: trivial Version:
Component: General Keywords: has-patch commit
Focuses: Cc:

Description

The /wp-admin/admin-header.php file fires both the generic admin_print_scripts and the dynamic admin_print_scripts-$hook_suffix action hooks. The counterpart /wp-admin/admin-footer.php, however, only fires a generic action hook, admin_print_footer_scripts, which does not get the $hook_suffix passed as a parameter.

So, I'd say let's add the dynamic admin_print_footer_scripts-$hook_suffix action hook and place it right before its generic sibling.

Why does one need this (kind of) hook, you ask?

Well, the only dynamic footer action hook is admin_footer-$hook_suffix. This, however, is fired after admin_print_footer_scripts, which happens to be last chance to localize any scripts. If one would want to localize such a script depending on the current page, a dynamic variant of the action hook would come very handy.

Attached please find a patch for this. Since the $hook_suffix global is now used twice, I imported it (at the top of the file), just like /wp-admin/admin-header.php does.

Attachments (5)

34334.patch (1.5 KB) - added by tfrommen 8 years ago.
34334.2.patch (1.6 KB) - added by tfrommen 8 years ago.
Refreshed patch for WordPress 4.5.0
34334.3.patch (1.6 KB) - added by tfrommen 8 years ago.
Refreshed patch, with @global annotation.
34334.4.patch (1.7 KB) - added by tfrommen 8 years ago.
Refresh patch for WordPress 4.6.0.
34334.5.patch (4.0 KB) - added by tfrommen 8 years ago.

Download all attachments as: .zip

Change History (14)

@tfrommen
8 years ago

#1 @tfrommen
8 years ago

  • Keywords has-patch added
  • Severity changed from normal to trivial

#2 follow-up: @swissspidy
8 years ago

admin-footer.php fires a generic action hook, admin_print_footer_scripts, which does not get the $hook_suffix passed as a parameter.

Why not pass the $hook_suffix as a parameter then? That's easier to read and debug than dynamic hooks.

#3 in reply to: ↑ 2 @tfrommen
8 years ago

Replying to swissspidy:

Why not pass the $hook_suffix as a parameter then?

For two reasons:

1. It's easier to use.

If I want to do something for a specific admin page only, I just can do this:

add_action( 'admin_print_footer_scripts-some_admin_page', function () {
    // Whatever it is that I want to do
} );

With a generic hook that gets the $hook_suffix passed as parameter, I still need to check on my own:

add_action( 'admin_print_footer_scripts', function ( $hook_suffix ) {
    if ( 'some_admin_page' !== $hook_suffix ) {
        return;
    }

    // Whatever it is that I want to do
} );

2. Less overhead.

Let's say I have 42 distinct admin pages (yes, that number is fictional :)), and I want to do individual things for each of these pages. With a generic hook (having the $hook_suffx as parameter), on every single admin page (even the ones I'm not interested in) all of my 42 functions are called. Every single one checks what the hook suffix is - and maybe one (and only one) will take action.

Having a dynamic action, only the function that will actually do something will be called.

@tfrommen
8 years ago

Refreshed patch for WordPress 4.5.0

@tfrommen
8 years ago

Refreshed patch, with @global annotation.

@tfrommen
8 years ago

Refresh patch for WordPress 4.6.0.

#4 @ocean90
8 years ago

  • Keywords commit added
  • Milestone changed from Awaiting Review to 4.6

@tfrommen
8 years ago

#5 @tfrommen
8 years ago

As suggested (i.e., required ;) ) by @ocean90, the last attachment adds a dynamic version of the action for every file with the static one.

#6 @SergeyBiryukov
8 years ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 37279:

Administration: Introduce admin_print_footer_scripts-$hook_suffix", a dynamic version of the admin_print_footer_scripts hook.

This is now more consistent with the generic admin_print_scripts and the dynamic admin_print_scripts-$hook_suffix hooks fired in wp-admin/admin-header.php.

Props tfrommen.
Fixes #34334.

This ticket was mentioned in Slack in #core by tfrommen. View the logs.


8 years ago

#8 @tfrommen
8 years ago

  • Keywords needs-dev-note added

#9 @ocean90
8 years ago

  • Keywords needs-dev-note removed
Note: See TracTickets for help on using tickets.