Add shortcodes for inline HTML5 audio / video

[wonderboymusic]

WordPress supports the 'audio' and 'video' post formats, however WordPress has no native ability to play audio or video.

I have been using and following MedaElement.js for a while. It is, in my opinion, the most cross-browser compatible and extensible HTML5 solution out there. ​https://github.com/johndyer/mediaelement

in a nutshell:

A complete HTML/CSS audio/video player built on top MediaElement.js and jQuery. Many great HTML5 players have a completely separate Flash UI in fallback mode, but MediaElementPlayer.js uses the same HTML/CSS for all players.

Compatibility?

Flash Fallbackforward

Instead of offering an HTML5 player to modern browsers and a totally separate Flash player to older browsers, MediaElement.js upgrades them with custom Flash and Silverlight plugins that mimic the HTML5 MediaElement API.

Magic happens, and then IE6-8 supports <video> and <audio>, Firefox and Opera support h.264, and Safari and IE9 support WebM (*if Adobe makes good on promises to support VP8 in Flash).

My patch makes 2 new shortcodes [audio] and [video], both will pull the first attached mp3 (for audio) or mp4 (for video) attached to the post. If the src attribute is passed, the shortcode doesn't check for an attachment.

The patch is proof of concept and could (maybe?) use some more features. I will attach a sample mp3 and mp4 to the ticket to use for testing.

UPDATE: mp3 and mp4 files are too big to upload to Trac*

[wonderboymusic]

No clue how to properly diff with images, so unzip and drop mediaelement.zip​ in your wp-includes/js folder

[wonderboymusic]

I uploaded a screenshot with the WordPress school colors

[wonderboymusic]

23282.2.diff​ supports fallbacks for video of type video/webm (WebM, V8 codec) and video/ogg (Ogg Theora). Also allows flash fallback to be turned on or off. Also supports the poster attribute on the video and the flash fallback when enabled (it is by default). Example shortcode with all of the current features:

[video src="1.mp4" ogv="2.ogv" webm="3.webm" poster="/4.jpg" flash="false"/]

Why is this necessary? Specifying webm AND H.264 give you the most native HTML5 support: ​http://mediaelementjs.com/#devices

Why a shortcode for video?

• Easiest way the slap a video anywhere
• For right now easiest way to specify multiple versions of a video, post_parent-ing and relationships can be a mess when you attach multiple videos to the same post, since currently there is no way to associate attachments with each other

I have implemented MediaElement a bunch of times, and each time I used the first plugin I ever wrote: ​Shuffle. Shuffle lets you attach attachments to other attachments, since all you are really doing is setting post_parent on the "child" attachments. By doing this, you can assign images and other codecs to a video or mp3. So your Post can have a Video that has a Poster Image and an Ogg Theora version and a WebM version. This way, you can attach 12 MP4s to your post if you so choose, and you can cleanly get their counterparts with having to dig through all media attached to the post ID.

Not sure what to propose happens to core for stuff like this right now, so sticking with the shortcode. Patch will evolve with my thinking.

[wonderboymusic]

I have uploaded a small video file in multiple formats. mp4 should encouraged for the src attribute, but it turns out MediaElement works so well at being compatible, fallbacks almost aren't necessary. To get the most native HTML5 playback, yes, you need fallbacks, but you can now put a .wmv file as a src and it will play in Chrome.

Will work on audio next

[oyvinds]

I find this "shortcodes" for inline HTML5 audio / video idea stupid and a total fail to begin with.

Want to insert a image into WordPress? Then click the media button and upload the image and insert it. You do not use shortcodes.

Ideally: If you want to upload any audio file to WordPress then you do the same, you click the media button and upload the audio file and click insert. The same should apply to video files. Now, only a few types are supported and users should be required to make files in the right formats - just like you have to upload a .png or .jpg image, you can't just upload some .bmp file and think that's fine.

My point is: Every modern CMS system, social network and the likes today allow you to upload a video file and have it inserted in your post easy as pie. It's just inserted. You don't use "shortcodes". shortcodes are a dump very stupid wordpress-only idea which should be put to death asap.

Just fix the very broken useless media uploader for wordpress and make it insert audio and video with proper html5 files, focus on that instead of wasting time on "shortcodes". Look at modern (WP isn't, it's stuck years behind) CMS systems or just look at how you post videos on Facebook.

Make videos be upload, click insert in post and you're done. make that a goal.

[helen]

Replying to oyvinds:

I find this "shortcodes" for inline HTML5 audio / video idea stupid and a total fail to begin with.

Please be civil.

Inserting an audio file now just inserts a hyperlink to the file unless you've installed a plugin to further handle it. The real work behind this ticket is a player with fallbacks that handles both audio and video in core itself. The shortcode would be what goes into the post when you insert from the media library that then becomes the player on display. The library doesn't just insert the URL of an image either - an image tag gets inserted, so in that same way, we need to insert something for audio and video that "just works" and doesn't have issues like disallowed tags for certain roles that then get stripped after the media library has let you insert it.

[tar.gz]

Replying to oyvinds: I believe what you want to say is "let me insert from the uploader, I don't want to type in shortcodes". I guess we all agree on that.

But did you notice what happens when you insert a picture gallery from the media uploader? It generates a shortcode for you! But you won'tsee it if you use the visual editor. That's the type of functionality that wonderboymusic's proposal is aiming for.

Another thing, to wonderboymusic: don't forget about captions! The html5 video element, and fully-featured players such as MediaElement, also support caption tracks (coming as .srt files). That's a great functionality, and something that "every modern CMS system, social network and the likes today" doesn't offer. For me that would be a compelling reason for self-hosting my videos on WP.

[wonderboymusic]

1) this is one piece of many, of course this is going to abstracted in the media library so no one actually *has* to paste shortcodes
2) oyvinds - who are you? welcome to Trac, I look forward to reviewing the alternate patch that you uploaded that's way less than stupid than mine ... hmm, that's weird, it must have been deleted

[wonderboymusic]

attachment:23282.4.diff​​ updates the patch - sends the audio and video shortcodes to the editor on Insert Into Post. The code in the patch is the bare minimum to make that happen. The media JS is 1000s of lines long. I'll refine my approach as I drink it in.

[wonderboymusic]

23282.5.diff​ combines with the patch from #23572

[beaulebens]

I tested out 23282.6.diff, and here are some notes/questions/etc:

• Really like the Insert into Post action from the Media Manager when you upload audio/video; hotness
• I tested output in /Twenty (Ten|Eleven|Twelve|Thirteen)/ and it worked in all of them OOTB
• get_post_audio() doesn't return anything if it doesn't find audio children. Probably should return either false or an empty array
• get_content_audio() also doesn't return anything if there's
• get_content_audio() and get_content_video() probably need to use the /i modifier on the regex to catch upper case html. Might also want to set the $count param in str_replace for a regexed out tag just to be safe and only ever remove one instance
• Extra space character on line 1753 of wp-includes/media.php
• For the fallbacks in post_formats_compat() where a link is inserted, it's probably worth adding a standardized class to them (class="wp-post-format-link-$format" or something?) Applies to #23347 also.
• Should post_formats_compat() now have defaults for audio/video of their shortcodes?
• MediaElements is licensed MIT, so it's GPL-compat. It also has a long dev history (back to '09) and seems actively maintained, so it looks like a nice choice for core.
• We might want to make the default color scheme for the media players a bit more "generic" perhaps.
• I haven't tried it, but I assume there are going to be conflicts with plugins that are using the audio/video shortcodes -- not sure what the best way of handling them is.

I think it's worth getting this into core to try it out along with the other post formats work, since without it, the PF stuff is kind of half-hearted.

[wonderboymusic]

attachment:23282.7.diff​​ is an updated patch with the recommendations from my GSOC counselor Beau. I'll give someone $5 to commit this.

[wonderboymusic]

reminder, you also have to unzip and drop attachment:mediaelement.zip​ into wp-includes/js

[Viper007Bond]

While shortcodes are nice, URLs on their own line are better and easier to use.

Adding a simple call to wp_embed_register_handler() with a large priority number (so it runs late) that catches *.mp3 and such and turns it into an embed would be great. It'd be a nice complement to the shortcodes.

[wonderboymusic]

need shortcodes if you want to specify many different sources for the same vid - you need to specify mp4 + webm + ogv to get the most HTML5 native playback cross-browser

[Viper007Bond]

I'm not saying ditch shortcodes -- they certainly have their uses such as advanced parameters, but if I want to embed an audio file for example, I should be able to just paste it's .mp3 URL. Both methods should be supported.

wp_embed_register_handler() also makes the core [embed] shortcode be able to handle stuff passed to it, if someone should choose to use that ("I want to embed something, so I'm using the embed shortcode!").

[wonderboymusic]

Alex, you are correct :) I have now added embed handlers for audio and video in attachment:23282.8.diff​ - now, if you just leave a *.(mp3|ogg|wma) or *.(mp4|webm|ogv|wma|flv) URL on a line by itself, it will magically appear as a player. #boom

[hchouhan]

Replying to wonderboymusic:

Alex, you are correct :) I have now added embed handlers for audio and video in attachment:23282.8.diff​ - now, if you just leave a *.(mp3|ogg|wma) or *.(mp4|webm|ogv|wma|flv) URL on a line by itself, it will magically appear as a player. #boom

You said now directly adding the link works. However like some themes, would it not be easy for anyone to have different fields where they could enter the URL of the file based on its format?
Shortcode is great, but I find many people getting confused.

[wonderboymusic]

what do you mean by "different fields" - in the post edit UI? One for each video type? That could be possible. This ticket is just to introduce core support for audio / video and iterate from there. It also combines the patch from #23572 since they are tightly coupled in the new post formats compat API.

The URL on its own line thing is a bit of automagic, but it can also be turned off by a themer. I can imagine more hooks and filters being added to this code for more user control as well.

[Viper007Bond]

Replying to wonderboymusic:

Alex, you are correct :) I have now added embed handlers for audio and video in attachment:23282.8.diff​ - now, if you just leave a *.(mp3|ogg|wma) or *.(mp4|webm|ogv|wma|flv) URL on a line by itself, it will magically appear as a player. #boom

Nice!

I would pass through the width/height values for the video shortcode though so that this will work:

[embed width="123" height="456"]video url[/embed]

You can check against the $rawattr array instead of the $attr array to see if the user specifically passed values rather than if the embed API generated the values instead (which it will do if the user doesn't supply them).

[hchouhan]

Hello,
I am new to WordPress trac. Can someone please let me know if what suggested in this ticket is final and going to be implemented in the upcoming WordPress version?

[sabreuse]

Replying to hchouhan:

Hello,
I am new to WordPress trac. Can someone please let me know if what suggested in this ticket is final and going to be implemented in the upcoming WordPress version?

The functionality that's described here is planned for 3.6 (we only assign a milestone to tickets when they're part of the roadmap for a specific release), but the details of what gets implemented and how are worked out in the course of multiple iterations. Don't assume that a specific version of the functionality will be the final one until after we reach beta.

(It also sometimes happens that a feature is planned for a given milestone and later dropped because we don't have the time or resources to make it work before release.)

[wonderboymusic]

Refreshed patch against trunk - added width and height to embed handler as per Alex when specified. Added functions to return the default extensions that are supported out of the box by mediaelement: wp_get_video_extensions() and wp_get_audio_extensions()

[wonderboymusic]

goes in wp-includes/js

[wonderboymusic]

attachment:mediaelement.zip​ is updated. Adds a few lines to wp-mediaelement.js to add mime-type aliases for .wmv and .wma files which trigger the Silverlight plugin when applicable.

attachment:23282.10.diff​​ adds some changes to PHP and doesn't dip into Post Formats heavily, remains generic as other patches will depend on these functions.

Adds shortcode_exists( $tag ) and has_shortcode( $content, $tag ) - we were / will be repeating ourselves in a bunch of places without them.

If anyone wants to do easy testing:

• clone me and checkout the post-formats/23282 branch: ​https://github.com/staylor/WordPress
• contains an MU plugin that will create a bunch of posts for you: ​https://github.com/staylor/WordPress/tree/post-formats/23282/wp-content/mu-plugins . The plugin runs on every page load but only adds posts if they aren't there. If you set your homepage to show 40 posts, you can test every file type on one page. The MU plugin also contains all of the media files in dir "media/*". If you want to test the shortcodes with attached media, drag all of the files in "media" into the Media Library uploader and attach them to the posts with the same name as their extension i.e. "small.mp3" attaches to "MP3"
• 3 posts are created for every file type: Shortcode, Link, Link with commentary. Example for MP3 type, posts titled: MP3, MP3 Link, MP3 Link with commentary
  • the shortcode doesn't work until you attach the media item to the post
  • the link is scooped up by the embed handler, same for link with commentary, which is why they work without a shortcode

I would appreciate any and all code feedback so we can make changes to get this commit-ready. Once this is committed, we can do a much more thorough compatibility check for audio / video post format scenarios.

xoxo

[wonderboymusic]

Latest patch ditches the extraction functions - those can be handled over here: #23572

[wonderboymusic]

has_shortcode(), shortcode_exists(), and get_tag_regex() are not cruff, they are needed by future patches and other post format stuff, some are used in this patch - without them, we will repeating ourselves all over the place. attachment:mediaelement.zip​​ still needs to be unloaded in wp-includes/js

[anointed]

Just want to put in a pitch for kaltura vs. mediaelements before everything is finalized just in case you are not familiar with it. I believe you will find the functionality far superior and the team behind it second to no one other than the WP core team of couse.

​https://github.com/kaltura/mwEmbed

If this isn't selected, then please, at the very least allow us to change from mediaelements very easily as I will never go back to that system after using mwembed.

[helen]

Replying to anointed:

Just want to put in a pitch for kaltura vs. mediaelements

Although this is not my strong suit by any means, the license does not appear to be compatible.

[nacin]

Replying to anointed:

Just want to put in a pitch for kaltura vs. mediaelements before everything is finalized just in case you are not familiar with it. I believe you will find the functionality far superior and the team behind it second to no one other than the WP core team of couse.

​https://github.com/kaltura/mwEmbed

As helen points out, it has a wholly incompatible license.

If this isn't selected, then please, at the very least allow us to change from mediaelements very easily as I will never go back to that system after using mwembed.

Certainly — this implementation should be pluggable to allow for this.

[wonderboymusic]

Updated patch to make everything filterable / replaceable. Added a function to insert fallback content into the <audio> and <video> tags when JS is disabled (defaults to linked URL - one again, filterable)

Here is the laundry list of what is in the patch:

• allows Insert Into Post to work for Audio / Video
• mediaelement.zip goes in wp-includes/js
• Introduce get_tag_regex(), utility to return regex for HTML tag content, needed by other patches
• Introduce shortcode_exists()- check whether short code is presently registered, needed by other patches
• Introduce has_shortcode( $content, $tag ), checks content for shortcodes, needed by other patches
• Introduce wp_get_audio_extensions(), filterable
• Introduce wp_audio_shortcode(), filterable
• Introduce wp_audio_embed(), passes URL to shortcode if it hasn't been unregistered
• Introduce get_post_audio(), wraps get_children()
• Introduce wp_get_video_extensions(), filterable
• Introduce wp_video_shortcode(), filterable
• Introduce wp_video_embed(), passes URL to shortcode if it hasn't been unregistered
• Introduce get_post_video(), wraps get_children()
• Introduce wp_mediaelement_fallback

the No-JS <object> HTML code is not jiving well with me in a local environment, gonna reach out upstream about it, but I don't think that is a blocker for this patch.

[DrewAPicture]

As I mentioned in ticket:23572:8, it seems like there's a lot of duplication in the audio/video function pairs. Had you considered moving to a more general approach of say get_post_media( 'audio|video' ) to reduce the necessity of these very specifically-named functions?

It seems like we'd be locking ourselves in to having to create additional media-named functions in the future. Going with the more generic naming convention you can always extend for new flags.

Also happy to work up patches in this vein.

[wonderboymusic]

well, seems like the functions would still exist, would just wrap the generic function:

function get_post_audio( $args ) {
    return get_post_media( 'audio', $args );
}

Because at that point, images are media, and they could be converted as well. Might be better to wait until the patches land, and then do a new ticket which can applied against all of them - because right now, none of them are tightly-coupled, they can be applied separately. So, you would have to put the optimized code in every ticket.

I still think image/audio/video need their own APIs, but I get your gist.

[helen]

Per ​dev chat, we should go ahead and get this soaking in trunk for more eyes and testing, especially on the front with a wide variety of theme situations.

[markjaquith]

In 23729:

Introduce [audio] and [video] shortcodes, and use MediaElement.js to play them.

props wonderboymusic. see #23282.

[markjaquith]

Action item: add MEjs to our sources list and repo.

[DrewAPicture]

move into wp-includes/js

[DrewAPicture]

It appears the /mediaelement/ directory was mistakenly unzipped into wp-includes/ instead of wp-includes/js in [23729]. So it currently doesn't work in most browsers.

I tried svn mv to a diff but 23282.13.diff​ seems to be fubar. Is this one of those situations where you have to manually unzip the archive then commit without a patch?

[dd32]

In 23732:

Move the Mediaelement.js include into the wp-includes/js folder, rather than wp-includes directly. Fixes the mediaplayer in browsers which don't support <audio> directly.
Props DrewAPicture See #23282.

[Otto42]

Not sure if this needs a new ticket or not, but, bug report:

When using the Add Media window to insert an MP3 into the post, the Attachment Details sidebar seems to default to Link To: None for me. Clicking Insert into Post at this point results in this being inserted into the post:

[audio mp3=""][/audio]

Changing Link To to "Media File" gives the correct shortcode.

Also of note, changing Link To to "Attachment Page" puts the wrong link in the audio shortcode, with the mp3 setting being the link to the attachment page.

This may be a separate ticket due to it being primarily related to the media window doing the wrong thing with the link here, but I felt it was worth reporting.

[helen]

Maybe there needs to be a separate option for "Media embed" if applicable?

[obenland]

Pass shortcode name to shortcode_atts() to provided for context to enable filtering

[obenland]

Testing the audio shortcode for 2013 I noticed that:

• my .m4a file was passed to the shortcode as mp3="path/to/file.m4a", thus failing the filetype check in the audio shortcode.
• I'd always get the following JS warning: "Specified "type" attribute of "audio/mpeg" is not supported. Load of media resource ​http://wpthemetestdata.files.wordpress.com/2008/06/originaldixielandjazzbandwithalbernard-stlouisblues.mp3 failed." and "All candidate resources failed to load. Media load paused."
• for some reason the container div (.wp-audio-shortcode) has it's height set to 34px (width: 454px) in 2013's audio post format (it should be 30px). It has the correct height and in standard post formats (width: 400px). Where can I find the code that deals with that?

It's a nit pick, but based on the singular in the function name, I didn't expect more than one result from get_post_audio|video().

[wonderboymusic]

Related #23831 for this

Replying to Otto42:

Not sure if this needs a new ticket or not, but, bug report:

When using the Add Media window to insert an MP3 into the post, the Attachment Details sidebar seems to default to Link To: None for me. Clicking Insert into Post at this point results in this being inserted into the post:

[audio mp3=""][/audio]

Changing Link To to "Media File" gives the correct shortcode.

Also of note, changing Link To to "Attachment Page" puts the wrong link in the audio shortcode, with the mp3 setting being the link to the attachment page.

This may be a separate ticket due to it being primarily related to the media window doing the wrong thing with the link here, but I felt it was worth reporting.

[wonderboymusic]

Replying to obenland:

• my .m4a file was passed to the shortcode as mp3="path/to/file.m4a", thus failing the filetype check in the audio shortcode.

Did this happen using Insert Into Post? that shouldn't happen: https://core.trac.wordpress.org/browser/trunk/wp-includes/js/media-editor.js#L99
I have opened up a ticket for Insert Into Post issues: #23831

• I'd always get the following JS warning: "Specified "type" attribute of "audio/mpeg" is not supported. Load of media resource ​http://wpthemetestdata.files.wordpress.com/2008/06/originaldixielandjazzbandwithalbernard-stlouisblues.mp3 failed." and "All candidate resources failed to load. Media load paused."

That MP3 worked for me...

• for some reason the container div (.wp-audio-shortcode) has it's height set to 34px (width: 454px) in 2013's audio post format (it should be 30px). It has the correct height and in standard post formats (width: 400px). Where can I find the code that deals with that?

Not sure why it is getting set to 34px, but:
https://core.trac.wordpress.org/browser/trunk/wp-includes/js/mediaelement/mediaelementplayer.css#L112
https://core.trac.wordpress.org/browser/trunk/wp-includes/js/mediaelement/mediaelement-and-player.js#L1841

It's a nit pick, but based on the singular in the function name, I didn't expect more than one result from get_post_audio|video().

yeah... get_post_videos() would make sense there but not get_post_audios()

[obenland]

Replying to wonderboymusic:

Replying to obenland:

• I'd always get the following JS warning: "Specified "type" attribute of "audio/mpeg" is not supported. Load of media resource ​http://wpthemetestdata.files.wordpress.com/2008/06/originaldixielandjazzbandwithalbernard-stlouisblues.mp3 failed." and "All candidate resources failed to load. Media load paused."

That MP3 worked for me...

It did work for me too, the warning would still be displayed though :)

yeah... get_post_videos() would make sense there but not get_post_audios()

Having one plural and the other one indistinguishable would be even more confusing I suppose. Maybe get_post_media( 'audio', $args ); without wrapper functions is not the worst of ideas?

[dimadin]

MedaElement.js has very poor i18n support that isn't compatible with one we use in WordPress since it relies on browser's language to determine locale, and strings are hardcoded in it (see ​pull that introduced this).

I looked how to improve this and have it backward compatible and attached patch contains my proposal. Note that this shouldn't go straight to WordPress since it contains changes to MedaElement.js too. I wanted to hear thoughts from others before submitting pull to MedaElement.js. When they add compatible support, we can patch WordPress.

[obenland]

Just discovered, that when there is an audio and a video shortcode on one page, you cant play both without stoping the other. Is this intended?

[wonderboymusic]

we need to add pauseOtherPlayers: true to the init method

[obenland]

I'm sorry, I missed an apostrophe in my previous comment. Currently, when you click play on the second player, it stops the first player. So I guess it pauses other players by default.

But that seems counterintuitive to me. What do you think?

[wonderboymusic]

yeah, we should set it to false

[obenland]

While testing I had an audio post and a video post on index. While the audio was playing, I clicked to play the video and was surprised that the audio would stop. It was just something I did not expect because to me the two did not seem logically and semantically related. If I had two youtube videos underneath each other, one wouldn't stop playing if I'd start playing the other one.

As you can see from my previous comments, it is not that I feel very strongly about either behavior, it's just that it caught me by surprise and I was wondering if others feel the same way or not.

[helen]

Having two separate players going at once seems counterintuitive to me, as in it's rarely the desired result :) But, not sure about which is more appropriate for default behavior. I'd lean toward leaving it just because it seems like better actual end behavior.

[helen]

Replying to dimadin:

Do you mind opening a separate ticket for i18n issues, since it's dependent on upstream changes? Will also be easier to point translators to it for review, etc.

[kylegilman]

wonderboymusic this is great! I have a plugin with some overlapping functionality (​http://wordpress.org/extend/plugins/video-embed-thumbnail-generator/) and now that I know about this I'm going to work on adding the option of using the built-in WordPress video shortcode instead of my own. I find that web video is unbelievably frustrating because of the proliferation of formats. oyvinds should note that those modern social networks take videos that users upload and transcode them all to the same formats, which is clearly not an option for the average shared hosting package.

I'd like to help out, although I don't really know how to contribute code. The first thing I noticed is some trouble with mime types. The shortcode takes the file's mime type at face value, which means that .m4v and .mov files don't play. M4V is primarily created by Apple's Compressor and is almost always an H.264/AAC file that will work if the source "type" tag is set to video/mp4 instead of video/m4v. MOV could be any of dozens of codecs, and there's no way to know what codec it is, so I don't know if you want to support it, but again, if it does happen to be a properly encoded H.264/AAC file then setting the source "type" tag to video/mp4 will make it work.

[wonderboymusic]

yeah, we could definitely take another look at the mime-type checks - the best way to help is to provide use cases and testing like you have above, and supply solutions when you have them. I will look at the internals of MediaElement and extend the mime-type checklist if necessary, I think I just conformed to what MediaElement expects instead of coercing the mime-type based on what will work. Thanks for the feedback, and add any other use cases if you notice any other issues

[nacin]

In 23921:

Third parameter for shortcode_atts() for audio and video shortcodes. props obenland, see #23282.

[SergeyBiryukov]

Replying to dimadin:

I wanted to hear thoughts from others before submitting pull to MedaElement.js. When they add compatible support, we can patch WordPress.

23282.i18n.patch​ looks good to me and works in my testing.

Please open a new ticket for MediaElement i18n, as helen suggested in comment:74.

[wonderboymusic]

New tickets, por favor

[SergeyBiryukov]

Follow-up for MediaElement.js i18n: #24183

[duck_]

In 24334:

Properly escape source parameter in audio/video shortcodes. See #23282.

[amereservant]

I guess I'm curious why the shortcode_exists() function was added since this is so simple to check this by checking the global $shortcode_tags. This function seems to just add extra bloat to the WordPress code.

The other curiosity is in regards to the performance impact has_shortcode() will have ... there are at least a few tutorials on how to make this function, but making it a part of the core code sort of encourages the use of such a function and can hurt the overall performance of WordPress.

Would a filter hook in the do_shortcode_tag() function be a better option performance wise so that as the shortcodes are being executed, a hooked function can alter the shortcode tag before it is rendered? Not sure why this function was introduced, but that's just a thought...

[Otto42]

Replying to amereservant:

I guess I'm curious why the shortcode_exists() function was added since this is so simple to check this by checking the global $shortcode_tags. This function seems to just add extra bloat to the WordPress code.

The shortcode_exists function is the correct way to check for this sort of thing, because checking the global $shortcode_tags directly in every place where it's used is not clear. Code should be self-explanatory and easy to maintain, not optimized beyond the point of sanity. The overhead of an additional function call is minimal, and the benefit of clarity in understanding what the code is doing is worth the cost.

The other curiosity is in regards to the performance impact has_shortcode() will have ... there are at least a few tutorials on how to make this function, but making it a part of the core code sort of encourages the use of such a function and can hurt the overall performance of WordPress.

You seem to be suffering from the horrible curse of ​Premature Optimization.

If the function can be made faster, or clearer, or simpler, then it can be fixed later, or even now if you have a patch. But initially, the most important thing of any piece of code is that it works, and is clear, and makes sense. Speedups can be introduced later, if and when the code that is causing the slowdowns can be identified. You must Profile code before Optimizing it.

Not saying that there's no room for performance improvements here, but shortcode_exists() is not an example of bloat. And while there may be better ways to implement has_shortcode(), it's not the same thing to find out if a shortcode exists immediately prior to executing it vs. before processing the content. Now, has_shortcode() may no longer be needed since post-formats is getting ripped out of core for 3.6 and I don't think it was used elsewhere. If it's going to become unused, it might be worth removing from 3.6 as well.

[amereservant]

Replying to Otto42:

The shortcode_exists function is the correct way to check for this sort of thing, because checking the global $shortcode_tags directly in every place where it's used is not clear. Code should be self-explanatory and easy to maintain, not optimized beyond the point of sanity. The overhead of an additional function call is minimal, and the benefit of clarity in understanding what the code is doing is worth the cost.

Fair enough ... I was just pondering it, not that I know better by any means. I spend a lot of time coding plugins, etc. and always trying to use best practices etc. So understanding when to propose new functions helps in knowing when to do so in my contributions as well.

Not saying that there's no room for performance improvements here, but shortcode_exists() is not an example of bloat. And while there may be better ways to implement has_shortcode(), it's not the same thing to find out if a shortcode exists immediately prior to executing it vs. before processing the content. Now, has_shortcode() may no longer be needed since post-formats is getting ripped out of core for 3.6 and I don't think it was used elsewhere. If it's going to become unused, it might be worth removing from 3.6 as well.

Well the funny thing is, has_shortcode() is a new function as of 3.6.

[Otto42]

Replying to amereservant:

Well the funny thing is, has_shortcode() is a new function as of 3.6.

Yes, I know. It was needed for the post-formats stuff, which ​isn't going into 3.6 anymore. So it might be able to be removed.

[nacin]

As part of discussions for removing post formats (Otto42 linked to the decisions we made), we realized that both has_shortcode() and shortcode_exists() were simple and solid improvements to the existing shortcode API.

It's not entirely uncommon to need to check if a shortcode is in post content, which is currently done using strpos( $content, '[shortcode' ). That's lame and there should ideally be an API for it.

has_shortcode() is still used in core, in some media API functions that we're keeping as, again, good enhancements to the existing API (even though post formats UI is removed).

[wonderboymusic]

5 different people wrote functions that looked for a shortcode in content, and it became clear that there would be more, so I added the function to unify the method. That way, if anyone wants to optimize or alter the preg_match, it only has to happen in one place.