[wp-trac] [WordPress Trac] #32474: Facilitate widgets to be stored in posts instead of options

[WordPress Trac]

#32474: Facilitate widgets to be stored in posts instead of options
------------------------------+--------------------------
 Reporter:  westonruter       |       Owner:  westonruter
     Type:  enhancement       |      Status:  reopened
 Priority:  normal            |   Milestone:  4.3
Component:  Widgets           |     Version:  2.8
 Severity:  normal            |  Resolution:
 Keywords:  has-patch commit  |     Focuses:
------------------------------+--------------------------

Comment (by westonruter):

 @[comment:19 azaozz]: Thanks for raising these concerns. Let me try to
 address them and explain.

 > Don't see what is fixed by adding that conditional.

 What is fixed is that plugins now have the ''option'' of filtering the
 `widget_{$id_base}` options to return `ArrayIterator` objects instead of
 intrinsic arrays, and `WP_Widget` will handle them successfully. The
 `array_*` functions that had been used in `WP_Widget`'s methods (e.g.
 `is_array()`, `array_key_exists()`, `array_keys()`) were not all
 compatible with the new Array objects in PHP 5. Now with the Core patch,
 `WP_Widget` is more agnostic to what kind of iterable is being passed
 around.

 > If a plugin or a theme wants to use the (hacky) way of intercepting
 options get and set, why should this be in core and not in the plugin?

 The patch is making it possible for plugins to do just this. The patch
 isn't intercepting the options get/set in Core. It's just gracefully
 handling the scenario where `get_option()` is filtered to return an
 `ArrayIterator` (or `ArrayObject`) as opposed to an intrinsic `array`. For
 an example of what a plugin can then do with this accommodation in Core,
 see the Customize Widgets Plus plugin's [https://github.com/xwp/wp-
 customize-widgets-plus/blob/master/php/class-widget-posts.php Widget
 Posts] module (yes, shameless plug).

 The bit of the patch you quote here is unfortunate because PHP (strangely)
 provides no `getKeys()` method for its Array objects. So that's why it is
 using the `getArrayCopy()` method to obtain an array of the keys. But note
 that this method bypasses the accessor methods like
 `ArrayIterator::offsetGet()` and `ArrayIterator::current()`, so if those
 methods are indeed lazy-loading widget instance data from posts, this will
 not be involved when `getArrayCopy()` is called.

 When developing the Widget Customizer plugin, I had to directly manipulate
 widget options extensively in order for them to be previewed and saved as
 expected. Even now, the Customizer in Core is creating
 `WP_Customize_Setting`s of type  `option` to represent widget instances.
 Based on this, my assumption has thus been that advanced plugins working
 with widgets may also interact with the underlying widget instance data
 via `get_option()` and `update_option()`, meaning that the filters need to
 be applied at this level. WP-CLI, for instance, [https://github.com/wp-cli
 /wp-
 cli/blob/6e626cc73e0463fe9564df3b98c9f08f9466b430/php/commands/widget.php#L394-L402
 gets the underlying option data] as opposed to accessing
 `WP_Widget::get_settings()`. I did a quick search on WordPress.org, and I
 see the Display Widgets plugin also interacts with widget options
 directly.

 > Also if we ever change the way widgets data is saved (using CPTs sounds
 like a good idea), this will have no backwards compatibility.

 Nevertheless, the `WP_Widget::get_settings()` and
 `WP_Widget::save_settings()` methods already work with arrays, and these
 arrays are ''mappings of widget instance numbers to their corresponding
 widget instance arrays''. Even if we do have Core switch to storing
 widgets in posts as opposed to options, we can't get around the fact that
 these methods have been implemented in this way. So we need to provide a
 way for these methods to efficiently pass around arrays of widget
 instances no matter the data backend. This is what `ArrayIterator` makes
 possible, and it is fully backwards compatible.

 > If we need to let plugins change the way widgets data is stored, a much
 better way would be to use actual filters, like we do in so many other
 places. Then we can maintain backwards compatibility.

 So yes, we could introduce filters in `WP_Widget::get_settings()` and
 `WP_Widget::save_settings()` to be able to short-circuit the normal logic
 and return `ArrayIterator` objects instead of intrinsic arrays. But other
 methods in `WP_Widget` (`_register`, `display_callback`, and
 `form_callback`) also then would have to be patched, as in this ticket's
 patch, to be compatible. And as above, this wouldn't help plugins that
 actually operate at the lower option-level.

 Instead, without introducing new filters, we can leverage the existing
 `pre_option_widget_{$id_base}` and `pre_update_option_widget_{$id_base}`
 ones.

 The patch is all backwards-compatible because without any plugins
 filtering `pre_option_widget_{$id_base}`, it will continue to work with
 intrinsic arrays as it always has. But then if a plugin does return
 `ArrayIterator` objects here instead, it is still backwards-compatible
 because these objects have the same interface as intrinsic arrays, as they
 are both iterables.

 Sorry for the long(-winded) reply.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/32474#comment:20>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform