Make WordPress Core

Opened 14 years ago

Closed 14 years ago

#12115 closed defect (bug) (fixed)

Viddler embeds broken by wpautop() due to line breaks

Reported by: viper007bond's profile 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 14 years ago.
12115_2.diff (487 bytes) - added by ampt 14 years ago.

Download all attachments as: .zip

Change History (11)

14 years ago

#1 @Viper007Bond
14 years ago

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

#2 @Denis-de-Bernardy
14 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
14 years ago

  • Keywords commit removed

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

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

#4 @nacin
14 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
14 years ago

Leaving open for 2.9.x

#6 @nacin
14 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
14 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.

14 years ago

#8 @ampt
14 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
14 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.