Make WordPress Core

Opened 5 years ago

Closed 4 years ago

#47421 closed defect (bug) (fixed)

Media with fragments duplicate in enclosures

Reported by: dshanske's profile dshanske Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.6 Priority: normal
Severity: normal Version:
Component: Feeds Keywords: good-first-bug has-patch needs-testing
Focuses: Cc:

Description

I am using media fragment ids(https://www.w3.org/TR/media-frags/) to specify retrieval points in audio/video and the enclosure code picks this up as different enclosures, so I have the same media file multiple times. Suggest duplicate checking in do_enclose after stripping the fragments.

Attachments (2)

47421.diff (695 bytes) - added by archduck 5 years ago.
Strip fragments from url
47421-2.diff (644 bytes) - added by archduck 5 years ago.
Send all the urls through the fragslasher

Download all attachments as: .zip

Change History (21)

#1 @raaaahman
5 years ago

Working on it at #WCEU

Edit: Didn't manage to solve it. Feel free to try.

Last edited 5 years ago by raaaahman (previous) (diff)

@archduck
5 years ago

Strip fragments from url

#2 @archduck
5 years ago

  • Keywords has-patch added; needs-patch removed

It looks like do_enclose does some checking for duplicate urls already, when it runs the enclosure_links filter - but it doesn't take url fragments into account. This patch adds a check for fragments on new urls and strips them away if present. It only affects the enclosures, so the rest of the markup stays intact.

Last edited 5 years ago by archduck (previous) (diff)

#3 @archduck
5 years ago

  • Keywords needs-testing added

#4 follow-up: @dshanske
5 years ago

@archduck Why check for fragments when the strip fragment function strips regardless? Isn't in more efficient just to run everything through it?

#5 in reply to: ↑ 4 @archduck
5 years ago

Replying to dshanske:

@archduck Why check for fragments when the strip fragment function strips regardless? Isn't in more efficient just to run everything through it?

Good point. Checking for fragments with parse_url is an unnecessary step here.

Especially since strip_fragment_from_url itself runs parse_url - and so urls with fragments would undergo parse_url a second time.

Better to just send everything through it.

@archduck
5 years ago

Send all the urls through the fragslasher

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


5 years ago

#7 @SergeyBiryukov
5 years ago

  • Milestone changed from Awaiting Review to 5.4

#8 @SergeyBiryukov
5 years ago

  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#9 @audrasjb
5 years ago

  • Milestone changed from 5.4 to Future Release

Hi,

With 5.4 Beta 3 approaching and the Beta period reserved for bugs introduced during the cycle, this is being moved to Future Release. If any maintainer or committer feels this should be included or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

#10 @dshanske
4 years ago

  • Milestone changed from Future Release to 5.6

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


4 years ago

#12 @Mista-Flo
4 years ago

Could you please give some steps to reproduce? How can I test to add media fragment to my dev website?

It would allow us to write a unit test for it.

#13 @dshanske
4 years ago

Add multiple urls to a post that are the same mp3 file, adding ?t=130, ?t=145, etc.

Version 0, edited 4 years ago by dshanske (next)

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


4 years ago

#15 @helen
4 years ago

For those of us who are not RSS spec experts, could I get a brief overview of what multiple enclosures are meant to be used for and what issues it causes to have the same media item but with different fragments represented as multiple enclosures?

I deal with enclosures but only in the context of a podcasting plugin that doesn't currently have any workflow for something like timestamp indicators, I suppose it would not be good for the podcast RSS feed to have the same item multiple times?

#16 @dshanske
4 years ago

@helen Enclosures are used for podcasts.. usually multiple enclosures would reflect a video and an audio file..this is generating multiple versions of the same file... creates podcatcher issues

#17 @helen
4 years ago

@dshanske Thanks! I'll leave this in 5.6 for the moment then.

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


4 years ago

#19 @helen
4 years ago

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

In 49552:

Feeds: Don't treat media URLs with fragments as unique for enclosures.

Props archduck, dshanske.
Fixes #47421.

Note: See TracTickets for help on using tickets.