Make WordPress Core

Opened 9 years ago

Closed 9 years ago

#27853 closed defect (bug) (fixed)

The "admin_footer" action isn't called on customize.php

Reported by: feedmeastraycat's profile feedmeastraycat Owned by: azaozz's profile azaozz
Milestone: 3.9.1 Priority: normal
Severity: normal Version: 3.9
Component: Editor Keywords: has-patch commit fixed-major
Focuses: Cc:

Description

I have a plugin called WP Editor Widget which adds a rich content widget using the wp_editor() function. It does this on the wp-admin/widgets.php page by using the "widgets_admin_page" action to output a hidden div containing the wp_editor() code. The widget itself has a button that opens and closes the editor overlay and copies the editor content to a hidden input inside the widget.

This works well on widgets.php but I have a weird issue when trying to make it work on customize.php.

It seems that customize.php doesn't include admin-footer.php but instead only calls the admin_print_footer_scripts action and admin_footer-widgets.php in WP_Customize_Widgets->print_footer_scripts(). Which is added to the "customize_controls_print_footer_scripts" action in customize.php.

This means for the _WP_Editor class that _WP_Editor::editor_js() is called. But never _WP_Editor::enqueue_scripts() (which both are added in _WP_Editor:: editor_settings()) making my wp editor only function half way. I can't switch to plain text mode for example.

I've solved this, I think, for now, by just adding a customize_controls_print_footer_scripts action on low prio (so it runs before the one in WP_Customize_Widgets because it needs to be called before _WP_Editor::editor_js(), but after a wp_editor() function is called) and calling _WP_Editors::enqueue_scripts() manually there.

I've created a patch that solves it by adding do_action( 'admin_footer' ) to WP_Customize_Widgets->print_footer_scripts();
But maybe there is a reason for it not to be there?
My bug is a bit low risk. But maybe other bugs can appear because the admin_footer action isn't called?
But maybe new bugs can appear if it is called? :)

Attachments (2)

class-wp-customize-widgets.php.diff (592 bytes) - added by feedmeastraycat 9 years ago.
Patch
class-wp-editor.php.diff (713 bytes) - added by feedmeastraycat 9 years ago.
Patch 2 (wp-editor suggestion)

Download all attachments as: .zip

Change History (13)

#1 @nacin
9 years ago

  • Milestone changed from Awaiting Review to 3.9.1

Moving to 3.9.1 for investigation.

#2 @nacin
9 years ago

I really have no idea why wp_editor() uses admin_footer and wp_footer rather than admin_print_footer_scripts and wp_print_footer_scripts.

Calling admin_footer here would probably have side effects as developers may be printing information on those hooks (while using a _scripts hook would not be such a good idea for that). But fixing wp_editor() should be easier anyway.

Does changing the hook make everything work for you?

#3 @feedmeastraycat
9 years ago

My patch fixes my issues. And I assume a patch on wp_editor that changes the actions to this:

if ( empty(self::$first_init) ) {
        if ( is_admin() ) {
                add_action( 'admin_print_footer_scripts', array( __CLASS__, 'enqueue_scripts'), 1 );
                add_action( 'admin_print_footer_scripts', array( __CLASS__, 'editor_js'), 50 );
        } else {
                add_action( 'wp_print_footer_scripts', array( __CLASS__, 'enqueue_scripts'), 1 );
                add_action( 'wp_print_footer_scripts', array( __CLASS__, 'editor_js'), 50 );
        }
}

Would fix it as well. The important part is that _WP_Editor::enqueue_scripts() is never called on customize.php. Both my submitted patch and the example above should fix that.

I can create a patch like that and test it. And if it seems to fix the issue I'll submit it.

But the big thing here (I guess) is that I don't know if any of these patches will have bad side effects elsewhere. :)

But maybe there is a third, easier and better way?

This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.


9 years ago

#5 @nacin
9 years ago

  • Owner set to azaozz
  • Status changed from new to assigned

Looks good, feedmeastraycat.

@feedmeastraycat
9 years ago

Patch 2 (wp-editor suggestion)

#6 @feedmeastraycat
9 years ago

I've tested and uploaded a second patch now. It also fixes the issue.

But I guess someone with a bit more knowledge about wp-editor or the customizer function should have a look at this so it doesn't causes issues elsewhere.

(Note, if I haven't been clear enough, both patches isn't required. Two solutions to the same issue).

#7 @azaozz
9 years ago

  • Keywords 2nd-opinion added

Changing _WP_Editors::enqueue_scripts() to run later still outputs all editor related scripts in the same order at the same place.

One possible regression is if a plugin was enqueueing a script on 'admin_footer' or 'wp_footer' in order to output it after the editor related scripts (word-count, editor, wplink, etc.), now it will be outputted before them.

In that terms the change should probably be added to trunk but maybe not to 3.9.1.

Last edited 9 years ago by azaozz (previous) (diff)

#8 @azaozz
9 years ago

In 28187:

Run WP_Editors::enqueue_scripts() on admin_print_footer_scripts priority 1 (later), instead of admin_footer. Fixes incompatibility with the customizer. Props feedmeastraycat, see #27853, for trunk.

#9 @feedmeastraycat
9 years ago

@azaozz: Does this mean it will end up in 3.9.1? (If it does, I will fix my plugin to only include my fix on 3.9 or lower version of WP)

#10 @nacin
9 years ago

  • Keywords has-patch commit fixed-major added; 2nd-opinion removed

Yes, it will end up in 3.9.1.

#11 @nacin
9 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 28203:

Run WP_Editors::enqueue_scripts() on admin_print_footer_scripts, instead of admin_footer.

Fixes incompatibility with the customizer.

Merges [28187] to the 3.9 branch.

props feedmeastraycat.
fixes #27853.

Note: See TracTickets for help on using tickets.