Make WordPress Core

Opened 17 years ago

Closed 16 years ago

Last modified 16 years ago

#4287 closed defect (bug) (fixed)

Links widget has duplicate ID issue

Reported by: otto42's profile Otto42 Owned by: mdawaffe's profile mdawaffe
Milestone: 2.5.1 Priority: normal
Severity: normal Version: 2.2
Component: General Keywords: has-patch commit
Focuses: Cc:

Description (last modified by rob1n)

wp_list_bookmarks can generate several "blocks" in the sidebar, one per category. In combination with the links widget, this can produce several list items (LI) all using the same id. This will cause the site to fail validation, since there are duplicated ID's.

The reason this happens is because dynamic_sidebar() builds the "before_widget" string using replacements for $1 and $2 as the id and class, respectively. This string then gets passed to wp_list_bookmarks as the "category_before" parameter.

What really needs to occur, in this particular case, is that the original, unreplaced, before_widget string needs to get "$1" replaced with "%id", allowing the normal wp_list_bookmarks code to work correctly, ensuring a unique ID for each of the bookmark blocks.

What the right was to do this is, I'm not entirely certain. We could copy the original string to $params[0]['original_before_widget'] and then let wp_widget_links handle it correctly by doing its own replacement. That would work.

Attachments (1)

wp-includes-widgets.php.patch (467 bytes) - added by docwhat 16 years ago.

Download all attachments as: .zip

Change History (16)

#1 @Otto42
17 years ago

Reference to somebody pointing out this issue:
http://wordpress.org/support/topic/118321

#2 @Otto42
17 years ago

Correction: dynamic_sidebar is replacing %1$s and %2$s with the id and class, respectively. My mistake.

#3 @rob1n
17 years ago

  • Description modified (diff)

#4 @rob1n
17 years ago

  • Milestone changed from 2.2.1 to 2.2.2

#5 @docwhat
17 years ago

Here's a stupid simple fix. Add this right before the call to wp_list_bookmarks() in wp_widget_links():

$before_widget = preg_replace('/id="[^"]*"/','id="%id"', $before_widget);

#6 @Otto42
17 years ago

  • Milestone changed from 2.2.2 to 2.3 (trunk)

#7 @ryan
17 years ago

  • Milestone changed from 2.3 to 2.4

#8 @docwhat
17 years ago

The impact of this bug is rather serious. I don't know why it keeps getting bumped even though there is a simple one line fix for it. It may not be the *best* fix, but it is a fix and it'll work reliably.

Without it CSS and JavaScript cannot do the right thing with respect to the link widget and the (X)HTML is not valid.

This is a huge problem for themes like Sandbox, where you need the markup to be valid to get the CSS to work.

Ciao!

#9 @Otto42
16 years ago

  • Milestone changed from 2.6 to 2.5.1

Any chance of this getting in soon? Still happening in 2.5.

#10 @lloydbudd
16 years ago

  • Owner changed from anonymous to mdawaffe

#11 follow-up: @hakre
16 years ago

Maybe this does not get in because of the new way widgets are handled in version 2.5. Does this patch works with the current widget-admin? Maybe it is related to the the ID-problems there (#6549) as well?

#12 in reply to: ↑ 11 @docwhat
16 years ago

  • Keywords has-patch added

Replying to hakre:

Maybe this does not get in because of the new way widgets are handled in version 2.5. Does this patch works with the current widget-admin? Maybe it is related to the the ID-problems there (#6549) as well?

I don't think #6549 is related at all. That's the widget admin page, not the normal views.

The patch still works just fine, though fuzzy. I'll upload a non-fuzzy one in a second. I'm using it on my blog (http://docwhat.gerf.org/) right now. Without it I can't have the nice collapsing Friends & Links.

Ciao!

#13 @mdawaffe
16 years ago

  • Keywords commit added

+! untested

#14 @ryan
16 years ago

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

(In [7723]) Make link widget IDs unique. Props docwhat. fixes #4287 for 2.5

#15 @ryan
16 years ago

(In [7724]) Make link widget IDs unique. Props docwhat. fixes #4287 for trunk

Note: See TracTickets for help on using tickets.