Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Commit

Permalink
prevent twitter -> mobile.twitter.com noscript redirect
Browse files Browse the repository at this point in the history
fix #2884

Auditors: @jonathansampson

Test Plan: turn on block scripts, go to twitter.com, verify that it doesn't redirect to mobile
  • Loading branch information
diracdeltas committed Sep 27, 2016
1 parent 5166964 commit 41acbd0
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 1 deletion.
9 changes: 8 additions & 1 deletion app/siteHacks.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,22 @@ module.exports.init = () => {
let domain = URL.parse(details.url).hostname.split('.').slice(-2).join('.')
let hack = siteHacks[domain]
let customCookie
let cancel
const firstPartyUrl = Filtering.getMainFrameUrl(details)
if (hack && hack.onBeforeSendHeaders) {
const result = hack.onBeforeSendHeaders.call(this, details)
if (result && result.customCookie) {
customCookie = result.customCookie
} else if (Filtering.isResourceEnabled(appConfig.resourceNames.NOSCRIPT, firstPartyUrl) &&
result && result.cancel) {
// cancel is only called on Twitter where noscript is enabled
cancel = true
}
}
return {
resourceName,
customCookie
customCookie,
cancel
}
})
Filtering.registerBeforeRequestFilteringCB((details) => {
Expand Down
11 changes: 11 additions & 0 deletions js/data/siteHacks.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,17 @@ module.exports.siteHacks = {
}
}
},
'twitter.com': {
onBeforeSendHeaders: function(details) {
if (details.requestHeaders.Referer &&
details.requestHeaders.Referer.startsWith('https://twitter.com/') &&
details.url.startsWith('https://mobile.twitter.com/')) {
return {
cancel: true
}
}
}
},
'play.spotify.com': {
enableFlashCTP: true
},
Expand Down

2 comments on commit 41acbd0

@jonathansampson
Copy link
Collaborator

Choose a reason for hiding this comment

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

@diracdeltas I am not able to confirm that this resolves the issue, as I understand it.

Scenario: User navigates to http://www.twitter.com, and then expands the Bravery Settings Panel and turns on Block Scripts. The frame reloads, and the user is then redirected to http://mobile.twitter.com via a <noscript> element.

The above scenario still plays through.

@srirambv
Copy link
Collaborator

Choose a reason for hiding this comment

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

@diracdeltas @jonathansampson Checked on 0.12.3dev-RC1 and this is the current behavior. If I enable block-script globally and then open twitter.com it doesn't redirect to mobile.twitter.com..But if I open twitter.com and then enable block-script at page level then it redirects to mobile.twitter.com

Please sign in to comment.