I'm using the media module with the markdown module and am encountering a situation where the media module is interfering with the proper processing of my markdown syntax.

Specifically, if I start a textarea input with a pound at the beginning of the first line (which, in markdown, should enclose the rest of the line in an h1 html tag), I find that it is not being processed correctly due an application of the media filter before the markdown filter.

includes/media.filter.inc defines function media_filter, which processes all double-bracketed (e.g., "[[...]]") into html image references.

Before the call to preg_replace() on the input, the code prepends and appends the input with a space:

<?php
>
/**
* Filter callback for media markup filter.
*
* @TODO check for security probably pass text through filter_xss
* @return unknown_type
*/
function media_filter($text) {
 
$text = ' ' . $text . ' ';
 
$text = preg_replace_callback("/\[\[.*?\]\]/s", 'media_token_to_markup', $text);
  return
$text;
}
?>

I looked through the git history for that line of code and couldn't find the reason for doing this. On my local environment, I did a manual test when that line is removed and got the expected behavior.

If the line is not doing anything, I'd like to suggest it's removal.

Files: 
CommentFileSizeAuthor
#9 media-filter-remove-extra-spaces-1828558-9.patch489 bytesazinck
PASSED: [[SimpleTest]]: [MySQL] 56 pass(es).
[ View ]
#1 media-filter-remove-extra-spaces-1828558-1.patch424 bytesankur
PASSED: [[SimpleTest]]: [MySQL] 5 pass(es).
[ View ]

Comments

StatusFileSize
new424 bytes
PASSED: [[SimpleTest]]: [MySQL] 5 pass(es).
[ View ]

Here's a patch that removes that line.

Status:Active» Needs review

i have no clue why those spaces are needed there:/ really
maybe Dave does or someone of the older maintainers

I can't see a good reason why adding white-space padding around the rendered markup is necessary. Browsers will still render an image tag even if there is text next to the tag.

// Still Works
<p>content text<img src="foo/bar" />more content.</p>

Sure it doesn't look good. But, that's not for the code to solve.

Also, the way it is now you would get even more white-space padding around the markup if it were to be saved back into the system, and rendered again.

Status:Needs review» Reviewed & tested by the community

I haven't discovered any adverse effects from the patch in #1. RTBC +1

Works great for me, RTBC

Assigned:Unassigned» Dave Reid

i think i have seen this before somewhere when working with regex..but cant remember when or where..anyway, assigning to dave, he might have a clue

StatusFileSize
new489 bytes
PASSED: [[SimpleTest]]: [MySQL] 56 pass(es).
[ View ]

Re-rolled against latest dev

I would like to commit this, but I recall what rootatwc said, about regular expressions. I believe that the code was originally copied from another place within Drupal, but I was not able to find it with a quick grep. I will defer to whatever Dave Reid says about this.

Assigned:Dave Reid» Unassigned

I can't find any reason why we're adding space around the string to match. I searched through the history and other similar filter usages and cannot find anything. The code here was added in http://drupalcode.org/project/media.git/blob/846c757c6a6d768428a67d71c60... as a new file, so I don't have any idea if it itself originated from somewhere else.

If this works and doesn't cause any regressions then I don't see any reason not to commit it.

Status:Reviewed & tested by the community» Fixed

ok then, no reason holding this anymore
committed to 2.x

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Status:Closed (fixed)» Active

I'm sorry to have to reopen this issue. But I'm almost positive the whitespace was put there for a reason. I'm having an issue where my paragraph after the media tag is removed by the regex. For example:

Here the paragraph will dissapear.

[[{"type":"media","view_mode":"media_original","fid":"721","attributes":{"class":"media-image","height":"133","id":"2","style":"WIDTH: 200px; FLOAT: left; HEIGHT: 133px; MARGIN-LEFT: 15px; MARGIN-RIGHT: 15px","width":"200"}}]]<p>Proin fermentum consectetur nunc! Proin mi velit, tincidunt vel vulputate ac, congue sit amet mi. Nullam fermentum bibendum metus ut sagittis? Pellentesque felis lorem, pellentesque a dictum eget, dignissim eu ligula. Aliquam sodales leo sit amet massa laoreet, ut aliquet leo egestas. Vestibulum vulputate augue nec augue porttitor, non gravida ipsum viverra. Pellentesque et commodo mi.</p>

When the media is encapsulated by the paragraph there is no problem:

<p>[[{"type":"media","view_mode":"media_original","fid":"721","attributes":{"class":"media-image","height":"133","id":"2","style":"WIDTH: 200px; FLOAT: left; HEIGHT: 133px; MARGIN-LEFT: 15px; MARGIN-RIGHT: 15px","width":"200"}}]]<p>Proin fermentum consectetur nunc! Proin mi velit, tincidunt vel vulputate ac, congue sit amet mi. Nullam fermentum bibendum metus ut sagittis? Pellentesque felis lorem, pellentesque a dictum eget, dignissim eu ligula. Aliquam sodales leo sit amet massa laoreet, ut aliquet leo egestas. Vestibulum vulputate augue nec augue porttitor, non gravida ipsum viverra. Pellentesque et commodo mi.</p>

Reinserting the whitespace line fixes this issue for me. So I guess this needs some more testing before definitavely taking it out.