Make WordPress Core

Opened 12 years ago

Closed 11 years ago

#12115 closed defect (bug) (fixed)

Viddler embeds broken by wpautop() due to line breaks

Reported by: Viper007Bond Owned by:
Milestone: 3.0 Priority: high
Severity: normal Version: 2.9.1
Component: Embeds Keywords: has-patch
Focuses: Cc:


Viddler contains some new lines too that completely breaks the embed due to silly wpautop(). It shouldn't add <br />'s in the middle of HTML tags, but that's a fix for another day.

Worth putting into 2.9 branch.

Attachments (2)

12115.patch (1.2 KB) - added by Viper007Bond 12 years ago.
12115_2.diff (487 bytes) - added by ampt 11 years ago.

Download all attachments as: .zip

Change History (11)

12 years ago

#1 @Viper007Bond
12 years ago

  • Summary changed from Fix Viddler Embeds to Viddler embeds broken by wpautop() due to line breaks

#2 @Denis-de-Bernardy
12 years ago

I'd like to suggest a tiny enhancement to this patch: remove the regexp outright. it's not anchored, and as such serves no purpose beyond slowing things down.

#3 @Denis-de-Bernardy
12 years ago

  • Keywords commit removed

if we decide to keep the regexp, it should at least be fixed, e.g.

$domains = array('foo.com', 'bar.com');
$domains = implode('|', array_map('preg_quote', $domains));
if ( preg_match( "#^https?://(?:www\.)?(?:$domains)/#i", ... 

#4 @nacin
12 years ago

(In [14109]) Strip new lines from Viddler embeds, as we do for Scribd embeds. Avoids conflicts with wpautop. props Viper007Bond. see #12115.

#5 @nacin
12 years ago

Leaving open for 2.9.x

#6 @nacin
11 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [14249]) Simply search for newlines to strip instead of checking the oEmbed provider. fixes #12115.

#7 @nacin
11 years ago

  • Milestone changed from 2.9.3 to 3.0

Viddler has since fixed this, so no need to commit to the 2.9 branch.

After discussion with Viper in IRC, I've removed the regexp check on the provider, and instead went to a simple strpos to identify any newlines, to slightly improve performance and future-proof any providers that return new lines in the embed code.

11 years ago

#8 @ampt
11 years ago

  • Cc notfornoone@… added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Fix strpos paramter order, which should be haystack, needle instead of needle, haystack. See 12115_2.diff attached

#9 @nacin
11 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [14976]) Fix order of params to strpos. props ampt, fixes #12115.

Note: See TracTickets for help on using tickets.