Make WordPress Core

Opened 10 years ago

Last modified 3 years ago

#21082 accepted defect (bug)

The Gallery shortcode relies on #gallery-instance to hook the CSS, but the id sometimes isn't specific enough

Reported by: matveb Owned by: obenland
Milestone: Priority: normal
Severity: normal Version:
Component: Gallery Keywords: needs-testing has-patch
Focuses: Cc:


The Gallery CSS selector uses the id attribute, which is of the form #gallery-{instance}. On some scenarios (like infinite scroll) a follow up gallery on a another post can have the same #gallery-x selector, thus over-writing the previous gallery style.

Proposed solution: make the #id more unique by including the postid number. (Attached patch.)

(It could potentially impact some themes that used #gallery-1 to style galleries, but theme's shouldn't be using that in the first place, I believe.)

Furthermore, it may be good to change the whole style block to use classes instead of the id so that it's easier for themes to deal with CSS specificity—and helps them not fall back to the #id.

Attachments (3)

21082.diff (480 bytes) - added by matveb 10 years ago.
21082.2.diff (1.7 KB) - added by MikeHansenMe 9 years ago.
21082.3.diff (1.2 KB) - added by antpb 6 years ago.

Download all attachments as: .zip

Change History (14)

10 years ago

#1 @lancewillett
10 years ago

  • Cc lancewillett added

+1 to not styling with ID values, in the core CSS output.

9 years ago

#2 @MikeHansenMe
9 years ago

  • Cc mdhansen@… added
  • Keywords needs-testing added

This patch moves the general styles to use the class and also takes advantage of the $instance var to only add it once. This will likely still be added on the first gallery of each infinite scroll load. This is still much better than being added to every gallery.

#3 @DrewAPicture
9 years ago

I like where you're going with 21082.2 but if we're going to remove $selector from the general styles, I think I'd much rather have them printed via wp_head than in with the galleries.

#4 follow-up: @MikeHansenMe
9 years ago

Yea, I thought the same thing but I do not think you can add anything to wp_head from within a shortcode. I think it is to late. I can/will check into that.

#5 in reply to: ↑ 4 @kovshenin
9 years ago

Replying to MikeHansenMe: wp_head is indeed too late for shortcodes.

#6 @ethitter
9 years ago

  • Cc erick@… added

#7 @MikeHansenMe
8 years ago

  • Keywords needs-refresh added; has-patch removed

#8 @antpb
6 years ago

  • Keywords has-patch added; needs-refresh removed

Refreshed the patch below...

6 years ago

#9 @obenland
6 years ago

  • Milestone changed from Awaiting Review to 4.5
  • Owner set to obenland
  • Status changed from new to accepted

21082.diff has been running on WP.com without issues for 3.5 years now.

The other patches still need a good amount of testing. I think we should search the theme repo to see if themes do use the id selector and try to bring this in early in the cycle to give it time to soak.

#10 @obenland
6 years ago

53 themes in the repo use a #gallery-1 selector in one way or another.

#11 @obenland
6 years ago

  • Milestone changed from 4.5 to Future Release
Note: See TracTickets for help on using tickets.