WordPress.org

Make WordPress Core

Opened 14 months ago

Last modified 5 months ago

#47421 reviewing defect (bug)

Media with fragments duplicate in enclosures

Reported by: dshanske Owned by: SergeyBiryukov
Milestone: Future Release 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 12 months ago.
Strip fragments from url
47421-2.diff (644 bytes) - added by archduck 12 months ago.
Send all the urls through the fragslasher

Download all attachments as: .zip

Change History (11)

#1 @raaaahman
14 months ago

Working on it at #WCEU

Version 0, edited 14 months ago by raaaahman (next)

@archduck
12 months ago

Strip fragments from url

#2 @archduck
12 months 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 12 months ago by archduck (previous) (diff)

#3 @archduck
12 months ago

  • Keywords needs-testing added

#4 follow-up: @dshanske
12 months 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
12 months 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
12 months ago

Send all the urls through the fragslasher

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


9 months ago

#7 @SergeyBiryukov
9 months ago

  • Milestone changed from Awaiting Review to 5.4

#8 @SergeyBiryukov
9 months ago

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

#9 @audrasjb
5 months 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.

Note: See TracTickets for help on using tickets.