widgets randomly get added in two or more sidebars

[Denis-de-Bernardy]

we've yet to identify a means to reproduce, but here goes some IRC conversations:

[03:18] <ddebernardy> I haven't seen it occur in over a week or so, but I've no idea of whether it's due to my not testing widgets moving (apart from adding one and testing as I rewrite it)

[03:19] <junsuijin> well i was wondering if it had to do with settings from widgets saved into the db before the most recent 2.8 is all

[03:20] <junsuijin> and it inconsistently updating those ids

[03:21] <ddebernardy> I suspect, personally, that it may have to do with a jquery bug

[03:21] <junsuijin> i can meticulously go back and set up the exact widgets i had before clearing my db i suppose, or just grab the db option from the blog i have set up heh

[03:21] <ddebernardy> like, when you have several sidebars open

[03:21] <ddebernardy> and hover a widget over them all

[03:21] <junsuijin> yeah i was thinking that too

[03:21] <junsuijin> but i've got several open right now

[03:21] <ddebernardy> and erroneously drop the widget in one

[03:21] <junsuijin> i'm wondering maybe if you drag 2 widgets and they don't finish saving

[03:21] <ddebernardy> immediately click on it

[03:21] <ddebernardy> and move it in a separate sidebar

[03:22] <junsuijin> yeah

[03:22] <junsuijin> sort of a similar scenario

[03:22] <junsuijin> basically initiating another ajax request before the other finishes

[03:22] <ddebernardy> that scenario seems likely to end us with the same widget in two sidebars

[Denis-de-Bernardy]

it might be worth adding a check somewhere, if it doesn't exist already, to verify that a widget doesn't exist in a separate sidebar before adding it.

[junsuijin]

Another note is that when the duplication happened it would sometimes overwrite other existing widgets with the duplicate widget, sometimes just add it to a different sidebar (not always one that was open at the time).

[junsuijin]

Just figured out a bit more info...
Starting from having no widgets, manually cleaning the db option.
I kept watch of the AJAX POST for widgets-order responses, and the widgets would number as expected, where each widget had widget-12_rig-html-16, widget-12_rig-html-17, etc., where the widget-12 corresponds to the type of widget, unique to each type of widget in the available widgets list.

I press F5, then click the Widgets admin menu link to totally reload the page.
I attempt to put a new rig-html widget into sidebar-1 and then quickly move it to sidebar-2.
I note 3 AJAX POSTs now, all numbering the widgets like widget-21_rig-html-16, widget-22_rig-html-17, where the widget-number corresponds to the order of active widgets from first registered sidebar to last registered sidebar. The widget did still get moved correctly, but this is inconsistent behavior, and could account for the widget overwriting/duping, since the new widget gets added as a widget-12 as expected, but amongst widgets of order determined by their place in the list of active widgets.

Example (note widget-12_rig-html-33):

---

RESPONSE 1

---

action
widgets-order
savewidgets
4ba6c52c16
sidebars[after-comments]
sidebars[after-content]
sidebars[after-title]
sidebars[after-welcome-content]
widget-44_pages-2,widget-45_links-2,widget-46_tag_cloud-2
sidebars[backgrounds]
widget-49_rig-html-20
sidebars[before-content]
sidebars[before-welcome-content]
sidebars[features]
widget-40_text-7,widget-41_rig-recent-posts-3,widget-42_rig-recent-comments-3,widget-43_categories-2
sidebars[footer]
widget-47_rig-seo-footer,widget-48_rig-html-21
sidebars[header]
widget-38_rig-seo-header,widget-39_rig-html-22
sidebars[scripts]
sidebars[sidebar-1]
widget-21_rig-html-15,widget-22_rig-seo-sidebar,widget-23_rig-html-23,widget-24_rig-html-24,widget-25_rig-html-28
,widget-26_rig-html-29,widget-27_rig-html-30,widget-28_rig-html-32,widget-29_rig-html-31,widget-30_rig-html-16
,widget-12_rig-html-33
sidebars[sidebar-2]
widget-31_rig-html-17,widget-32_rig-html-25,widget-33_rig-html-19,widget-34_rig-html-26,widget-35_text-6
,widget-36_rig-html-27,widget-37_rig-html-18
sidebars[sidebar-top]
sidebars[wp_inactive_widgets]

---

RESPONSE 2

---

action
widgets-order
savewidgets
4ba6c52c16
sidebars[after-comments]
sidebars[after-content]
sidebars[after-title]
sidebars[after-welcome-content]
widget-44_pages-2,widget-45_links-2,widget-46_tag_cloud-2
sidebars[backgrounds]
widget-49_rig-html-20
sidebars[before-content]
sidebars[before-welcome-content]
sidebars[features]
widget-40_text-7,widget-41_rig-recent-posts-3,widget-42_rig-recent-comments-3,widget-43_categories-2
sidebars[footer]
widget-47_rig-seo-footer,widget-48_rig-html-21
sidebars[header]
widget-38_rig-seo-header,widget-39_rig-html-22
sidebars[scripts]
sidebars[sidebar-1]
widget-21_rig-html-15,widget-22_rig-seo-sidebar,widget-23_rig-html-23,widget-24_rig-html-24,widget-25_rig-html-28
,widget-26_rig-html-29,widget-27_rig-html-30,widget-28_rig-html-32,widget-29_rig-html-31,widget-30_r
ig-html-16
sidebars[sidebar-2]
widget-31_rig-html-17,widget-32_rig-html-25,widget-33_rig-html-19,widget-34_rig-html-26,widget-35_text-6
,widget-36_rig-html-27,widget-37_rig-html-18
sidebars[sidebar-top]
sidebars[wp_inactive_widgets]

---

RESPONSE 3

---

action
widgets-order
savewidgets
4ba6c52c16
sidebars[after-comments]
sidebars[after-content]
sidebars[after-title]
sidebars[after-welcome-content]
widget-44_pages-2,widget-45_links-2,widget-46_tag_cloud-2
sidebars[backgrounds]
widget-49_rig-html-20
sidebars[before-content]
sidebars[before-welcome-content]
sidebars[features]
widget-40_text-7,widget-41_rig-recent-posts-3,widget-42_rig-recent-comments-3,widget-43_categories-2
sidebars[footer]
widget-47_rig-seo-footer,widget-48_rig-html-21
sidebars[header]
widget-38_rig-seo-header,widget-39_rig-html-22
sidebars[scripts]
sidebars[sidebar-1]
widget-21_rig-html-15,widget-22_rig-seo-sidebar,widget-23_rig-html-23,widget-24_rig-html-24,widget-25_rig-html-28
,widget-26_rig-html-29,widget-27_rig-html-30,widget-28_rig-html-32,widget-29_rig-html-31,widget-30_r
ig-html-16
sidebars[sidebar-2]
widget-31_rig-html-17,widget-12_rig-html-33,widget-32_rig-html-25,widget-33_rig-html-19,widget-34_rig-html-26
,widget-35_text-6,widget-36_rig-html-27,widget-37_rig-html-18
sidebars[sidebar-top]
sidebars[wp_inactive_widgets]

[Denis-de-Bernardy]

Replying to junsuijin:

Another note is that when the duplication happened it would sometimes overwrite other existing widgets with the duplicate widget, sometimes just add it to a different sidebar (not always one that was open at the time).

that part is normal. it's the same widget, after all.

[Denis-de-Bernardy]

k, so based on your comments, the diagnosis is correct -- we might be missing a check, over in widgets, to double-check for existing widgets with the same ID in sidebars, to work around race conditions

[junsuijin]

Well you probably have to look at the source of this page to be able to read those responses sensibly, but basically, the admin-ajax.php sets the widgets option every time it does this response. There would be a problem when adding say, the 21st available widget to sidebar-1 in the example, as it is not actually a widget-21.

[azaozz]

The widget-##_ part of the id is not used when saving the positions and sidebars, it's a temp id for the widget only. Think I have an idea what's going on there. See if the attached patch fixes it.

[Denis-de-Bernardy]

odd... didn't get any email as this ticket was being updated...

@junsuijin -- can you try the patch?

[junsuijin]

Not 100% sure this is the only means to reproduce the bug. It seems that if I load up several instances of any given widget, and then remove an instance with an id in the middle of the many others, then proceed to reload the admin widgets.php page, the id count for newly added instances of the given multi-widget starts from the now-unused id in the middle, which allows for any added instances past the first (or however many unused ids there are in the middle), to overwrite (duplicating themselves) the consecutive instances past the unused id(s), since additional new instance ids continue incrementing from the first added instance.

I patched the 2.8 release's wp-admin/includes/widgets.php with azaozz's patch. After doing so new instances are always given an id past the highest instance id, instead of trying to utilize the unused id(s) in the middle. This effectively eliminates the problem since new instance ids cannot increment over established instance ids.

I made a db backup at the point where the bug can be reproduced in case anyone needs it, but I think the described situation is the cause.

[Denis-de-Bernardy]

Aaaah, now I get it. So, to reproduce, do the following:

add three widgets. Delete the send one. Add yet another widget. Without the patch, we get a dup id, with the patch, we no longer do.

[Denis-de-Bernardy]

Delete he second one even.

[junsuijin]

Yes Denis; but to clarify slightly, after deleting the second widget instance, add two more instances (adding just one would not pose a problem). Or, add 1 instance and remove it, and add another. Both ways produce the same effect.

The key though, is that after removing that second widget instance, one must physically reload the widgets.php admin page, via the menu link or by pressing enter with the address bar selected (a simple F5 refresh will not do the job). Without 'leaving' the widgets.php page, the instance id count is unaffected (so in a case where a user would log out after deleting the middle widget instance, and then the user returns later, they could experience the problem, if they add more than one instance of that type of widget).

[azaozz]

(In [11576]) Do not reuse deleted widget instances IDs, fixes #10092 for 2.8.1

[azaozz]

(In [11577]) Do not reuse deleted widget instances IDs, fixes #10092 for trunk