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

Commit

Permalink
Replace noscript with javascript content setting
Browse files Browse the repository at this point in the history
Fix #2671
Follow-up: restore the 'Allow scripts once' functionality

Auditors: @bbondy, @bridiver
  • Loading branch information
diracdeltas committed Jul 28, 2016
1 parent 440e93f commit 6b0d144
Show file tree
Hide file tree
Showing 9 changed files with 36 additions and 135 deletions.
1 change: 1 addition & 0 deletions app/extensions.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ let generateBraveManifest = () => {
'content/scripts/adInsertion.js',
'content/scripts/passwordManager.js',
'content/scripts/flashListener.js',
'content/scripts/noScript.js',
'content/scripts/themeColor.js'
]
},
Expand Down
11 changes: 11 additions & 0 deletions app/extensions/brave/content/scripts/noScript.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

if (chrome.contentSettings.javascript == 'block') {
document.querySelectorAll('script').forEach((s) => {

This comment has been minimized.

Copy link
@bridiver

bridiver Jul 28, 2016

Collaborator

I'm pretty sure you don't need this. You should be able to get them in this.webview.addEventListener('content-blocked', ...

This comment has been minimized.

Copy link
@diracdeltas

diracdeltas Jul 28, 2016

Author Member

thanks, i'll use that instead

// TODO: Send all of these in one IPC call
chrome.ipc.sendToHost('scripts-blocked',
s.src ? s.src : window.location.href)
})
}
2 changes: 0 additions & 2 deletions app/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ const PackageLoader = require('./package-loader')
const Extensions = require('./extensions')
const Filtering = require('./filtering')
const TrackingProtection = require('./trackingProtection')
const NoScript = require('./noScript')
const AdBlock = require('./adBlock')
const HttpsEverywhere = require('./httpsEverywhere')
const SiteHacks = require('./siteHacks')
Expand Down Expand Up @@ -366,7 +365,6 @@ app.on('ready', () => {
Extensions.init()
Filtering.init()
SiteHacks.init()
NoScript.init()
spellCheck.init()
HttpsEverywhere.init()
TrackingProtection.init()
Expand Down
92 changes: 0 additions & 92 deletions app/noScript.js

This file was deleted.

24 changes: 5 additions & 19 deletions js/components/frame.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ const debounce = require('../lib/debounce.js')
const getSetting = require('../settings').getSetting
const settings = require('../constants/settings')
const FindBar = require('./findbar.js')
const consoleStrings = require('../constants/console')
const { aboutUrls, isSourceAboutUrl, isTargetAboutUrl, getTargetAboutUrl, getBaseUrl } = require('../lib/appUrlUtil')
const { isFrameError } = require('../lib/errorUtil')
const locale = require('../l10n')
Expand Down Expand Up @@ -503,6 +502,11 @@ class Frame extends ImmutableComponent {
windowActions.setBlockedBy(this.props.frame, 'fingerprintingProtection', description)
}
break
case messages.SCRIPTS_BLOCKED:
method = (src) => {
windowActions.setBlockedBy(this.props.frame, 'noScript', src)
}
break
case messages.THEME_COLOR_COMPUTED:
method = (computedThemeColor) =>
windowActions.setThemeColor(this.props.frame, undefined, computedThemeColor || null)
Expand Down Expand Up @@ -737,14 +741,6 @@ class Frame extends ImmutableComponent {
this.webview.addEventListener('media-paused', ({title}) => {
windowActions.setAudioPlaybackActive(this.props.frame, false)
})
this.webview.addEventListener('console-message', (e) => {
if (this.props.enableNoScript && e.level === 2 &&
e.message && e.message.includes(consoleStrings.SCRIPT_BLOCKED)) {
// Note that the site was blocked
windowActions.setBlockedBy(this.props.frame,
'noScript', this.getScriptLocation(e.message))
}
})
this.webview.addEventListener('did-change-theme-color', ({themeColor}) => {
// Due to a bug in Electron, after navigating to a page with a theme color
// to a page without a theme color, the background is sent to us as black
Expand Down Expand Up @@ -772,16 +768,6 @@ class Frame extends ImmutableComponent {
this.webview.addEventListener('mousewheel', this.onMouseWheel.bind(this))
}

getScriptLocation (msg) {
const defaultMsg = '[Inline script]'
if (msg.includes(consoleStrings.EXTERNAL_SCRIPT_BLOCKED)) {
let match = /'.+?'/.exec(msg)
return match ? match[0].replace(/'/g, '') : defaultMsg
} else {
return defaultMsg
}
}

goBack () {
this.webview.goBack()
}
Expand Down
7 changes: 2 additions & 5 deletions js/components/noScriptInfo.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,12 @@ class NoScriptInfo extends ImmutableComponent {
<div>
<div className='truncate' data-l10n-args={JSON.stringify(l10nArgs)}
data-l10n-id={this.numberBlocked === 1 ? 'scriptBlocked' : 'scriptsBlocked'} />
<div>
<Button l10nId='allowScriptsOnce' className='actionButton'
onClick={this.onAllowOnce.bind(this)} />
</div>
<div>
{

This comment has been minimized.

Copy link
@diracdeltas

diracdeltas Jul 28, 2016

Author Member
// TODO: restore the allow-once button
// TODO: If this is a private tab, this should only allow scripts
// temporarily. Depends on #1824
<Button l10nId='allowScripts' className='subtleButton'
<Button l10nId='allow' className='actionButton'
onClick={this.onAllow.bind(this, false)} />
}
</div>
Expand Down
13 changes: 0 additions & 13 deletions js/constants/console.js

This file was deleted.

1 change: 1 addition & 0 deletions js/constants/messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ const messages = {
APP_STATE_CHANGE: _,
STOP_LOAD: _,
THEME_COLOR_COMPUTED: _,
SCRIPTS_BLOCKED: _, /** @arg {string} src */
HIDE_CONTEXT_MENU: _,
LEAVE_FULL_SCREEN: _,
ENTER_FULL_SCREEN: _,
Expand Down
20 changes: 16 additions & 4 deletions js/state/contentSettings.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,18 @@ const getContentSettingsFromSiteSettings = (appState) => {
setting: getPasswordManagerEnabled(appState) ? 'allow' : 'block',
primaryPattern: '*'
}],
javascript: [],
javascript: [{
setting: braveryDefaults.noScript ? 'block' : 'allow',
primaryPattern: '*'
}, {
setting: 'allow',
secondaryPattern: '*',
primaryPattern: 'file:///*'
}, {
setting: 'allow',
secondaryPattern: '*',
primaryPattern: 'chrome-extension://*'

This comment has been minimized.

Copy link
@bridiver

bridiver Jul 28, 2016

Collaborator

should we restrict this to our internal extension? What is the behavior on Chrome when js is disabled?

This comment has been minimized.

Copy link
@diracdeltas

diracdeltas Jul 28, 2016

Author Member

pretty sure chrome allows extensions to run since pdfjs works fine with scripts off

}],
canvasFingerprinting: [{
setting: braveryDefaults.fingerprintingProtection ? 'block' : 'allow',
primaryPattern: '*'
Expand All @@ -97,9 +108,10 @@ const getContentSettingsFromSiteSettings = (appState) => {
let hostSettings = appState.get('siteSettings').toJS()
for (var hostPattern in hostSettings) {
let hostSetting = hostSettings[hostPattern]
if (hostSetting.noScript) {
// TODO(bridiver) - enable this when we support temporary overrides
// addContentSettings(contentSettings.javascript, hostPattern)
if (typeof hostSetting.noScript === 'boolean') {
// TODO: support temporary override
addContentSettings(contentSettings.javascript, hostPattern, '*',
hostSetting.noScript ? 'block' : 'allow')
}
if (hostSetting.cookieControl) {
if (hostSetting.cookieControl === 'block3rdPartyCookie') {
Expand Down

0 comments on commit 6b0d144

Please sign in to comment.