WordPress.org

Make WordPress Core

Opened 16 months ago

Closed 13 months ago

Last modified 10 months ago

#22960 closed defect (bug) (fixed)

Add functions to extract images from content, attached to post, [gallery], or arbitrary string

Reported by: kcssm Owned by: markjaquith
Milestone: 3.6 Priority: normal
Severity: normal Version: 3.5
Component: Post Formats Keywords: needs-codex has-unit-tests has-patch commit
Focuses: Cc:

Description

Sorry if this is not a bug. If we create the gallery with the existing image from the media, we cannot get the images from that post using get_children() or do_shortcode() or get_posts(). It only returns which are actual attachments to that post. And as of WordPress 3.5, galleries are no more related to particular post. Any thoughts on this?

Attachments (8)

22960.diff (3.7 KB) - added by wonderboymusic 14 months ago.
22960.2.diff (3.8 KB) - added by markjaquith 13 months ago.
refresh
22960.3.diff (5.9 KB) - added by wonderboymusic 13 months ago.
22960.4.diff (5.6 KB) - added by wonderboymusic 13 months ago.
22960.5.diff (8.4 KB) - added by wonderboymusic 13 months ago.
22960-tests.diff (6.0 KB) - added by wonderboymusic 13 months ago.
22960.6.diff (8.9 KB) - added by wonderboymusic 13 months ago.
22960.7.diff (848 bytes) - added by wonderboymusic 13 months ago.

Download all attachments as: .zip

Change History (58)

comment:1 SergeyBiryukov16 months ago

  • Component changed from General to Media

comment:3 SergeyBiryukov16 months ago

  • Keywords needs-codex added
  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Severity changed from major to normal
  • Status changed from new to closed

And as of WordPress 3.5, galleries are no more related to particular post.

Related: #22439

The ability to specify attachment IDs in a gallery shortcode isn't new to 3.5, it just got a handy UI.

You can grab attachment IDs from a gallery with a snippet like this:

function grab_attachment_ids_from_gallery() {
	global $post;

	$attachment_ids = array();
	$pattern = get_shortcode_regex();

	if ( preg_match_all( "/$pattern/s", $post->post_content, $matches ) ) {
		if ( $key = array_search( 'gallery', $matches[2] ) )
			$atts = shortcode_parse_atts( $matches[3][ $key ] );
		if ( isset( $atts['ids'] ) )
			$attachment_ids = explode( ',', $atts['ids'] );
	}

	echo '<pre>'; print_r( $attachment_ids ); echo '</pre>';
}
add_action( 'wp', 'grab_attachment_ids_from_gallery' );

comment:4 nacin16 months ago

  • Milestone set to 3.6
  • Resolution invalid deleted
  • Status changed from closed to reopened

We're going to want to do an API around this.

comment:5 DrewAPicture16 months ago

  • Cc xoodrew@… added

comment:6 philiparthurmoore16 months ago

  • Cc philip@… added

comment:7 sixhours16 months ago

  • Cc caroline@… added

comment:8 iandstewart16 months ago

  • Cc ian@… added

comment:9 iamtakashi16 months ago

  • Cc takashi@… added

comment:10 mfields16 months ago

  • Cc michael@… added

comment:11 ethitter16 months ago

  • Cc erick@… added

comment:12 Jayjdk16 months ago

  • Cc kontakt@… added

comment:14 jond3r16 months ago

  • Cc jond3r@… added

comment:15 greenshady16 months ago

  • Cc justin@… added

comment:16 sabreuse15 months ago

  • Cc sabreuse added

comment:17 cais15 months ago

  • Cc edward.caissie@… added

comment:18 adamsilverstein15 months ago

  • Cc adamsilverstein@… added

comment:20 lancewillett14 months ago

Bump! Chatting with Helen about this yesterday in terms of default themes and gallery post formats.

Twenty Eleven and Twenty Thirteen both have code to grab the 1st image from the gallery, so having a core function would allow us to remove a whole bunch of code.

comment:21 wonderboymusic14 months ago

yeah, I have been adding patches per post format, I just ran out of steam last night - this is forthcoming

comment:22 wonderboymusic14 months ago

  • Summary changed from grab the images of a gallery to Add functions to extract images from content, attached to post, [gallery], or arbitrary string

wonderboymusic14 months ago

comment:23 wonderboymusic14 months ago

  • Keywords has-patch added

attachment:22960.diff provides utility functions to get all image srcs in post_content and / or images attached to the post plus image src(s) from gallery shortcode(s)

Introduces get_content_images( &$content, $remove = false ) which will assemble [gallery]s and read <img>s, extract their srcs, optionally remove them from the content, and return them in a unique array.

Introduces get_the_images() which assembles a unique list of every image in the post's content retrieved via get_content_images() + all of the images attached to the post, displayed or not

Introduces get_post_images() which retrieves WP_Post objects for all images attached to the post

If more or different functionality is needed / wanted, let's iterate!

comment:24 toscho14 months ago

  • Cc info@… added

comment:26 lancewillett14 months ago

For adding support for new functionality in default themes, see:

  • Twenty Ten, gallery post format - #23617
  • Twenty Eleven: gallery post format - #22907
  • Twenty Thirteen: gallery post format - #23621

comment:27 chellycat14 months ago

  • Cc chellycat added

comment:28 follow-up: nacin13 months ago

  • Keywords needs-unit-tests added

I would suggest the following breakdown:

  • get_attached_images() — get images attached to the post. Simple wrapper of get_children(), as in the patch.
  • get_content_images() — fetch all images referenced in the content
  • get_post_galleries() — fetch all galleries referenced in the content

To bring it all together:

  • get_associated_post_images() — which should also return post thumbnails. More or less covers #16349.

To also cover #23593:

  • get_post_gallery() — first gallery referenced in the content
  • get_content_image() — first image referenced in the content

Let's avoid the get_tag_regex() abstraction for now, I am thinking.

Also, the content and gallery extraction functions need unit tests like whoa.

comment:29 wonderboymusic13 months ago

get_tag_regex() only exists because every patch like this was redoing the logic

markjaquith13 months ago

refresh

comment:30 in reply to: ↑ 28 nacin13 months ago

Replying to nacin:

get_post_galleries() — fetch all galleries referenced in the content

Specifically, an array of galleries, each an array of images. May need to be more robust if you want to know more about the gallery itself (like its settings), but the nature of how galleries are implemented make that fairly painful to implement anyway.

The primary use case for post formats is going to be "get me the first image of the first gallery". Indeed, this is the focus of #23593. (It has also been brought up previously in #20157, as gallery cover images.) I am thinking a bit farther here. The original title for this ticket is "grab the images of a gallery", and came out of 3.5 — see #23041, #22829.

comment:31 wonderboymusic13 months ago

  • Component changed from Media to Post Formats

wonderboymusic13 months ago

comment:32 wonderboymusic13 months ago

  • get_post_images() becomes get_attached_images(), returns WP_Post objects keyed by ID, wraps get_children()
  • get_attached_image_srcs(), calls the above and returns the result of passing post IDs to wp_get_attachment_url()
  • get_content_images() grabs <img> src attributes from the passed content blob
  • get_content_image() grabs first <img> src, calls above and returns first image, above can be limited to parse $limit # of items
  • get_content_galleries() grabs [gallery]s from passed content and returns structured data, which currently is only one node: src ,which contains a list of image URLs. In the future, more nodes can hold more data about the gallery
  • get_post_galleries_images() calls get_content_galleries() for passed post and returns a list of lists by calling wp_list_pluck( $data, 'src' ) on each parsed gallery above
  • get_post_gallery_images() calls get_content_galleries() for passed post's content and limits it to parsing one shortcode
  • get_associated_post_images() is not impl'd / saved for later

UNIT TESTS also attached

comment:33 wonderboymusic13 months ago

#23593 was marked as a duplicate.

comment:34 wonderboymusic13 months ago

Of note, none of those deal with the post format meta for gallery, I am going to suggest we get rid of it

comment:35 helen13 months ago

If it parses all good and reliable and stuff, that is fine by me.

comment:36 wonderboymusic13 months ago

once the functions exist in the WP, we can do another sweep to see if meta is necessary - also, if the UI vibes change to make meta more desirable, we can change the funcs

comment:37 DrewAPicture13 months ago

Reiterating from ticket:23572:8 and ticket:23282:49, it seems like there's a lot of duplicated code in these get_content_* functions. Had you considered a more generic approach such as maybe get_content_media( 'audio|video|link|url|etc' )? We should be careful about creating specifically-named getters because we'd lock ourselves into the pattern of having to create brand new functions for every new kind of supported media.

Also happy to contribute with patch(es) :).

wonderboymusic13 months ago

comment:38 wonderboymusic13 months ago

Updated the patch so it doesn't have funcs that are already committed. Gonna take another pass through this.

wonderboymusic13 months ago

comment:39 wonderboymusic13 months ago

Updated patch and docs, get_content_galleries() now additionally returns the parsed shortcode attrs with the list of image srcs. get_content_galleries() is the meat and potatoes for all of the other functions

comment:40 wonderboymusic13 months ago

Unit Tests updated

wonderboymusic13 months ago

comment:41 wonderboymusic13 months ago

Helen pointed at that we need to remove the [caption] for an image when we remove the <img>s in get_content_images() - updated patch does so, what a joy!

comment:42 markjaquith13 months ago

  • Owner set to markjaquith
  • Resolution set to fixed
  • Status changed from reopened to closed

In 23772:

Add functions to extract images from posts in various forms

props wonderboymusic. fixes #22960

comment:43 markjaquith13 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

What about filters for this? There may be other ways an image is associated that might want to become accessible via these new API functions.

comment:44 alexvorn213 months ago

  • Cc alexvornoffice@… added

comment:45 helen13 months ago

  • Keywords needs-patch has-unit-tests added; has-patch needs-unit-tests removed

Added some filters that affect the get_attached_images() function in [23776]. Looking at this, do we need filters for more than get_post_gallery|galleries? The ones that only get $content for context should probably be left alone.

wonderboymusic13 months ago

comment:46 wonderboymusic13 months ago

  • Keywords has-patch commit added; needs-patch removed

attachment:22960.7.diff​ adds filters to the get_content_* functions for images, which are used by all extraction functions for images

comment:47 markjaquith13 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 23821:

Add some filters to the image extraction-from-post bits so people with
existing postmeta-based flows can hook in and make magic happen.

props wonderboymusic. fixes #22960.

comment:48 nacin13 months ago

Cross-referencing with #23621 with the note that Twenty Thirteen actually doesn't use the first image from the gallery.

comment:49 wonderboymusic12 months ago

In 1260/tests:

Add initial Unit Tests for images, galleries. See #22960

comment:50 cramdesign10 months ago

  • Cc matt@… added
Note: See TracTickets for help on using tickets.