Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protect from Link Tracking on Tumblr #2733

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ablanathtanalba
Copy link
Contributor

@ablanathtanalba ablanathtanalba commented Jan 13, 2021

Turns out tumblr has familiar redirects on every outgoing link. Whether or not it's for tracking or just analytics, this proposed change scrubs the links of all unwanted things and reassigns the href to what the user expects.

Before:

Screen Shot 2021-01-13 at 2 31 35 PM

After:

Screen Shot 2021-01-13 at 2 31 06 PM

@ablanathtanalba ablanathtanalba added the first-party relating to first-party scripts label Jan 13, 2021
@ablanathtanalba ablanathtanalba changed the title Tumblr link tracking Protect from Link Tracking on Tumblr Jan 13, 2021
@ablanathtanalba
Copy link
Contributor Author

Realized I never wrote a test to go along with this PR, so I just added one

Copy link
Member

@ghostwords ghostwords left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Please see inline feedback.

}

for (let attr of a.attributes) {
if (!['target', 'class', 'style'].includes(attr.name)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also keep ARIA attributes?

@@ -0,0 +1,36 @@
// only observed links with this format out in the wild
let tumblr_links = "a[href^='https://t.umblr.com/redirect?']";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is almost identical to https://github.com/EFForg/privacybadger/blob/master/src/js/firstparties/google-static.js. What if we reuse google-static.js, conditionally setting the DOM selector variable (based on document.domain maybe)?

@@ -164,4 +165,38 @@ QUnit.test('google search de-instrumentation', (assert) => {
fixture.appendChild(util_script);
});

QUnit.test('tumblr link unwrapping', (assert) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably don't need a unit test for this PR, especially if we reuse google-static.js for Tumblr.

Copy link
Member

@ghostwords ghostwords left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where should I be seeing these links, in the Tumblr dashboard, on individual Tumblr blogs, or both?

Has t.umblr.com been retired in favor of href.li? I see href.li in the dashboard and on individual blogs.

The dashboard comes with infinite scrolling, so a single pass of unwrapping isn't enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
first-party relating to first-party scripts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants