Make WordPress Core

Opened 14 years ago

Closed 21 months ago

#14819 closed defect (bug) (wontfix)

Gallery shortcode output does not validate.

Reported by: jameslaws's profile jameslaws Owned by: chriscct7's profile chriscct7
Milestone: Priority: normal
Severity: normal Version: 3.0.1
Component: Gallery Keywords: has-patch needs-testing
Focuses: Cc:

Description

I was validating a sites markup when I realized that the gallery shortcode output does not validate where know "caption" exists due to the lack of an associated value. Basically a <dt> must be followed by at least one <dd>.

Not sure if dl is the best tag to use for outputting galleries since I imagine lot's of people exclude captions from their galleries. Not sure what the best solution is at the moment.

Thoughts?

This is my first submission so be gentle.

Attachments (2)

wp301-gallery-patch-ref-14819.patch (557 bytes) - added by t31os_ 14 years ago.
Add empty dd for gallery with empty captions
14819.diff (546 bytes) - added by mdawaffe 13 years ago.

Download all attachments as: .zip

Change History (12)

@t31os_
14 years ago

Add empty dd for gallery with empty captions

#1 follow-up: @t31os_
14 years ago

  • Cc wp-t31os@… added
  • Keywords has-patch needs-testing added

I assume an empty dd tag is valid, so i've simply added an extra conditional statement into the gallery_shortcode function.

Patch is against 3.0.1 (finally figured out how that works), but i can re-do if any issues.

#2 in reply to: ↑ 1 @jameslaws
14 years ago

Replying to t31os_:

I assume an empty dd tag is valid, so i've simply added an extra conditional statement into the gallery_shortcode function.

Patch is against 3.0.1 (finally figured out how that works), but i can re-do if any issues.

I tested your patch but it doesn't seem to change the output in any way. Source and validation errors remain.

#3 @t31os_
14 years ago

It change the output for gallery shortcode, the code works perfectly fine for me (i was working on a gallery page when i spotted your ticket), so i was able to confirm that firstly the gallery shortcode did not produce the dd tags, and secondly that adding the new condition ensures empty tags are now introduced.

All i did was wrap up the change into a patch for testing.

Can you confirm the patch applied correctly and that the new code exists in wp-includes/media.php.

#4 @jameslaws
14 years ago

I simply placed the code you added into my media.php file just as you had it and uploaded. I cleared my cache and visited a page outputting a gallery from a shortcode and viewed the source. There were no new dd tags.

I'm positive the code is exactly the same as your patch and is in wp-includes/media.php. Not sure if there is something I else I would need to do but it seemed like that would be all.

#5 @t31os_
14 years ago

It works for me, perhaps another Trac user can confirm if the patch applies for them.

@mdawaffe
13 years ago

#6 @mdawaffe
13 years ago

  • Milestone changed from Awaiting Review to Future Release

Patch works for me. Refreshed.

#7 follow-up: @nacin
12 years ago

  • Component changed from Validation to Gallery

I wonder if a change like this, while resulting in validation, could break a site visually. (Empty but styled caption boxes.)

#8 in reply to: ↑ 7 @iseulde
10 years ago

Replying to nacin:

I wonder if a change like this, while resulting in validation, could break a site visually. (Empty but styled caption boxes.)

It would. Twenty Thirteen is an example. See #28089.

#9 @chriscct7
8 years ago

  • Owner set to chriscct7
  • Status changed from new to reviewing

#10 @desrosj
21 months ago

  • Resolution set to wontfix
  • Status changed from reviewing to closed

I'm going to close this out. Changing this would definitely be bad for backwards compatibility in themes.

However, after this ticket was created the markup output by a gallery shortcode was changed in [27302] to use <figure>, <div> and <figcaption> tags instead when a theme opts in to HTML5 markup using add_theme_support( 'html5', 'gallery' ); (see #26697). So while this was not technically fixed, it is fixed in the sense that valid HTML5 markup can be used instead of the <dl>, <dt> and <dd> tags that were used previously.

I've opened up #56206 as a follow up to this to improve the historical references within the function's documentation in the form of @since tags documenting when certain attributes were added and changes like the ones in [27302] were made.

Note: See TracTickets for help on using tickets.