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

ankur’s picture

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

Here's a patch that removes that line.

ankur’s picture

Status:Active» Needs review
ParisLiakos’s picture

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

travelertt’s picture

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.

travelertt’s picture

Status:Needs review» Reviewed & tested by the community
Devin Carlson’s picture

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

azinck’s picture

Works great for me, RTBC

ParisLiakos’s picture

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

azinck’s picture

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

Re-rolled against latest dev

aaron’s picture

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.

Dave Reid’s picture

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.

ParisLiakos’s picture

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.

alexverb’s picture

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.