WordPress.org

Make WordPress Core

Opened 4 months ago

Last modified 10 days ago

#26675 new enhancement

Rewrite of get_media_embedded_in_content() function to produce expected results.

Reported by: kopepasah Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.6
Component: Media Keywords: has-patch needs-testing
Focuses: Cc:

Description

The get_media_embedded_in_content() function does not output the media in the array in the order it appears in the content (as it should). Rather, it loops through each tag and gets the media in the order of tag (if get_tag_regex() worked correctly, see #26674).

The attached patch adds a third parameter 'sortby' to declare how the results should sort. By default it sorts by 'type' and the remains as it is now. If a user declares the 'sortby' parameter as 'source', the results would appear in the order they are found in the content (which is what I expected).

Also preg_match() was changed to preg_match_all() to find all the media of a type (instead of just one).

Lastly, the regex used in the 'source' sort by is a modification of the get_tag_regex() to try subpatterns in alternation. An optional fix would be to add the named group in the get_tag_regex() function, but that is another patch.

Side note: I added 'img' to the option of allowed media types (because it is media). If anyone has a reason this should not be in there, I would like to know why. I did not find any complications while testing.

Attachments (3)

26675.patch (2.0 KB) - added by kopepasah 4 months ago.
media.php_unit-test.patch (3.5 KB) - added by kopepasah 4 months ago.
26675-2014.02.19.patch (1.1 KB) - added by Kopepasah 2 months ago.

Download all attachments as: .zip

Change History (10)

kopepasah4 months ago

comment:1 kopepasah4 months ago

  • Cc justin@… added
  • Keywords has-patch needs-testing added

comment:2 ircbot3 months ago

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

comment:3 nacin3 months ago

  • Milestone changed from Awaiting Review to Future Release

Hey, Justin. I think we could get away with sorting these by how they appear in the content always, without a parameter. Would be nice to still use get_tag_regex() but I get how the sorting makes it tough.

"media" was designed to refer to "audio" or "video" (or some other form of interaction).

We could make it additionally allow for "img" (probably not as a default). The problem with images is they can be inserted all sorts of ways. Gallery shortcodes, captioned images, image HTML, etc., etc. We actually had some image extraction functions as a counterpart to this function, but they weren't really good enough to ship in core, because of all of the edge cases. Sticking to these elements makes us limit the edge cases pretty well, and we ditched the other functions later in the 3.6 cycle.

comment:4 Kopepasah2 months ago

Nacin, great suggestions. I will submit another patch based on those.

For the images, should I add an additional parameter or a filter. I personally vote filter, as this will allow users to not only add additional media types to search for (e.g. img), but also search for a specific media type (e.g. iframe) or multiple types (e.g. audio & video). With that being said, it will also allow for searching for any HTML element if a users sets the filter that way, but that could be edge case.

What do you think?

comment:5 Kopepasah2 months ago

Nacin,

I've attached 26675-2014.01.19.patch to address your suggestions.

I removed the $sortby parameter and changed the sorting to how the media was located in the content. Should we add an additional parameter to allow for sorting of by media item and not by found?

I also added a filter (get_media_embedded_in_content_allowed) to customize the allowed embedded media types. I think this is a better alternative than adding an additional parameter.

Feedback is appreciated.

comment:6 ircbot2 months ago

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

comment:7 johnbillion10 days ago

  • Version changed from trunk to 3.6
Note: See TracTickets for help on using tickets.