Make WordPress Core

Opened 5 years ago

Last modified 2 years ago

#21569 new enhancement

Filter gallery styles with access to $attr

Reported by: betzster Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.5
Component: Media Keywords: has-patch needs-refresh
Focuses: Cc:


Currently you can replace the entire output, but that's all or nothing — if you decide to do that, you also need to parse the attributes array, decide on good defaults, etc. You can also filter the styles, but you don't have access to the attributes array.

Use case: In my theme, I'd like to use the number of columns as more of a suggestion. I could set min-width to what $itemwidth is currently and it would be more responsive. If the page shrinks too small, it would only load 1 image per line instead of being locked into the right number of columns.

Attachments (2)

media.diff (1.8 KB) - added by drywallbmb 5 years ago.
patch referenced in comment 5
21569.diff (1.8 KB) - added by markoheijnen 5 years ago.

Download all attachments as: .zip

Change History (13)

#2 @markoheijnen
5 years ago

  • Milestone changed from Awaiting Review to 3.6
  • Version set to 3.5

I think we should do this for 3.6 too. The gallery shortcode is broken as it is. #15155 fixes it in a way. I also think that '<br style="clear: both" />' should be an argument. So people can easily clean it out or replace it with something else like a <hr>.

#3 @drywallbmb
5 years ago

I'd like to see two things happen here at the least. These are simple but could be handy...

  1. Add a gallery_output filter. This could/should be called around line 802 of wp-includes/media.php, right before returning $output.
  2. Fix line 299 of wp-includes to use double-quotes around the style attribute as in line 795. (or, alternatively, change line 795 to use single quotes). This is minor but the inconsistency is annoying.

I'd submit a patch but I haven't even done so before and it'll take me a while to remember how to use SVN, I've been on git for years... :)

Last edited 5 years ago by drywallbmb (previous) (diff)

#4 @markoheijnen
5 years ago

The gallery_output filter doesn't make sense to me. There is already the filter 'gallery_style'.

Your second point isn't inconsistency. That is how WordPress does it. I also don't like it. You are right if you meant the splitter (<br/>). One is with single and the other with double quotes.

5 years ago

patch referenced in comment 5

#5 @drywallbmb
5 years ago

  • Keywords has-patch added

The problem is that the gallery_style filter only runs on the string before it's actually been populated with items. Thus, you can't use to, for example, use a str_replace to get rid of all the <br> elements, because they haven't been tacked onto $output yet.

And yes, sorry for the ambiguity, I meant the splitter <br>'s — in the loop they're being output with double quotes in the HTML and after the loop the final <br> element is being output with single quotes around the style attribute.

For giggles (and/or clarity), I'm attaching my proposed patch, which normalizes all HTML element attribute values generated by gallery_shortcode() to be double-quoted, and adds the gallery_output filter making it possible to filter the entire gallery's markup. As mentioned, I'm rusty with SVN so apologies if the patch is borked.

#6 @markoheijnen
5 years ago

The patch isn't going to happen like this. Everywhere in WordPress they use single quotes for HTML. See enqueueing script and styles. So the splitter (<br/>) as an argument is the solution for this ticket.

The filter still doesn't make sense since the <br/> elements should be an argument. Doing a str_replace or a regex is wrong to do. Then you can better use the filter 'post_gallery'.

5 years ago

#7 @markoheijnen
5 years ago

I added a patch that creates a splitter argument and uses that. I choose for double quotes since it is better fitting here. I also added a additional class: gallery-link-{$attrlink?}. This gist show the reason why: https://gist.github.com/markoheijnen/4635255

@drywallbmb: About your patch. I also should have said that that kind of escaping isn't really what you will find in WordPress core code. It's then better too change from double quotes to single quotes. Unless it isn't posible what you can see in regex's.

#8 @drywallbmb
5 years ago

I'm with you 100% here. Using a $splitter argument eliminates the need for the filter I was proposing, and also nicely does away with the quotation mark inconsistency I was trying to eliminate. Nice work, let's all just pretend I never said anything. :)

Now what happens?

#9 @DrewAPicture
5 years ago

  • Keywords punt added

#15155 allowed us to filter shortcode attributes going forward with [23626].

I'd guess it's probably too late to add the $splitter arg and juggle up $link in time for 3.6.

#10 @SergeyBiryukov
5 years ago

  • Keywords punt removed
  • Milestone changed from 3.6 to Future Release

#11 @chriscct7
2 years ago

  • Keywords needs-refresh added
Note: See TracTickets for help on using tickets.