Make WordPress Core

Opened 13 years ago

Closed 10 years ago

#24604 closed defect (bug) (fixed)

replace id atribute with class for calendar_wrap value of the Calendar Widget

Reported by: alexvorn2's profile alexvorn2 Owned by: chriscct7's profile chriscct7
Milestone: 4.4 Priority: normal
Severity: normal Version:
Component: Widgets Keywords: has-patch
Focuses: Cc:

Description

I think the echo '<div id="calendar_wrap">'; should be replaced with echo '<div class="calendar_wrap">'; because having the same id on the pages is not valid, if we will have two calendars on the page.

Attachments (4)

calendar_wrap.diff (500 bytes) - added by transom 13 years ago.
Changes id="calendar_wrap" to class="calendar_wrap" in default widget
24604.diff (926 bytes) - added by MikeHansenMe 12 years ago.
Only show the id once.
24604.2.diff (930 bytes) - added by MikeHansenMe 12 years ago.
24604.3.diff (984 bytes) - added by MikeHansenMe 10 years ago.

Download all attachments as: .zip

Change History (11)

@transom
13 years ago

Changes id="calendar_wrap" to class="calendar_wrap" in default widget

#2 @transom
13 years ago

The issues in #16539 are somewhat applicable in that it is possible that third-party themes are looking for the #calendar_wrap div. That said, the core themes do not address that #calendar_wrap. This change is not as critical given that the #calendar_wrap is not attached to an input field (or label) and unlikely to negatively impact screen readers.

The benefit is semantically correct markup. The negative is that a third-party theme may be looking for the #calendar_wrap

#3 @transom
13 years ago

  • Keywords has-patch added; needs-patch removed

#4 @nacin
13 years ago

Normally we'd try to print #calendar_wrap on the first instance, and hide it from all subsequent instances. That way it's backwards compatible at least for a primary use case (one calendar, styled by a theme).

I think when adding a class name (which we'd do for all instances), we can probably come up with a better name in the process.

@MikeHansenMe
12 years ago

Only show the id once.

#5 @chriscct7
10 years ago

  • Keywords needs-refresh added
  • Milestone changed from Awaiting Review to 4.4
  • Owner set to chriscct7
  • Severity changed from minor to normal
  • Status changed from new to assigned

#6 @MikeHansenMe
10 years ago

  • Keywords needs-refresh removed

Refreshed with a strict check.

#7 @wonderboymusic
10 years ago

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

In 34381:

Widgets: add a static property to WP_Widget_Calendar to ensure that the id attribute is only output once.

Props MikeHansenMe, wonderboymusic.
Fixes #24604.

Note: See TracTickets for help on using tickets.