WordPress.org

Make WordPress Core

Opened 6 weeks ago

Closed 6 weeks ago

#41784 closed enhancement (fixed)

Twitter oembeds include visitor tracking

Reported by: norcross Owned by: johnbillion
Milestone: 4.9 Priority: normal
Severity: normal Version:
Component: Embeds Keywords: has-patch commit
Focuses: Cc:

Description

Currently, Twitter adds a visitor tracking mechanism when tweets are embedded. They allow for sites to turn it off via a meta tag, or adding the dnt parameter to the embed request URL. This can be re-enabled using the existing oembed_fetch_url filter. Thanks to @joostdevalk for writing the code.

Attachments (1)

41784.diff (546 bytes) - added by norcross 6 weeks ago.
Additional line for adding the dnt argument

Download all attachments as: .zip

Change History (15)

@norcross
6 weeks ago

Additional line for adding the dnt argument

#1 @norcross
6 weeks ago

  • Keywords has-patch 2nd-opinion added

#2 @joostdevalk
6 weeks ago

+1, obviously. Seems such a simple way to prevent a lot of unnecessary tracking. Core doesn’t track visitors, why would we let other providers do that?

#3 @joostdevalk
6 weeks ago

Note that this works for more than just Twitter, it seems to be a pseudo standard, see for instance:

http://docs.embed.ly/docs/oembed

#4 @SergeyBiryukov
6 weeks ago

  • Milestone changed from Awaiting Review to 4.9

#5 follow-up: @johnbillion
6 weeks ago

  • Keywords 2nd-opinion removed
  • Type changed from defect (bug) to enhancement
  • Version trunk deleted

+1 from me as long as it doesn't cause any of the other oEmbed providers to malfunction. I can't imagine it would, but it needs to be double checked. The tests in the patch on #32360 could be used to verify this, with a bit of persuading.

#6 follow-up: @norcross
6 weeks ago

I pulled down the oembed test from #32360 and the testing results were identical when including the dnt flag, but another set of eyes would be helpful.

#7 in reply to: ↑ 6 @boogah
6 weeks ago

Replying to norcross:

I pulled down the oembed test from #32360 and the testing results were identical when including the dnt flag, but another set of eyes would be helpful.

I am also seeing identical output between the oEmbed tests on trunk and with @norcross's patch in place.

#8 in reply to: ↑ 5 @mikeschroder
6 weeks ago

Replying to johnbillion:

+1 from me as long as it doesn't cause any of the other oEmbed providers to malfunction. I can't imagine it would, but it needs to be double checked. The tests in the patch on #32360 could be used to verify this, with a bit of persuading.

+1 for doing this from me as well.

#9 @matt
6 weeks ago

This looks like a good change, let's get it in trunk and see if there are any unintended downsides.

#10 @peterwilsoncc
6 weeks ago

  • Keywords commit added
  • Owner set to peterwilsoncc
  • Status changed from new to assigned

I'll aim for Monday AEST.

#11 follow-up: @netweb
6 weeks ago

If the tests from # #32360 are going to be pulled in, please remove Vine before committing

The Vine oEmbed is being removed for 4.9 in #41817

#12 in reply to: ↑ 11 @peterwilsoncc
6 weeks ago

Replying to netweb:

If the tests from # #32360 are going to be pulled in, please remove Vine before committing

They've gone in once and were subsequently reverted as they were frequently throwing errors due to API limits and network issues. I'll use them to test locally but leave them as their own ticket.

#13 @johnbillion
6 weeks ago

  • Owner changed from peterwilsoncc to johnbillion

@peterwilsoncc I spent the afternoon on Wednesday updating the tests in #32360 and I can confirm that adding the dnt parameter doesn't break tests for any of the providers.

I'll get this in.

#14 @johnbillion
6 weeks ago

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

In 41345:

Embeds: Add the dnt (Do Not Track) query parameter to all oEmbed provider URLs.

This means that, for those providers that support this somewhat de-facto standard, visitor tracking is disabled by default for all embeds.

Props norcross, joostdevalk

Fixes #41784

Note: See TracTickets for help on using tickets.