WordPress.org

Make WordPress Core

Opened 8 years ago

Last modified 3 years ago

#14819 reviewing defect (bug)

Gallery shortcode output does not validate.

Reported by: jameslaws Owned by: chriscct7
Milestone: Future Release 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_ 8 years ago.
Add empty dd for gallery with empty captions
14819.diff (546 bytes) - added by mdawaffe 7 years ago.

Download all attachments as: .zip

Change History (11)

@t31os_
8 years ago

Add empty dd for gallery with empty captions

#1 follow-up: @t31os_
8 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
8 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_
8 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
8 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_
8 years ago

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

@mdawaffe
7 years ago

#6 @mdawaffe
7 years ago

  • Milestone changed from Awaiting Review to Future Release

Patch works for me. Refreshed.

#7 follow-up: @nacin
6 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
4 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
3 years ago

  • Owner set to chriscct7
  • Status changed from new to reviewing
Note: See TracTickets for help on using tickets.