WordPress.org

Make WordPress Core

Opened 20 months ago

Closed 12 months ago

Last modified 12 months ago

#25335 closed defect (bug) (wontfix)

Twenty Thirteen: plugins cannot filter `the_content` for Gallery post format on index views

Reported by: RDall Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.6
Component: Bundled Theme Keywords: close has-patch
Focuses: Cc:

Description

Using the sharing widget of Jet Pack in Twenty Thirteen the sharing widget doesn't show under the gallery like it does on all other posts.

See Post Format Gallery Screenhot: http://cl.ly/image/1B3f2A1A051y
(It does so on the single post format page, just not the front or search pages event though the search pages are clicked.)

But it does seem to show in Post Format Quote and Page.

See Screenshot: http://cl.ly/image/0X1t120A360l

Note: I did see this on the my own child theme on my own website. But I checked it with Twenty Thirteen and it is replicated.

See Forums post for further info. I first though it was Jetpack and not Twenty Thirteen.

George Stephanis suggests it's due to a ifcheck:

Due to the ifcheck you can see there, it looks like the_content() never actually fires sometimes. As the sharing buttons tie in via a filter on the_content(), that filter would never actually fire.

http://wordpress.org/support/topic/sharing-buttons-not-available-on-post-format-gallery?replies=2#post-4639464

Attachments (2)

25335.diff (828 bytes) - added by obenland 20 months ago.
25335.2.diff (1.2 KB) - added by SergeyBiryukov 12 months ago.

Download all attachments as: .zip

Change History (33)

comment:1 @georgestephanis20 months ago

Well, not that it's due to an ifcheck, it's due to the_content() not firing, so the filters that would append it (and many other non-jetpack things) also don't fire.

@obenland20 months ago

comment:2 follow-up: @obenland20 months ago

  • Keywords has-patch added; needs-patch removed
  • Version changed from trunk to 3.6

This should only be the case on index pages.

I really don't know if manually calling the content filter is a good idea though. :/

comment:3 in reply to: ↑ 2 @georgestephanis20 months ago

Replying to obenland:

I really don't know if manually calling the content filter is a good idea though. :/

Maybe use a filter to swap in get_post_gallery() in for the_content() at a higher priority?

Not sure.

comment:4 @lancewillett20 months ago

  • Summary changed from Twenty Thirteen - Jetpack Sharing buttons not available on post format gallery to Twenty Thirteen - Jetpack Sharing buttons not available on index view for Gallery post format

comment:5 follow-up: @lancewillett20 months ago

I'm leaning toward wontfix on this one, since it's only on index views.

comment:6 @rdall20 months ago

I politely disagree as the index page when the theme is in a blog format is a highly visible page. One would say more then the single post view. This also might suggest that one post format is lesser then the other.

comment:7 @celloexpressions20 months ago

This is more than a little annoying for anyone adding things like sharing buttons. However, I usually just do the_content for gallery posts on index pages in child themes because I prefer displaying the textual context of the gallery anyway. My plugin's users can't really use that workaround, though.

comment:8 @RDall20 months ago

I don't really understand your reasoning for won't fix as the gallery post in the index page is like any other post… But maybe you could enlighten me as to why you see this as an edge case?

comment:9 @RDall20 months ago

Just to be clear if this was another post type say quote, aside or chat I would be making the same justification for a patch as I am for the gallery format here…

Last edited 20 months ago by RDall (previous) (diff)

comment:10 in reply to: ↑ 5 @obenland20 months ago

Replying to lancewillett:

I'm leaning toward wontfix on this one, since it's only on index views.

My proposed solution may not be pretty, but a lot of plugins rely on the filter and I think we should make sure they can do that even on index gallery views.

comment:11 follow-up: @lancewillett20 months ago

  • Summary changed from Twenty Thirteen - Jetpack Sharing buttons not available on index view for Gallery post format to Twenty Thirteen: plugins cannot filter `the_content` for Gallery post format on index views

I think we should be very clear if this is fixed it's not just to patch one plugin like Jetpack. Our decisions need to be based on what's best for the entire community, not just one plugin.

comment:12 in reply to: ↑ 11 @RDall20 months ago

Replying to lancewillett:

Our decisions need to be based on what's best for the entire community, not just one plugin.

I see the slippery slope that you have foreseen I couldn't agree more with that issue It shouldn't be about just one plugin.. If this filter is commonly used then it should be a fixed to provide support for all plugins that might want to use that feature. (make sense?)

Last edited 20 months ago by RDall (previous) (diff)

comment:13 follow-up: @celloexpressions20 months ago

This definitely effects a ton of plugins, pretty much anything adding social sharing. There are bigger usability problems here with not displaying the_content, but fixing plugin filtering is a first step (not that anything else will be changed at this point).

comment:14 in reply to: ↑ 13 ; follow-up: @obenland20 months ago

Replying to celloexpressions:

There are bigger usability problems here with not displaying the_content [...]

Can you be more specific?

comment:15 @georgestephanis20 months ago

This is why we had done the_remaining_content()/the_extra_content() back with Post Formats.

For just this reason, I'd support adding them back in.

comment:16 in reply to: ↑ 14 @celloexpressions20 months ago

Replying to obenland:

Replying to celloexpressions:

There are bigger usability problems here with not displaying the_content [...]

Can you be more specific?

Well, I don't think site visitors realize that there is/might be additional content visible when they go to the single post page (and they may not know how to get there, despite the conventions). Since we see (almost?) everything else on the index views for (every?) other post format, hiding the content for gallery posts and just displaying the gallery breaks the pattern and you wouldn't expect any content to be hidden.

I'm actually surprised that there haven't (?) been complaints from new-ish users who think the non-gallery part of a gallery post has disappeared (probably a combination of people not understanding/using post formats and people landing on single view pages from the view post links in the admin, rather than looking at their main blog page). Personally, I like to provide a short paragraph for context at the beginning of a gallery [post], so losing the context on index pages is problematic. A lot of it depends on what's in the gallery, if it's just a bunch of pictures of food or scenery or something, the post title typically provides adequate context, but if it's about a project or event context may be needed, depending on the organization.

comment:17 follow-up: @lancewillett14 months ago

  • Keywords close added; has-patch removed

Suggesting we close this one.

comment:18 @lancewillett14 months ago

  • Milestone Awaiting Review deleted

comment:19 @ircbot14 months ago

This ticket was mentioned in IRC in #wordpress-dev by RDall. View the logs.

comment:20 in reply to: ↑ 17 @RDall14 months ago

  • Keywords needs-patch added

Replying to lancewillett:

Suggesting we close this one.

Ok Lance I make compromise with you.

If I find someone to patch this ticket. Will you at least review it?

comment:21 @nacin14 months ago

  • Milestone set to Awaiting Review

comment:22 @SergeyBiryukov14 months ago

  • Keywords has-patch added; needs-patch removed

25335.diff seems like a valid patch for most of the social sharing plugins, not just for Jetpack.

A workaround:

function apply_the_content_to_get_post_gallery_25335( $gallery ) {
	if ( ! is_single() ) {
		$gallery = apply_filters( 'the_content', $gallery );
	}

	return $gallery;
}
add_filter( 'get_post_gallery', 'apply_the_content_to_get_post_gallery_25335' );

comment:23 @nacin14 months ago

I don't think this is the intended use of get_post_gallery() — I mean, I guess it is, but I would have expected the_content() to be used here. get_post_gallery() is more of a utility, which is partially why there isn't a the_* variant. Is there a compelling reason to avoid the_content() here?

comment:24 @RDall14 months ago

I believe (correct me if I am wrong) this was used because it was part of the Post Format feature that never got committed and then in this theme all the other post formats were changed except for this post format and only in the index / archive view.

In conclusion from what I have learnt it was just never reverted back.

comment:25 @obenland14 months ago

Joen's design called for just the gallery itself on index, not the entire content. Like using a featured image as an representation of the post. Just a gallery on gallery formats.

@SergeyBiryukov12 months ago

comment:26 @SergeyBiryukov12 months ago

25335.2.diff always uses the_content() in content-gallery.php (per comment:23), which makes it consistent with content-image.php.

comment:27 follow-up: @obenland12 months ago

I'd be very sad if we'd just display the content on index for gallery post formats.

The purpose of Twenty Thirteen was to show off post formats and display them in a special way. After PFUI didn't make it into 3.6, a lot of the things that Joen had envisioned for the design had to be removed, with the gallery being one of the few exceptions.

Additionally, changing it to the_content() would change the display of gallery posts on index for existing installs significantly, with no visual backwards compatibility.

comment:28 @lancewillett12 months ago

I vote to wontfix as the current behavior is working as expected.

comment:29 @RDall12 months ago

As much as I have advocated for this change… I see that this is a damned if we do and damned if we don't scenario as twenty thirteen seems to be hobbled by functionality that never made it into core.

Can't say I didn't try to get this solved though…

comment:30 @wonderboymusic12 months ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

Based on the discussion above, wontfix.

comment:31 in reply to: ↑ 27 @SergeyBiryukov12 months ago

Replying to obenland:

I'd be very sad if we'd just display the content on index for gallery post formats.
...
Additionally, changing it to the_content() would change the display of gallery posts on index for existing installs significantly, with no visual backwards compatibility.

FWIW, get_post_gallery() extracts the gallery from post content, not from attached images.

So changing it to the_content() would not remove the galleries, it would just add content.

Note: See TracTickets for help on using tickets.