WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 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:

Description

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 4 years ago.
12115_2.diff (487 bytes) - added by ampt 4 years ago.

Download all attachments as: .zip

Change History (11)

Viper007Bond4 years ago

comment:1 Viper007Bond4 years ago

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

comment:2 Denis-de-Bernardy4 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.

comment:3 Denis-de-Bernardy4 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", ... 

comment:4 nacin4 years ago

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

comment:5 nacin4 years ago

Leaving open for 2.9.x

comment:6 nacin4 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.

comment:7 nacin4 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.

ampt4 years ago

comment:8 ampt4 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

comment:9 nacin4 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.