WordPress.org

Make WordPress Core

Opened 6 months ago

Closed 4 months ago

#40977 closed defect (bug) (fixed)

Widgets: Query param for `loop` added for non-hosted external videos

Reported by: westonruter Owned by: westonruter
Milestone: 4.8.1 Priority: normal
Severity: normal Version: 4.8
Component: Widgets Keywords: has-patch has-unit-tests fixed-major
Focuses: Cc:

Description (last modified by westonruter)

When embedding an external video file into a video widget, the video shortcode is currently failing to render the video due to the erroneous injection of a loop query parameter. The loop parameter was specifically added to work around a Vimeo issue, but the logic was not properly restricted to only effect Vimeo URLs. For more background, see https://github.com/xwp/wordpress-develop/pull/232#issuecomment-306861073

Note that the need for the loop parameter will be eliminated once the upgrade to ME.js 4.x is done. See #39686.

Attachments (1)

40977.1.diff (2.3 KB) - added by timmydcrawford 6 months ago.

Download all attachments as: .zip

Change History (8)

This ticket was mentioned in Slack in #core-customize by westonruter. View the logs.


6 months ago

#2 @westonruter
6 months ago

  • Description modified (diff)

#3 @timmydcrawford
6 months ago

  • Keywords has-patch has-unit-tests added; needs-patch removed

In 40977.1.diff the logic for adding the vimeo loop query param has been moved to wp_video_shortcode and tests have been added.

YouTube loop bug is still present, hopefully the mejs upgrade lands soon so we can back out this fix entirely.

This diff does fix the issue seen with linking to external mp4 files.

#4 @westonruter
6 months ago

  • Owner set to westonruter
  • Status changed from new to accepted

#5 @westonruter
6 months ago

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

In 40892:

Media: Restrict appending loop parameter to Vimeo URLs specifically and not all external URLs in Video widget (via shortcode).

Fixes issue where Video widgets embedding external files fail to get recognized due to the presence of the loop param after the video filename, even though it has a recognized extension. Regardless, the loop param is only present to fix a Vimeo issue in ME.js 2.x.

Props timmydcrawford.
Amends [40640].
See #39686, #39994.
Fixes #40977.

#6 @westonruter
6 months ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Re-opening for merging into 4.8 branch for 4.8.1 release.

#7 @westonruter
4 months ago

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

In 41054:

Media: Restrict appending loop parameter to Vimeo URLs specifically and not all external URLs in Video widget (via shortcode).

Fixes issue where Video widgets embedding external files fail to get recognized due to the presence of the loop param after the video filename, even though it has a recognized extension. Regardless, the loop param is only present to fix a Vimeo issue in ME.js 2.x.

Merges [40892] into the 4.8 branch.
Props timmydcrawford.
Amends [40640].
See #39686, #39994.
Fixes #40977 for 4.8.1.

Note: See TracTickets for help on using tickets.