WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#34995 closed defect (bug) (fixed)

WP_Widget::widget not called

Reported by: btwatts Owned by: wonderboymusic
Milestone: 4.4.1 Priority: normal
Severity: normal Version: 4.4
Component: Widgets Keywords: fixed-major
Focuses: Cc:

Description (last modified by SergeyBiryukov)

I have what I believe is a proper WP_Widget subclass that uses new style constructor.

I started with the WPBeginner example: http://www.wpbeginner.com/wp-tutorials/how-to-create-a-custom-wordpress-widget/

And worked through: http://stackoverflow.com/questions/23921729/how-to-create-a-custom-wordpress-widget-with-widget-options/34171737

I also referenced the codex which suggests that everything I've got implemented is correct.

When I run this code on WordPress 4.3 it works fine. But when I upgrade to WordPress 4.4 the __constructor is correctly called, but WP_Widget::widget is never called.

I haven't debugged down into the core to see what changed. I was hoping someone else has seen a problem similar and that we have a solution coming soon.

The website that I am depending on this code for is http://www.excellhobby.com/

I did a similar test on a brand new site that produced the same results (no plugins installed to conflict with the WP_Widget code). ie., Install WordPress 4.3, install the plugin and theme that has modifications to allow topleftwidget. With WordPress 4.3 it worked flawlessly. But when I upgraded to WordPress 4.4 it stopped working. All other variables are the same (same system, same PHP version, etc).

Here is the code for my plugin which is slightly modified from the example:

<?php
/*
Plugin Name: Byron Image Widget
*/

// https://codex.wordpress.org/Widgetizing_Themes#How_to_Register_a_Widget_Area

function biw_register_widget_area() {

register_sidebar( array(
                'name' => __( 'Top Left', 'biw_text_domain'),
                'id' => 'topleftwidget',
                'description' => __( 'Top Left Widget', 'biw_text_domain'),
                'before_widget' => '<aside id="%1$s" class="widget %2$s">',
                'after_widget' => '</aside>',
                'before_title' => '<h3 class="widget-title">',
                'after_title' => '</h3>',
        ) );
}


// Creating the widget
class biw_topleft_widget extends WP_Widget {

        /**
         *  Register widget with WordPress.
         */
        function __construct() {
                parent::__construct(
                        'biw_topleft_widget',                   // Base ID of widget
                        __('Byron Image Widget', 'biw_text_domain'), // Widget name will appear in UI
                        array( 'description' => __( 'Insert Images In Widget Areas', 'biw_text_domain' ), )     // Widget description
                );
                // This appears echo '<br/>Constructor Testing<br/>';
        }

        /**
         * Front-end display of widget.
         *
         * @see WP_Widget::widget()
         *
         * @param array $args     Widget arguments.
         * @param array $instance Saved values from database.
         *
         * Note:
         * Creating widget front-end
         * This is where the action happens
         */
        public function widget( $args, $instance ) {
                echo $args['before_widget'];

                //if ( ! empty( $instance['title'] ) ) {
                //      echo $args['before_title'] . apply_filters( 'widget_title', $instance['title'] ) . $args['after_title'];
                //}

                //echo '<br/>THIS IS THE TOP LEFT WIDGET<br/>';

                // This is where you run the code and display the output
                echo '<div align="center">';
                //Home Page
                if (is_front_page()) { 
                        echo '<img width="300" height="190" src="//excellhobby.com/wp-content/uploads/2015/04/Excell_Service_crop-300x190.png">';
                        echo '<br/>';
                        echo __( 'Excellent Service and Support!', 'biw_text_domain' );
                } else
                if (is_page("about")) { 
                        //About Page
                        echo '<img width="300" height="225" src="//excellhobby.com/wp-content/uploads/2014/07/Excell_Service_01-300x225.jpg">';
                        echo '<br/>';
                        echo __( 'Excelling at Service and Support!', 'biw_text_domain' );
                } else
                if (is_page("contact")) { 
                        //Contact Page
                        echo '<img width="297" height="200" src="//excellhobby.com/wp-content/uploads/2015/04/Chris_2-297x300.png">';
                        echo '<br/>';
                        echo __( 'Contact Chris!', 'biw_text_domain' );
                } else
                if (is_page("about/current-projects")) { 
                        //CURRENT PROJECTS Page
                        echo '<img width="300" height="225" src="//excellhobby.com/wp-content/uploads/2015/03/100_21601-300x225.jpg">';
                        echo '<br/>';
                        echo __( 'Get R/C Today!', 'biw_text_domain' );
                } else
                if (is_page("blog")) { 
                        //Blog Page
                        echo '<img width="300" height="225" src="//excellhobby.com/wp-content/uploads/2014/07/Chris_Flying_09-300x225.jpg">';
                        echo '<br/>';
                        echo __( 'Keep In Touch.', 'biw_text_domain' );
                } else
//              if (is_page("shop")) /* For some reason "shop" isn't matching.... */
                { 
                        //Shop Page
                        echo '<img width="300" height="215" src="//excellhobby.com/wp-content/uploads/2015/07/Store_02_small.png">';
                        echo '<br/>';
                        echo __( 'You Deserve It.', 'biw_text_domain' );
                }
                echo '</div>';

                echo $args['after_widget'];
        }

        /**   
         * Back-end widget form.
         *
         * @see WP_Widget::form()
         *
         * @param array $instance Previously saved values from database.
         *
         * Note:
         * This form doesn't do much because the args are hard coded in the plugin.
         *
         */
        public function form( $instance ) {
                $title = ! empty( $instance['title'] ) ? $instance['title'] : __( 'New title', 'biw_text_domain' );
                // Widget admin form
?>
<p>
        <label for="<?php echo $this->get_field_id( 'title' ); ?>"><?php _e( 'Title:' ); ?></label>
        <input class="widefat" id="<?php echo $this->get_field_id( 'title' ); ?>" name="<?php echo $this->get_field_name( 'title' ); ?>" type="text" value="<?php echo esc_attr( $title ); ?>" />
</p>
<?php
        }

        /**
         * Sanitize widget form values before saving.
         *
         * @param array $new_instance Values just sent to be saved.
         * @param array $old_instance Previously saved values from database.
         *
         * @return array Updated safe values to be saved.
         */
        public function update( $new_instance, $old_instance ) {
                $instance = array();
                $instance['title'] = ( ! empty( $new_instance['title'] ) ) ? strip_tags( $new_instance['title'] ) : '';
                return $instance;
        }
} // Class biw_topleft_widget ends here
 
// Register and load the widget
function biw_load_widget() {
        biw_register_widget_area();
        register_widget( 'biw_topleft_widget' );
}
add_action( 'widgets_init', 'biw_load_widget' );

?>

Attachments (1)

get_field_name.patch (1.4 KB) - added by pbearne 5 years ago.
patch and unit tests

Download all attachments as: .zip

Change History (23)

#1 @westonruter
5 years ago

  • Description modified (diff)

#2 follow-up: @westonruter
5 years ago

  • Keywords close reporter-feedback added

@btwatts Hi there.

I just tried your plugin in 4.4 and I saw the widget load as expected with the widget method being called. Did you add the widget to the “Top Left” widget area (sidebar) or to an existing widget area in your theme? Just calling register_sidebar() is not enough for the widget area to appear on your site. You have to add the required dynamic_sidebar() call in the template as well.

#3 @btwatts
5 years ago

Sorry, I overlooked the header.php file which does include the dynamic_sidebar() call.

<?php
// File Security Check
if ( ! empty( $_SERVER['SCRIPT_FILENAME'] ) && basename( __FILE__ ) == basename( $_SERVER['SCRIPT_FILENAME'] ) ) {
    die ( 'You do not have sufficient permissions to access this page!' );
}

/**
 * Header Template
 *
 * Here we setup all logic and XHTML that is required for the header section of all screens.
 *
 * @package WooFramework
 * @subpackage Template
 */
global $woo_options, $woocommerce;
?><!DOCTYPE html>
<html <?php language_attributes(); ?> class="<?php if ( $woo_options['woo_boxed_layout'] == 'true' ) echo 'boxed'; ?> <?php if (!class_exists('woocommerce')) echo 'woocommerce-deactivated'; ?>">
<head>

<meta charset="<?php bloginfo( 'charset' ); ?>" />

<title><?php woo_title(''); ?></title>
<?php woo_meta(); ?>
<link rel="stylesheet" type="text/css" href="<?php bloginfo( 'stylesheet_url' ); ?>" media="screen" />
<link rel="pingback" href="<?php bloginfo( 'pingback_url' ); ?>" />
<?php
	wp_head();
	woo_head();
?>

</head>

<body <?php body_class(); ?>>
<?php woo_top(); ?>

<div id="wrapper">

	<div id="top">
		<nav class="col-full" role="navigation">
			<?php if ( function_exists( 'has_nav_menu' ) && has_nav_menu( 'top-menu' ) ) { ?>
			<?php wp_nav_menu( array( 'depth' => 6, 'sort_column' => 'menu_order', 'container' => 'ul', 'menu_id' => 'top-nav', 'menu_class' => 'nav fl', 'theme_location' => 'top-menu' ) ); ?>
			<?php } ?>
			<?php
				if ( class_exists( 'woocommerce' ) ) {
					echo '<ul class="nav wc-nav">';
					woocommerce_cart_link();
					echo '<li class="checkout"><a href="'.esc_url($woocommerce->cart->get_checkout_url()).'">'.__('Checkout','woothemes').'</a></li>';
					echo get_search_form();
					echo '</ul>';
				}
			?>
		</nav>
	</div><!-- /#top -->

    <?php woo_header_before(); ?>

	<header id="header" class="col-full">

	    <hgroup>
	    	 <?php
			    $logo = esc_url( get_template_directory_uri() . '/images/logo.png' );
					if ( isset( $woo_options['woo_logo'] ) && $woo_options['woo_logo'] != '' ) { $logo = $woo_options['woo_logo']; }
					if ( isset( $woo_options['woo_logo'] ) && $woo_options['woo_logo'] != '' && is_ssl() ) { $logo = preg_replace("/^http:/", "https:", $woo_options['woo_logo']); }
				?>
			<?php if ( ! isset( $woo_options['woo_texttitle'] ) || $woo_options['woo_texttitle'] != 'true' ) { ?>

			    <a id="logo" href="<?php echo esc_url( home_url( '/' ) ); ?>" title="<?php esc_attr( get_bloginfo( 'description' ) ); ?>">
			    	<img width="50%" style="float:right;margin:0 5px 0 0;" src="<?php echo $logo; ?>" alt="<?php echo esc_attr( get_bloginfo( 'name' ) ); ?>" />
			    </a>
		    <?php } ?>

			<h1 class="site-title"><a href="<?php echo esc_url( home_url( '/' ) ); ?>"><?php bloginfo( 'name' ); ?></a></h1>
			<h2 class="site-description"><?php bloginfo( 'description' ); ?></h2>
			<h3 class="nav-toggle"><a href="#navigation"><mark class="websymbols">&#178;</mark> <span><?php _e('Navigation', 'woothemes'); ?></span></a></h3>

		</hgroup>

<?php /* http://uploadwp.com/community/index.php?threads/mystile-homepage-white-space-change-and-social-media-plugins.49/#post-180 */ ?>

<div id="topleftwidgetarea">
<?php if ( is_active_sidebar( 'TopLeftWidget' ) ) : ?>
    <?php dynamic_sidebar( 'TopLeftWidget' ); ?>
<?php endif; ?>
</div>
        <?php woo_nav_before(); ?>

		<nav id="navigation" class="col-full" role="navigation">

			<?php
			if ( function_exists( 'has_nav_menu' ) && has_nav_menu( 'primary-menu' ) ) {
				wp_nav_menu( array( 'depth' => 6, 'sort_column' => 'menu_order', 'container' => 'ul', 'menu_id' => 'main-nav', 'menu_class' => 'nav fr', 'theme_location' => 'primary-menu' ) );
			} else {
			?>
	        <ul id="main-nav" class="nav fl">
				<?php if ( is_page() ) $highlight = 'page_item'; else $highlight = 'page_item current_page_item'; ?>
				<li class="<?php echo $highlight; ?>"><a href="<?php echo esc_url( home_url( '/' ) ); ?>"><?php _e( 'Home', 'woothemes' ); ?></a></li>
				<?php wp_list_pages( 'sort_column=menu_order&depth=6&title_li=&exclude=' ); ?>
			</ul><!-- /#nav -->
	        <?php } ?>

		</nav><!-- /#navigation -->

		<?php woo_nav_after(); ?>

	</header><!-- /#header -->

	<?php woo_content_before(); ?>
Last edited 5 years ago by SergeyBiryukov (previous) (diff)

#4 in reply to: ↑ 2 @btwatts
5 years ago

Replying to westonruter:

@btwatts Hi there.

I just tried your plugin in 4.4 and I saw the widget load as expected with the widget method being called. Did you add the widget to the “Top Left” widget area (sidebar) or to an existing widget area in your theme? Just calling register_sidebar() is not enough for the widget area to appear on your site. You have to add the required dynamic_sidebar() call in the template as well.

In multiple tests I have not seen WP_Widget::widget get called on any version I have of WordPress 4.4

It works very well with 4.3.

If I need to dig in deeper, I can set up to debug inside WP_Widget.

...but my debugging inside WordPress is so far untried.

#5 @btwatts
5 years ago

  • Resolution set to invalid
  • Status changed from new to closed

CLOSE

I found the problem.

The ID I was registering and the ID I was declaring in dynamic_sidebar were case mismatched.

Apparently the case didn't matter in 4.3 but it matters in 4.4.

#6 @SergeyBiryukov
5 years ago

  • Description modified (diff)
  • Keywords close reporter-feedback removed
  • Milestone Awaiting Review deleted

#7 @SergeyBiryukov
5 years ago

  • Keywords needs-patch needs-unit-tests added
  • Milestone set to 4.4.1
  • Resolution invalid deleted
  • Status changed from closed to reopened

I was able to confirm the regression.

[34465] made the $index argument of dynamic_sidebar() case-sensitive, because it no longer runs through sanitize_title().

The unit test added in that changeset doesn't make much sense to me, because it doesn't seem to test the reported issue with using special characters in sidebar ID. It does not even use the sidebar it registers. Instead, it relies on the assumption that the current default theme does not register a sidebar named 'Sidebar 1', and verifies that dynamic_sidebar( 'Sidebar 1' ) does not display the sidebar registered with 'id' => 'sidebar-1'. This might be a related issue, but it's not what was reported in #23423. It's more related to #22116.

Is there a reason to use special characters in sidebar IDs? Perhaps [34465] should be reverted, with #23423 closed as invalid, or pushed to a future release pending a proper fix.

Last edited 5 years ago by SergeyBiryukov (previous) (diff)

#8 @pbearne
5 years ago

Hi guys

I am seeing a problem as well

if you pass in display][avatar_size as the name this used to work and you would get and nice nested array in the post data

not it is outputting widget-author_avatars[4][display]][avatar_size

which is breaking the form data

would you like an unit-test for this use-case?

Paul

#9 @pbearne
5 years ago

OK I see the problem

if an array format is passed it doesn't check that it ends with a closing ']'

I will have a patch in a bit

@pbearne
5 years ago

patch and unit tests

#10 @pbearne
5 years ago

Unit test added for strings in the format of display][avatar_size as this was an old hack to allow "Array format field names"

The patch is not pretty but does work

#11 @pbearne
5 years ago

  • Keywords has-patch needs-testing has-unit-tests dev-feedback added; needs-patch needs-unit-tests removed

#12 @pbearne
5 years ago

It would better if we can spot strings in this format and use the old path to handle it rather then "fixing afterwards as the patches does!

#13 follow-up: @SergeyBiryukov
5 years ago

  • Keywords needs-patch needs-unit-tests added; has-patch needs-testing has-unit-tests dev-feedback removed

Replying to pbearne:

if you pass in display][avatar_size as the name this used to work and you would get and nice nested array in the post data

not it is outputting widget-author_avatars[4][display]][avatar_size

which is breaking the form data

This seems unrelated to the dynamic_sidebar() function and the issue described in comment:7.

Could you please create a new ticket for your issue? Thanks!

#14 in reply to: ↑ 13 @pbearne
5 years ago

Replying to SergeyBiryukov:

Replying to pbearne:

if you pass in display][avatar_size as the name this used to work and you would get and nice nested array in the post data

not it is outputting widget-author_avatars[4][display]][avatar_size

which is breaking the form data

This seems unrelated to the dynamic_sidebar() function and the issue described in comment:7.

Could you please create a new ticket for your issue? Thanks!

Have realised I had put it in the wrong place and had already started creating the new ticket :-)

https://core.trac.wordpress.org/ticket/35023

Last edited 5 years ago by pbearne (previous) (diff)

#15 @jorbin
5 years ago

@SergeyBiryukov I support reverting 34465. I am fine with forcing no special characters in sidebars. Perhaps even going so far as throwing a _doing_it_wrong if you use a special character in a sidebar id.

#16 follow-up: @jorbin
5 years ago

  • Owner set to wonderboymusic
  • Status changed from reopened to assigned

@wonderboymusic, @tyxla, @fjarrett - This is related to [34465] which you worked on. Can you please leave your comments?

#17 in reply to: ↑ 16 @tyxla
5 years ago

Replying to jorbin:

@wonderboymusic, @tyxla, @fjarrett - This is related to [34465] which you worked on. Can you please leave your comments?

If you guys consider making $index case-sensitive a regression, I believe reverting would be the best way to continue here. I guess [34465] has caused unexpected side effects.

If we go that way, it is possibly best to invoke a _doing_it_wrong when using special characters, as @jorbin suggested.

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


5 years ago

#19 @jorbin
5 years ago

If there isn't any movement on this in the next day, it is going to miss 4.4.1 and be moved to the 4.5 milestone. If that happens, once something is committed, it could be considered for backporting into a potential 4.4.2

#20 @SergeyBiryukov
5 years ago

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

In 36130:

Widgets: Revert [34465], as it introduced a regression, making the $index argument of dynamic_sidebar() case-sensitive.

Fixes #34995 for trunk. See #23423.

#21 @SergeyBiryukov
5 years ago

  • Keywords fixed-major added; needs-patch needs-unit-tests removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

#22 @dd32
5 years ago

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

In 36148:

Widgets: Revert [34465], as it introduced a regression, making the $index argument of dynamic_sidebar() case-sensitive.

Merges [36130] to the 4.4 branch.
See #23423.
Fixes #34995.

Note: See TracTickets for help on using tickets.