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:

>
/**
 * 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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ankur’s picture

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

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.

Chris Matthews’s picture

Issue summary: View changes
Status: Active » Closed (outdated)

Closing this issue as outdated. However, if you think this issue is still important, please let us know and we will gladly re-open it for review.
sincerely,
- the Drupal Media Team