Make WordPress Core

Opened 16 years ago

Closed 16 years ago

#9651 closed defect (bug) (fixed)

New Widget API - breaks plugins - widget options

Reported by: ceenz's profile ceenz Owned by:
Milestone: 2.8 Priority: normal
Severity: blocker Version: 2.8
Component: Widgets Keywords: has-patch dev-feedback
Focuses: Cc:

Description

The new Widget API breaks plugins 'Widget Context' and 'Widget Logic'. Both these plugins attempt to filter the display of widgets through different settings. Both plugins employee complicated hacks to WP and The Loop to achieve their means.
These hacks are now broken with the introduction of the new Widget API.

It is my belief due to the nature of the hacks employed by the above plugins there should be no work around in the new Widget API, however the goals and aims of these plugins maybe easily implemented through the addition of three new callbacks in the new API.

I have included a patch to the main widget.php that enables plugins to achieve the addition of filtering the display of widgets, and the addition of 'default' widget display options. I have also attached a plugin that makes use of these new callbacks to give widgets filter options that limit the display of widgets to the site homepage, individual page, categories etc.

I would very much like this patch to be included in the main branch.

Cheers,
Chris

Attachments (3)

widgets.php.diff (2.1 KB) - added by ceenz 16 years ago.
widget-options.zip (2.9 KB) - added by ceenz 16 years ago.
remove-pass-by-reference.diff (2.3 KB) - added by jbsil 16 years ago.
changes 3 &$this to $this in apply_filters calls

Download all attachments as: .zip

Change History (22)

#1 @ryan
16 years ago

This:

if ( false !== $instance ) { 
    $widget_options_compare = array($new_instance,$instance);
    $instance = apply_filters('default_widget_options_update',$widget_options_compare, $this);
}

Seems like it should be:

if ( false !== $instance ) { 
    $instance = apply_filters('default_widget_options_update', $instance, $new_instance, $this);
}

Otherwise, all widgets will break because $instance is being set to a multi-dimensional array containing both $new_instance and $instance, which is unexpected. With your plugin installed, the problem is obscured.

@ceenz
16 years ago

@ceenz
16 years ago

#2 follow-up: @ceenz
16 years ago

Agreed and updated.

#3 @azaozz
16 years ago

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

(In [11095]) Add hooks to WP_Widget, props ceenz, fixes #9651, see #8441

#4 in reply to: ↑ 2 @azaozz
16 years ago

Replying to ceenz:

Agreed and updated.

Changed the hooks a little as they were too specific, made them more generic (powerful). Will need to update the plugin a bit.

#5 @Viper007Bond
16 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened
Warning: Call-time pass-by-reference has been deprecated; If you would like to pass it by reference, modify the declaration of apply_filters(). If you would like to enable call-time pass-by-reference, you can set allow_call_time_pass_reference to true in your INI file in D:\Webserver\htdocs\wordpress-dev\wp-includes\widgets.php on line 170

Warning: Call-time pass-by-reference has been deprecated; If you would like to pass it by reference, modify the declaration of apply_filters(). If you would like to enable call-time pass-by-reference, you can set allow_call_time_pass_reference to true in your INI file in D:\Webserver\htdocs\wordpress-dev\wp-includes\widgets.php on line 231

Warning: Call-time pass-by-reference has been deprecated; If you would like to pass it by reference, modify the declaration of apply_filters(). If you would like to enable call-time pass-by-reference, you can set allow_call_time_pass_reference to true in your INI file in D:\Webserver\htdocs\wordpress-dev\wp-includes\widgets.php on line 261

@jbsil
16 years ago

changes 3 &$this to $this in apply_filters calls

#6 follow-up: @Denis-de-Bernardy
16 years ago

that'll only work in php5, no?

shouldn't do_action_ref_callback or whatever it's called be used instead, here?

#7 in reply to: ↑ 6 @hakre
16 years ago

Replying to Denis-de-Bernardy:

that'll only work in php5, no?

shouldn't do_action_ref_callback or whatever it's called be used instead, here?

php4: should work on a copy instead of a reference.

do_action_ref_callback: 0 matches in wp 2.8-bleeding

#8 @Denis-de-Bernardy
16 years ago

do_action_ref reveals the do_action_ref_array() function, which I believe should be used here instead.

#9 @ceenz
16 years ago

Passing a filtered $instance to $this->form defeats my original intended purpose of being able to add a plug-in that displays universal widget options on the widget admin form. While this may not be appropriate with these filters, any ideas as to how to do this?

#10 @Denis-de-Bernardy
16 years ago

@ceenz: I've a plugin that's similar to yours. I grab the callback on the init hook and wrap it around the needed functionality.

#11 @Denis-de-Bernardy
16 years ago

else, that's what do_action_ref_array() would do. I wish that was around too, as it would greatly simplify things indeed. ;-)

#12 @ceenz
16 years ago

yes using init is possible, although messy.

#13 follow-up: @hakre
16 years ago

I do not see any problems. This should all be done with callbacks, $callback = array($this, 'functionToCall');. You can bind any class instance to a callback.

#14 in reply to: ↑ 13 @Denis-de-Bernardy
16 years ago

Replying to hakre:

I do not see any problems. This should all be done with callbacks, $callback = array($this, 'functionToCall');. You can bind any class instance to a callback.

The problem is that, on php4 systems, passing the object without the reference will (best I'm aware) pass a clone of the object rather than the object itself. It's not much of a big deal, except that we end up using more memory that we should.

#15 @hakre
16 years ago

I doubt that this is the same for callbacks.

#16 @Denis-de-Bernardy
16 years ago

  • Severity changed from normal to blocker

#17 @Denis-de-Bernardy
16 years ago

  • Keywords has-patch dev-feedback added; widget plugin multi-widget widget options widget context widget logic removed

#18 @hakre
16 years ago

+1 to remove pass by reference. this will create problems with current WPs running on php5 (the minority?) and then all the upcomming WPs running php5+.

for php4, this should be not that much a problem. widgets force on "the reference" should register "their reference" in the globals variable space and catch it from there (in case of a copy).

nothing a plugin author can not handle, this is an upgrade that _can_ break things.

#19 @azaozz
16 years ago

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

(In [11155]) Add action "in_widget_form" allowing plugins to add extra fields, don't show "Save" button if a widget doesn't have settings form, fixes #9651

Note: See TracTickets for help on using tickets.