WordPress.org

Make WordPress Core

Opened 13 months ago

Last modified 9 months ago

#23801 new defect (bug)

Audio Shortcode: MP3s Display above plain text.

Reported by: mfields Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.6
Component: Shortcodes Keywords: needs-patch
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

Change History (11)

comment:1 follow-up: mfields13 months 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 SergeyBiryukov13 months ago

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

Related: #23282

comment:3 mfields13 months 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 wonderboymusic13 months ago

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

Last edited 13 months ago by wonderboymusic (previous) (diff)

comment:5 kovshenin11 months ago

  • Keywords needs-patch added

I reproduced this with Twenty Twelve, Twenty Thriteen and Firefox 20.0 and 21.0. There really needs to be no space between the content and the shortcode. Best if they're on the same line:

Before [audio mp3="http://wp-content/uploads/2013/03/mp3-hello.mp3"][/audio] After

You'll see the audio player, and then "Before After" in FF.

Last edited 11 months ago by kovshenin (previous) (diff)

comment:6 kovshenin11 months 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 nacin10 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 markjaquith9 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 azaozz9 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: rmarks9 months ago

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

comment:11 in reply to: ↑ 10 dd329 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.

Note: See TracTickets for help on using tickets.