Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#41052 closed defect (bug) (fixed)

Need validation update in video widget

Reported by: ravipatel's profile ravipatel Owned by: westonruter's profile westonruter
Milestone: 4.9 Priority: high
Severity: normal Version: 4.8
Component: Widgets Keywords: has-patch needs-testing
Focuses: ui, javascript Cc:


Without Validation not insert video widget content.

When i have insert just text not validate this one.

Attachments (7)

video-widget-validation.png (63.7 KB) - added by ravipatel 7 years ago.
video-widget-1.png (7.4 KB) - added by ravipatel 7 years ago.
video-widget-forntside.png (29.5 KB) - added by ravipatel 7 years ago. (2.3 KB) - added by gk.loveweb 7 years ago.
Fixes for video widget (2.5 KB) - added by octalmage 7 years ago.
Patch didn't apply, refreshed at 4.8.2.
41052.3.diff (2.9 KB) - added by westonruter 7 years ago.
41052.4.diff (2.9 KB) - added by westonruter 7 years ago.

Download all attachments as: .zip

Change History (18)

#1 @westonruter
7 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 4.8.1

@timmydcrawford This seems to relate to #40935.

#2 @ravipatel
7 years ago

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

Same issue with image and audio widget also. please mark this one.

#3 @westonruter
7 years ago

@ravipatel I don't think we should be accepting invalid URLs though. It should have the required extension or be a valid oEmbed. So is this not valid?

#4 @SergeyBiryukov
7 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.

7 years ago

#6 @jbpaul17
7 years ago

  • Milestone changed from 4.8.1 to 4.9

Punting to 4.9 per today's bug scrub.

7 years ago

Fixes for video widget

#7 @westonruter
7 years ago

  • Keywords has-patch added; needs-patch removed

Directly relates to #34115 which will impact how we handle arbitrary URLs for video embeds. We'll use the oEmbed response then to validate URLs that don't match the supplied pattern.

#8 @westonruter
7 years ago

  • Keywords needs-testing added

#9 @westonruter
7 years ago

  • Priority changed from normal to high

Bumping priority to high for visibility and alignment with 4.9 goals, and given proximity to beta 1 deadline.

7 years ago

Patch didn't apply, refreshed at 4.8.2.

7 years ago

7 years ago

#10 @westonruter
7 years ago

@gk.loveweb I iterated on your patch a bit and I think I came to something that works better in 41052.4.diff. Would you give it a try? By moving the logic to updateoEmbed we can short-circuit the fetch logic earlier, since updateoEmbed is what calls fetch to begin with.

The audio widget needs some more work, I believe. For one it doesn't support oEmbeds at all currently.

#11 @westonruter
7 years ago

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

In 41933:

Widgets: Harden logic for checking for valid media URLs.

  • Update deprecated isHostedVideo method to always return true since all oEmbeds are now supported.
  • Disable scanImage for non-image media widgets.
  • Ensure embed URL field element is matched from current media frame instance due to bug where media frames are not destroyed upon closing.
  • Update error message for invalid video URLs to ask user to check the URL for validity.

Props gk.loveweb, octalmage, westonruter.
See #42039, #40935.
Fixes #41052.

Note: See TracTickets for help on using tickets.