Bring widgets into WordPress core

[rob1n]

We need to do this before we can release 2.2. Basically just make Automattic Widgets part of the core.

[rob1n]

Things to consider:

1. Which implementation we're going to integrate. I think the most viable choice here is the Automattic Widgets plugin (​http://automattic.com/code/widgets/).

2. Possible conflict with existing widget plugins. My proposed solution to work around this is to put wp_ before all those "internal" functions (i.e. not the template tags). Then, for the template tags, we do if (!function_exists()) checks so the internal implementation of widgets can:
   1. Be replaced by other plugins. Not everyone agrees on how something is implemented, and thus they would probably use a plugin.
   2. Work with existing plugins (so you won't get a fat fatal error when you upgrade).

3. Where to put the widgets? Before, in the case of the Automattic plugin, the widgets were located in wp-content/plugins/widgets. I propose wp-content/widgets. It's the user's content, technically, and keeping it in wp-content keeps it independent of includes and admin.

4. Where the admin menu will be. Either Manage > Widgets or Presentation > Widgets. Either way, but now that I think a bit more about it the second option makes more semantical sense.

That's about it, really. The default and classic themes are already widget-aware. Opening to debate.

[filosofo]

Replying to rob1n:

3. Where to put the widgets? Before, in the case of the Automattic plugin, the widgets were located in wp-content/plugins/widgets. I propose wp-content/widgets. It's the user's content, technically, and keeping it in wp-content keeps it independent of includes and admin.

Why not let them be standalone plugins, if include them at all? That's what they are now (if you mean stuff like the del.icio.us widget).

[Otto42]

Re: #2: Possibly we can check for and automatically deactivate the old widgets plugin as part of the upgrade.php? Might save some headaches.

Re: #3: Widgets are plugins, plugins can be widgets. There's no need to have a defined place to put widgets. The core code moves into the core, the widgets.php plugin file disappears, done and done. There's no need to move things around.

[rob1n]

That's the first part (60%, I'd say). What's left to do is the admin. That's it. Well, and fix the core part ;).

I'd appreciate it if people could look it over. In those 900 lines of code, I probably made a few mistakes. Hopefully not that many though.

[matt]

Don't forget to change the link in the meta widget to go to WordPress.org and not WordPress.com.

[Otto42]

In wp_widget_links();

This:

wp_list_bookmarks( array( 'title_before' => $before_title, 'title_after' => $after_title ) );

Should be this:

wp_list_bookmarks(array( 'title_before'=>$before_title, 'title_after'=>$after_title, 'category_before'=>$before_widget, 'category_after'=>$after_widget));

In order to make the links widget work properly.

[technosailor]

Big whopping +1 on this to be committed. Would be nice to be able to get fixed code in SVN for our blogs.

[rob1n]

Alright, new version that has Otto42 and matt's fixes.

[rob1n]

Note that there's a parse error. Some missing bracket or something. I'll track it down in the morning. (this is re widgets.php in wp-includes).

[ryan]

Extra close paren on line 16 of widgets.js.php. Warnings on lines 11 and 12 of wp-admin/widgets.php because $wp_registered_widgets is not initialized. wp-includes/widgets.php: $sidbar typo on line 59, $wp_registered_sidebar_defaults on line 133 does not match $wp_register_sidebar_defaults global, $items not initialized on line 771. Also, wp_widget_recent_entries() looks like it is missing a brace. I suggest just returning immediately if !have_posts() rather than putting the positive case inside an if block. Also, using the if:endif; style blocks for the function_exists() checks and not indenting the functions within the if:endif block is easier to read, IMO.

[rob1n]

Extra close paren on line 16 of widgets.js.php.

Warnings on lines 11 and 12 of wp-admin/widgets.php because $wp_registered_widgets is not initialized.

wp-includes/widgets.php: $sidbar typo on line 59

$wp_registered_sidebar_defaults on line 133 does not match $wp_register_sidebar_defaults global

$items not initialized on line 771.

Also, wp_widget_recent_entries() looks like it is missing a brace.

I suggest just returning immediately if !have_posts() rather than putting the positive case inside an if block.

Also, using the if:endif; style blocks for the function_exists() checks and not indenting the functions within the if:endif block is easier to read, IMO.

(Just getting it more like a checklist)

[rob1n]

widgets-all.2.diff is what I've got so far. I've tested it lightly on my local testing server.

[rob1n]

Alright, we are good to go, as far as I can tell.

[ryan]

Looks good. No warnings in Eclipse.

[rob1n]

(In [5294]) Widgets support. fixes #4169 for 2.2

[Sewar]

We need widgets-rtl.css for RTL support.

[Sewar]

I will try to create patch and submit it later.

[rob1n]

Let's split it up into different tickets.

#4185 is the RTL CSS ticket.

[technosailor]

Anyone with commit access care to commit Otto's change to the widget repository itself? Seems silly that the widget for Links hasn't been updated since WP 2.1 and bookmarks.

[ryan]

Option names shouldn't get the wp_ prefix. We don't want to throw out sidebar provisioning made with the plugin.

[ryan]

Also, I see wp_widget_recent_entries() but not wp_widget_recent_entries_control() or wp_flush_widget_recent_entries().

[ryan]

Whoa, wp-includes/widgets.php has some ancient and screwy gettext bits in it. I see that the latest in the plugins svn is also ancient. Gotta go thump skeltoac.

[ryan]

wordpress.com version of widgets.php

[ryan]

Attached the latest bits as used on wordpress.com. Notice that register_sidebar_widget() in this version doesn't try to gettext variables, which is a no-no.

[ryan]

(In [5301]) Update widgets to latest bits. see #4169

[ryan]

(In [5306]) Update widgets to latet bits. For 2.2. see #4169

[filosofo]

I've attached a patch to put the proper names of widgets (not their ids) back into the admin page.

[rob1n]

(In [5310]) Variable name fix. Props filosofo. see #4169

[rob1n]

(In [5311]) Variable name fix. Props filosofo. For 2.2. see #4169

[Otto42]

The latest patches once again broke the links widget... :(

Change:

wp_list_bookmarks(array('title_before'=>$before_title, 'title_after'=>$after_title, 'show_images'=>true, 'class'=>'linkcat widget')); 

to:

wp_list_bookmarks(array(
'title_before'=>$before_title, 'title_after'=>$after_title, 
'show_images'=>true, 'class'=>'linkcat widget',
'category_before' => $before_widget, 'category_after' => $after_widget 
)); 

And it's fixed.

[technosailor]

The new widgets integration seems to be ignoring existing widgets. That's a big problem. :) It also does not play well when the widgets plugin is installed. We need to find a way to disable or ignore the widgets plugin or we're going to have lots of breakage.

r5313

[rob1n]

(In [5317]) Links widget fixes from Otto42. see #4169

[rob1n]

Replying to technosailor:

The new widgets integration seems to be ignoring existing widgets. That's a big problem. :) It also does not play well when the widgets plugin is installed. We need to find a way to disable or ignore the widgets plugin or we're going to have lots of breakage.

Do you mean existing widgets as in the ones that are in the wp-content/plugins folder, or the existing sidebar(s) setup? The DB option names are different, in order to coexist.

Also, any specifics on how the widgets plugin messes core widgets up?

[technosailor]

I mean the 3rd party widgets are active in the plugin panel, but do not show on the Sidebar Widgets drag and drop panel in wp-admin and don't show up on the blog itself.

As far as widgets in the core, I had to go in and delte the widgets plugin before the core widgets took over. Obviously, this is not the best scenario when we go gold.

[filosofo]

Widgets is ignoring sidebars that are named something other than the default, so that if you have a custom-named sidebar, none of its widgets appear on the appropriate pages (although it does appear in the admin section).

I've attached another patch ( wp-includes_widgets.php.diff20070428 ) that fixes this problem.

[rob1n]

(In [5343]) Move widgets' JS into the <head> of widgets.php. Should fix text/rss widget controls. see #4169

[rob1n]

Automattically disable Automattic Widgets if it's active

[rob1n]

(In [5344]) Automattically deactivate Automattic Widgets, if activated. see #4169

[rob1n]

(In [5345]) Check basename(). see #4169

[rob1n]

(In [5348]) Bring back old behavior for sidebar ID's. Props filosofo. see #4169

[andy]

undo last patch, no more using name as id

[andy]

On deeper review, I found [5348] to be more trouble. It's time to stop using Name for anything other than display.

If a sidebar is registered with no ID, it will be given a number and dynamic_sidebar() must be called with that number. If it has an ID, it must be called by that ID.

Numbered sidebars are ideal for most cases because they are more likely to convey when the theme is switched. If a theme uses IDs, other themes probably won't use them and the user will have different sidebars for different themes. If this is desired, use IDs.

[andy]

widgets.2.diff reverts [5348] and creates a new sidebars_widgets array version. The upgrade logic attempts to match orphaned sidebars to the ones registered by the theme.

[filosofo]

Can you explain why the status quo is trouble? It currently creates an ID from a sanitized version of the name, which doesn't strike me as being qualitatively different from "sidebar-" + an integer. It also seems easier to remember which is the "left sidebar" rather than which is "sidebar-2" or whatever.

[andy]

Yes. It's trouble because sometimes a name is translated or changed. If that happens, the id changes and this makes previously saved versions impossible to find.

This problem affects sidebars and widgets. That is, widgets registered with a translated name cannot be found after changing the translation or changing the blog's language. So widgets should be fixed as well. The fix I had for wordpress.com, and which was inadvertently copied into core, was to add a parameter similar to the sidebar id. I'm not sure how well that works right now, but it's the way to go. Names are for people to use and id's are for programs to use.

[andy]

If a theme has named sidebars, e.g. "Main sidebar" and "Bottom bar", the name parameter should still be used for these and it should be translated. The program needs a non-translated way to refer to the sidebar, so if you want to call dynamic_sidebar('main-sidebar') all you have to do is register your sidebar with that as that id.

Otherwise you pass an integer indicating the order in which the sidebar was registered. That works as well. Users don't see ID's, they see names.

[ryan]

(In [5358]) Widget registration fixes from Andy. For 2.3. see #4169

[ryan]

(In [5359]) Widget registration fixes from Andy. For 2.2. see #4169

[filosofo]

New problem: the maybe_disable_automattic_widgets function doesn't do any good, because what happens is that wp-includes/widgets.php gets included *before* the widgets plugin (hence all the checks in wp-includes/widgets.php for existing widgets functions are worthless), and it gets included *before* upgrade-functions.php has a chance to define maybe_disable_automattic_widgets. You end up with a fatal error, even when just requesting wp-admin/upgrade.php

So it makes sense to check for the widgets plugin in wp-includes/widgets.php then deactivate it.

My patch does that, and it removes the extraneous existing widget function checks.

[rob1n]

(In [5360]) Move widget stuff around. see #4169

[rob1n]

(In [5363]) New way of loading widgets that works with PHP 5. Thanks to filosofo for testing. see #4169

[rob1n]

Any other problems with this, so we can mark this as fixed?

[ryan]

Widgets written against the plugin version of widgets do this:

register_widget_control($name, $i <= $number ?
'widget_adsense_control' : /* unregister */ , 460, 350, $i);

Insead of:

register_widget_control($name, $i <= $number ?
'widget_adsense_control' : /* unregister */ , 460, 350, $id, $i);

$i is being interpreted as an ID and mucking things up.

Distinguishing the intent of that fifth arg, whether it be
an ID or an arbitrary arg being passed along, is difficult. Do we
need to have separate funcs for old and new?

Old:
function register_widget_control($name, $control_callback, $width =
300, $height = 200, ...)

New:
function wp_register_widget_control($id, $name, $control_callback,
$width = 300, $height = 200, ...)

The same for [wp_]register_sidebar_widget(). And update the
plugin with all of the changes we've done in core so that 2.0 and 2.1
users have it.

[ryan]

Let's do this. Remove the $id from register_widget_control() and register_sidebar_widget() to restore them back to the old way. Make them wrappers around the new functions wp_register_widget_control() and wp_register_sidebar_widget(). They will look like:

function wp_register_widget_control($id, $name, $control_callback, $width = 300, $height = 200, $args = array())

function wp_register_sidebar_widget($id, $name, $output_callback, $classname = , $args = array())

Change default widgets to use the new API.

[ryan]

Alternatively:

function wp_register_widget_control($id, $name, $control_callback, $args = array())

Collapse height and width into args.

[ryan]

That patch fixes back compat for me. RSS and recent comments widgets aren't saving their options, though. Also, we need to do an attribute_escape() audit.

[ryan]

Back compat and rss fixes

[ryan]

RSS widget fixed. Registration API fractured for back compat support.

[ryan]

(In [5398]) Fix widget registration back compat by splitting API. Fix feed widget. For 2.2. see #4169

[ryan]

(In [5399]) Fix widget registration back compat by splitting API. Fix feed widget. For 2.3. see #4169

[ryan]

(In [5400]) Make sure Widgets menu comes after Themes. see #4169

[ryan]

(In [5401]) Make sure Widgets menu comes after Themes. see #4169

[ryan]

(In [5402]) attribute_escape for widgets. see #4169

[ryan]

(In [5403]) attribute_escape for widgets. see #4169

[johnbillion]

Under Presentation -> Widgets the sidebar on the left is titled "Sidebar 1" implying there is more than one sidebar available. Is it possible to add more sidebars?

[JeremyVisser]

It is up to the theme to tell WP how many sidebars are in the theme. I think it's in the register_sidebar() function or something like that.

If there's only one sidebar, it probably shouldn't be labelled 'Sidebar 1'.

[Otto42]

Themes can define specific names of the sidebars if they want to do so. If they don't, then you get the default "Sidebar X" stuff.

I say let the theme deal with it. A well made theme will have its own sidebar names anyway.

[rob1n]

Yep.

[ryan]

Someone wrote in with this report.

There is a bug in your widgets.php code. Around line 464 it says:



var left = $('shadow').offsetWidth - (el.offsetWidth + left);



I guess this should be:



var left = $('shadow').offsetWidth - (el.offsetWidth + right);

[Otto42]

That would be line 99 in wp-admin/widgets.php, and yes, he is correct. That line of code will cause a javascript error of some sort, and his fix is correct as well.

[ryan]

(In [5436]) Left, right, dragChange, hop. s/left/right/. Props Lawrence Pit. see #4169

[ryan]

(In [5437]) Left, right, dragChange, hop. s/left/right/. Props Lawrence Pit. see #4169