WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 13 months ago

#23801 closed defect (bug) (worksforme)

Audio Shortcode: MP3s Display above plain text.

Reported by: mfields Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.6
Component: Shortcodes Keywords:
Focuses: Cc:

Description

MP3's render above text and inline elements that have been entered immediately before.

To reproduce, insert the following content using the HTML editor:

Just some text up here ...
[audio mp3="http://wp-content/uploads/2013/03/mp3-hello.mp3"][/audio]

Something similar to the following should be rendered on the front-end:

<div style="width: 400px; height: 30px;" id="mep_1" class="mejs-container svg wp-audio-shortcode mejs-audio">...</div>
<p>Just some text up here ...<br>
</p>

I've tested similar content using both OGG and WAV file types. These files are not effected by the bug. Only MP3s seem to render above paragraphs.

This only seems to occur with the shortcode, I've tested a url on it's own line and it displays correctly:

Just some text up here ...
http://wp-content/uploads/2013/03/mp3-hello.mp3

Attachments (1)

Screen Shot 2014-05-12 at 8.03.56 PM.png (21.4 KB) - added by wonderboymusic 13 months ago.

Download all attachments as: .zip

Change History (13)

comment:1 follow-up: @mfields2 years ago

I did a bit more testing and it turns out that this only happens in Firefox. The following code seems to be the culprit here (line 1140 of wp-includes/js/mediaelement/mediaelement-and-player.js)

// check for placement inside a <p> tag (sometimes WYSIWYG editors do this)
node = htmlMediaElement.parentNode;
while (node !== null && node.tagName.toLowerCase() != 'body') {
	if (node.parentNode.tagName.toLowerCase() == 'p') {
		node.parentNode.parentNode.insertBefore(node, node.parentNode);
		break;
	}
	node = node.parentNode;
}

Is this safe to remove?

comment:2 @SergeyBiryukov2 years ago

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

Related: #23282

comment:3 @mfields2 years ago

I've also tested the video shortcode and this appears to happen with .flv files too. In this screenshot you will see two videos in the same post. The text "FLV" is rendered below the video (unexpected) while the text "MP4" is rendered above the video (expected). Here is the post content:

<strong>FLV</strong>
[video flv="http://wp-content/uploads/2013/03/small.flv"][/video]

<strong>MP4</strong>
[video mp4="http://wp-content/uploads/2013/03/small.mp4"][/video]

comment:4 @wonderboymusic2 years ago

I can't reproduce the MP3 thing... worked in 2011 and 2013 themes no probs. What browser?

Last edited 2 years ago by wonderboymusic (previous) (diff)

comment:5 @kovshenin2 years ago

  • Keywords needs-patch added

I reproduced this with Twenty Twelve, Twenty Thriteen and Firefox 20.0 and 21.0.

Version 0, edited 2 years ago by kovshenin (next)

comment:6 @kovshenin2 years ago

Here's the commit mfields was referring to. Doesn't seem safe to remove. https://github.com/johndyer/mediaelement/commit/9d2ce31b7ea525f8da747e39f7a0e11980e91289

Perhaps we can ensure there's extra \n around the shortcode callbacks so they're never placed inside a p tag? Thoughts?

comment:7 @nacin23 months ago

Verdict here? Do we care about this for now? Technically [audio] and [video] are valid and fully functional as self-closing shortcodes. Do we insert them as self-closing via the UI? Punt?

comment:8 @markjaquith23 months ago

  • Milestone changed from 3.6 to Future Release

I don't think I care about this right now. Call it a known issue and recommend people put audio players on their own line.

comment:9 in reply to: ↑ 1 @azaozz23 months ago

Replying to mfields:

Is this safe to remove?

Don't think so. It breaks the <div> out of the <p>. Not sure why the <div> is inserted before the <p> though. Seems it has to be inserted after.

comment:10 follow-up: @rmarks22 months ago

I'm unable to reproduce this on 3.7-alpha.

comment:11 in reply to: ↑ 10 @dd3222 months ago

Confirmed this is still an issue in Firefox 21 & 23 and Opera (Presto, and, Chromium). Chrome 28 Unaffected.

It certainly doesn't look like the media player is supposed to be inserted inside a paragraph though.

comment:12 @wonderboymusic13 months ago

  • Keywords needs-patch removed
  • Milestone Future Release deleted
  • Resolution set to worksforme
  • Status changed from new to closed

Added screenshot, this appears to no longer be an issue.

My post's content is:

Before [audio mp3="http://wordpress-core-develop/wp-content/uploads/2014/05/small.mp3"][/audio] After

Note: See TracTickets for help on using tickets.