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

Commit

Permalink
Autoplay refactor
Browse files Browse the repository at this point in the history
1. Move autoplay to site settings
2. Introduce built-in site settings

fix #8847

Auditors: @bbondy, @bsclifton

Test Plan:
Covered by webdriver test and unit test
  • Loading branch information
darkdh authored and bsclifton committed May 15, 2017
1 parent d9748ae commit 1e35477
Show file tree
Hide file tree
Showing 18 changed files with 408 additions and 150 deletions.
26 changes: 16 additions & 10 deletions app/browser/reducers/autoplayReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ const {makeImmutable} = require('../../common/state/immutableUtil')
const {ipcMain, webContents} = require('electron')
const AppStore = require('../../../js/stores/appStore')
const siteSettings = require('../../../js/state/siteSettings')
const settings = require('../../../js/constants/settings')
const appActions = require('../../../js/actions/appActions')
const {getOrigin} = require('../../../js/state/siteUtil')
const locale = require('../../locale')
const messages = require('../../../js/constants/messages')
const urlParse = require('../../common/urlParse')
const getSetting = require('../../../js/settings').getSetting
const {autoplayOption} = require('../../common/constants/settingsEnums')

const showAutoplayMessageBox = (tabId) => {
const tab = webContents.fromTabID(tabId)
Expand All @@ -22,8 +24,12 @@ const showAutoplayMessageBox = (tabId) => {
}
const location = tab.getURL()
const origin = getOrigin(location)
if (getSetting(settings.AUTOPLAY_MEDIA) === autoplayOption.ALWAYS_ALLOW) {
appActions.changeSiteSetting(origin, 'autoplay', true)
return
}
const originSettings = siteSettings.getSiteSettingsForURL(AppStore.getState().get('siteSettings'), origin)
if (originSettings && originSettings.get('noAutoplay') === true) {
if (originSettings && originSettings.get('autoplay') === false) {
return
}
const message = locale.translation('allowAutoplay', {origin})
Expand All @@ -43,20 +49,20 @@ const showAutoplayMessageBox = (tabId) => {
ipcMain.once(messages.NOTIFICATION_RESPONSE, (e, msg, buttonIndex, persist) => {
if (msg === message) {
appActions.hideNotification(message)
let ruleKey = origin
const parsedUrl = urlParse(location)
if ((parsedUrl.protocol === 'https:' || parsedUrl.protocol === 'http:')) {
ruleKey = `https?://${parsedUrl.host}`
}
if (buttonIndex === 0) {
appActions.changeSiteSetting(ruleKey, 'noAutoplay', false)

appActions.changeSiteSetting(origin, 'autoplay', true)
if (tab && !tab.isDestroyed()) {
tab.reload()
tab.on('destroyed', function temporaryAllow (e) {
if (!persist) {
appActions.removeSiteSetting(origin, 'autoplay')
// tab.removeListener('did-finish-load', temporaryAllow)
}
})
}
} else {
if (persist) {
appActions.changeSiteSetting(ruleKey, 'noAutoplay', true)
appActions.changeSiteSetting(origin, 'autoplay', false)
}
}
}
Expand Down
8 changes: 7 additions & 1 deletion app/common/constants/settingsEnums.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,16 @@ const fullscreenOption = {
ALWAYS_ALLOW: 'alwaysAllow'
}

const autoplayOption = {
ALWAYS_ASK: 'alwaysAsk',
ALWAYS_ALLOW: 'alwaysAllow'
}

module.exports = {
startsWithOption,
newTabMode,
bookmarksToolbarMode,
tabCloseAction,
fullscreenOption
fullscreenOption,
autoplayOption
}
1 change: 0 additions & 1 deletion app/extensions/brave/locales/en-US/bravery.properties
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,3 @@ httpReroutes={[plural(httpsUpgradeCount)]}
httpReroutes[one]=HTTPS Upgrade
httpReroutes[other]=HTTPS Upgrades
editBraveryGlobalSettings=Edit default shield settings…
noAutoplay=Block Autoplay
1 change: 1 addition & 0 deletions app/extensions/brave/locales/en-US/preferences.properties
Original file line number Diff line number Diff line change
Expand Up @@ -367,3 +367,4 @@ scaleSizeSmaller=Smaller
scaleSizeNormal=Normal
scaleSizeLarger=Larger
scaleSizeSuper=Supersize
autoplay=Autoplay Media
1 change: 1 addition & 0 deletions app/filtering.js
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,7 @@ function registerPermissionHandler (session, partition) {
appActions.hideNotification(message)
const result = !!(buttonIndex)
cb(result)
console.log(result)
if (persist) {
// remember site setting for this host
appActions.changeSiteSetting(origin, permission + 'Permission', result, isPrivate)
Expand Down
3 changes: 2 additions & 1 deletion app/locale.js
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,8 @@ var rendererIdentifiers = function () {
'importSuccessOk',
'connectionError',
'unknownError',
'allowAutoplay'
'allowAutoplay',
'autoplayMedia'
]
}

Expand Down
22 changes: 22 additions & 0 deletions app/sessionStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const downloadStates = require('../js/constants/downloadStates')
const {tabFromFrame} = require('../js/state/frameStateUtil')
const siteUtil = require('../js/state/siteUtil')
const { topSites, pinnedTopSites } = require('../js/data/newTabData')
const { defaultSiteSettingsList } = require('../js/data/siteSettingsList')
const sessionStorageVersion = 1
const filtering = require('./filtering')
const autofill = require('./autofill')
Expand Down Expand Up @@ -507,6 +508,25 @@ module.exports.runPostMigrations = (data) => {
return data
}

module.exports.runImportDefaultSettings = (data) => {
// import default site settings list
if (!data.defaultSiteSettingsListImported) {
for (var i = 0; i < defaultSiteSettingsList.length; ++i) {
let setting = defaultSiteSettingsList[i]
if (!data.siteSettings[setting.pattern]) {
data.siteSettings[setting.pattern] = {}
}
let targetSetting = data.siteSettings[setting.pattern]
if (!targetSetting.hasOwnProperty[setting.name]) {
targetSetting[setting.name] = setting.value
}
}
data.defaultSiteSettingsListImported = true
}

return data
}

/**
* Loads the browser state from storage.
*
Expand All @@ -532,6 +552,7 @@ module.exports.loadAppState = () => {
console.log('could not parse data: ', data, e)
}
data = exports.defaultAppState()
data = module.exports.runImportDefaultSettings(data)
}

if (loaded) {
Expand Down Expand Up @@ -561,6 +582,7 @@ module.exports.loadAppState = () => {
}

data = module.exports.runPostMigrations(data)
data = module.exports.runImportDefaultSettings(data)
}

data = setVersionInformation(data)
Expand Down
4 changes: 3 additions & 1 deletion docs/state.md
Original file line number Diff line number Diff line change
Expand Up @@ -276,9 +276,11 @@ AppStore
savePasswords: boolean, // only false or undefined/null
shieldsUp: boolean,
widevine: (number|boolean), // false = block widevine, 0 = allow once, 1 = allow always
zoomLevel: number
zoomLevel: number,
autoplay: boolean,
}
},
defaultSiteSettingsListImported: boolean,
sync: {
lastFetchTimestamp: integer // the last time new sync records were fetched in seconds
deviceId: Array.<number>,
Expand Down
22 changes: 15 additions & 7 deletions js/about/preferences.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const messages = require('../constants/messages')
const settings = require('../constants/settings')
const {changeSetting} = require('../../app/renderer/lib/settingsUtil')
const {passwordManagers, extensionIds} = require('../constants/passwordManagers')
const {startsWithOption, newTabMode, bookmarksToolbarMode, tabCloseAction, fullscreenOption} = require('../../app/common/constants/settingsEnums')
const {startsWithOption, newTabMode, bookmarksToolbarMode, tabCloseAction, fullscreenOption, autoplayOption} = require('../../app/common/constants/settingsEnums')

const aboutActions = require('./aboutActions')
const appActions = require('../actions/appActions')
Expand All @@ -49,7 +49,6 @@ const adInsertion = appConfig.resourceNames.AD_INSERTION
const trackingProtection = appConfig.resourceNames.TRACKING_PROTECTION
const httpsEverywhere = appConfig.resourceNames.HTTPS_EVERYWHERE
const safeBrowsing = appConfig.resourceNames.SAFE_BROWSING
const noAutoplay = appConfig.resourceNames.NOAUTOPLAY
const noScript = appConfig.resourceNames.NOSCRIPT
const flash = appConfig.resourceNames.FLASH

Expand Down Expand Up @@ -77,7 +76,8 @@ const permissionNames = {
'openExternalPermission': ['boolean'],
'protocolRegistrationPermission': ['boolean'],
'flash': ['boolean', 'number'],
'widevine': ['boolean', 'number']
'widevine': ['boolean', 'number'],
'autoplay': ['boolean']
}

const braveryPermissionNames = {
Expand All @@ -88,8 +88,7 @@ const braveryPermissionNames = {
'safeBrowsing': ['boolean'],
'httpsEverywhere': ['boolean'],
'fingerprintingProtection': ['boolean'],
'noScript': ['boolean', 'number'],
'noAutoplay': ['boolean']
'noScript': ['boolean', 'number']
}

class GeneralTab extends ImmutableComponent {
Expand Down Expand Up @@ -498,7 +497,6 @@ class ShieldsTab extends ImmutableComponent {
this.onChangeAdControl = this.onChangeAdControl.bind(this)
this.onToggleHTTPSE = this.onToggleSetting.bind(this, httpsEverywhere)
this.onToggleSafeBrowsing = this.onToggleSetting.bind(this, safeBrowsing)
this.onToggleNoAutoplay = this.onToggleSetting.bind(this, noAutoplay)
this.onToggleNoScript = this.onToggleSetting.bind(this, noScript)
}
onChangeAdControl (e) {
Expand Down Expand Up @@ -549,7 +547,6 @@ class ShieldsTab extends ImmutableComponent {
<SettingCheckbox checked={this.props.braveryDefaults.get('safeBrowsing')} dataL10nId='safeBrowsing' onChange={this.onToggleSafeBrowsing} />
<SettingCheckbox checked={this.props.braveryDefaults.get('noScript')} dataL10nId='noScriptPref' onChange={this.onToggleNoScript} />
<SettingCheckbox dataL10nId='blockCanvasFingerprinting' prefKey={settings.BLOCK_CANVAS_FINGERPRINTING} settings={this.props.settings} onChangeSetting={this.props.onChangeSetting} />
<SettingCheckbox checked={this.props.braveryDefaults.get('noAutoplay')} dataL10nId='noAutoplay' onChange={this.onToggleNoAutoplay} />
<Button l10nId='manageAdblockSettings' className='primaryButton manageAdblockSettings'
onClick={aboutActions.createTabRequested.bind(null, {
url: 'about:adblock'
Expand Down Expand Up @@ -658,6 +655,17 @@ class SecurityTab extends ImmutableComponent {
</SettingDropdown>
</SettingItem>
</SettingsList>
<DefaultSectionTitle data-l10n-id='autoplay' />
<SettingsList>
<SettingItem>
<SettingDropdown
value={getSetting(settings.AUTOPLAY_MEDIA, this.props.settings)}
onChange={changeSetting.bind(null, this.props.onChangeSetting, settings.AUTOPLAY_MEDIA)}>
<option data-l10n-id='alwaysAsk' value={autoplayOption.ALWAYS_ASK} />
<option data-l10n-id='alwaysAllow' value={autoplayOption.ALWAYS_ALLOW} />
</SettingDropdown>
</SettingItem>
</SettingsList>
<DefaultSectionTitle data-l10n-id='doNotTrackTitle' />
<SettingsList>
<SettingCheckbox dataL10nId='doNotTrack' prefKey={settings.DO_NOT_TRACK} settings={this.props.settings} onChangeSetting={this.props.onChangeSetting} />
Expand Down
3 changes: 0 additions & 3 deletions js/components/braveryPanel.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ class BraveryPanel extends ImmutableComponent {
this.onToggleCookieControl = this.onToggleSiteSetting.bind(this, 'cookieControl')
this.onToggleHTTPSE = this.onToggleSiteSetting.bind(this, 'httpsEverywhere')
this.onToggleFp = this.onToggleSiteSetting.bind(this, 'fingerprintingProtection')
this.onToggleNoAutoplay = this.onToggleSiteSetting.bind(this, 'noAutoplay')
this.onReload = this.onReload.bind(this)
this.onEditGlobal = this.onEditGlobal.bind(this)
this.onInfoClick = this.onInfoClick.bind(this)
Expand Down Expand Up @@ -166,7 +165,6 @@ class BraveryPanel extends ImmutableComponent {
const shieldsUp = this.props.braverySettings.shieldsUp
const noScriptEnabled = this.props.braverySettings.noScript
const httpseEnabled = this.props.braverySettings.httpsEverywhere
const noAutoplayEnabled = this.props.braverySettings.noAutoplay
const adControl = this.props.braverySettings.adControl
const fpEnabled = this.props.braverySettings.fingerprintingProtection
const adsBlockedStat = (this.blockedAds ? this.blockedAds.size : 0) + (this.blockedByTrackingList ? this.blockedByTrackingList.size : 0)
Expand Down Expand Up @@ -300,7 +298,6 @@ class BraveryPanel extends ImmutableComponent {
</FormDropdown>
<SwitchControl onClick={this.onToggleHTTPSE} rightl10nId='httpsEverywhere' checkedOn={httpseEnabled} disabled={!shieldsUp} className='httpsEverywhereSwitch' />
<SwitchControl onClick={this.onToggleNoScript} rightl10nId='noScript' checkedOn={noScriptEnabled} disabled={!shieldsUp} className='noScriptSwitch' />
<SwitchControl onClick={this.onToggleNoAutoplay} rightl10nId='noAutoplay' checkedOn={noAutoplayEnabled} disabled={!shieldsUp} className='noAutoplaySwitch' />
</div>
<div className='braveryControlGroup'>
<div className={cx({
Expand Down
7 changes: 2 additions & 5 deletions js/constants/appConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,14 @@ const adHost = process.env.AD_HOST || 'https://oip.brave.com'

const buildConfig = require('./buildConfig')
const isProduction = buildConfig.nodeEnv === 'production'
const {fullscreenOption} = require('../../app/common/constants/settingsEnums')
const {fullscreenOption, autoplayOption} = require('../../app/common/constants/settingsEnums')

module.exports = {
name: 'Brave',
contactUrl: 'mailto:[email protected]',
quitTimeout: 10 * 1000,
resourceNames: {
ADBLOCK: 'adblock',
NOAUTOPLAY: 'noAutoplay',
SAFE_BROWSING: 'safeBrowsing',
HTTPS_EVERYWHERE: 'httpsEverywhere',
TRACKING_PROTECTION: 'trackingProtection',
Expand All @@ -36,9 +35,6 @@ module.exports = {
cookieblock: {
enabled: true
},
noAutoplay: {
enabled: true
},
cookieblockAll: {
enabled: false
},
Expand Down Expand Up @@ -171,6 +167,7 @@ module.exports = {
'security.passwords.enpass-enabled': false,
'security.passwords.bitwarden-enabled': false,
'security.fullscreen.content': fullscreenOption.ALWAYS_ASK,
'security.autoplay.media': autoplayOption.ALWAYS_ASK,
'security.flash.installed': false,
// sync
'sync.enabled': false,
Expand Down
1 change: 1 addition & 0 deletions js/constants/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ const settings = {
SHUTDOWN_CLEAR_SITE_SETTINGS: 'shutdown.clear-site-settings',
FLASH_INSTALLED: 'security.flash.installed',
FULLSCREEN_CONTENT: 'security.fullscreen.content',
AUTOPLAY_MEDIA: 'security.autoplay.media',
// Autofill
AUTOFILL_ENABLED: 'privacy.autofill-enabled',
// Payments Tab
Expand Down
15 changes: 15 additions & 0 deletions js/data/siteSettingsList.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/* 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/. */
module.exports.defaultSiteSettingsList = [
{
"name" : "autoplay",
"pattern" : "https://www.youtube.com",
"value" : true,
},
{
"name" : "autoplay",
"pattern" : "https://www.twitch.tv",
"value" : true,
}
]
7 changes: 3 additions & 4 deletions js/state/contentSettings.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ const getDefaultUserPrefContentSettings = (braveryDefaults, appSettings, appConf
braveryDefaults = makeImmutable(braveryDefaults)
return Immutable.fromJS({
autoplay: [{
setting: braveryDefaults.get('noAutoplay') ? 'block' : 'allow',
setting: 'block',
primaryPattern: '*'
}],
cookies: getDefault3rdPartyStorageSettings(braveryDefaults, appSettings, appConfig),
Expand Down Expand Up @@ -281,8 +281,8 @@ const siteSettingsToContentSettings = (currentSiteSettings, defaultContentSettin
if (typeof siteSetting.get('widevine') === 'number' && braveryDefaults.get('widevine')) {
contentSettings = addContentSettings(contentSettings, 'plugins', primaryPattern, '*', 'allow', appConfig.widevine.resourceId)
}
if (typeof siteSetting.get('noAutoplay') === 'boolean') {
contentSettings = addContentSettings(contentSettings, 'autoplay', primaryPattern, '*', siteSetting.get('noAutoplay') ? 'block' : 'allow')
if (typeof siteSetting.get('autoplay') === 'boolean') {
contentSettings = addContentSettings(contentSettings, 'autoplay', primaryPattern, '*', siteSetting.get('autoplay') ? 'allow' : 'block')
}
})
// On the second pass we consider only shieldsUp === false settings since we want those to take precedence.
Expand All @@ -296,7 +296,6 @@ const siteSettingsToContentSettings = (currentSiteSettings, defaultContentSettin
contentSettings = addContentSettings(contentSettings, 'javascript', primaryPattern, '*', 'allow')
contentSettings = addContentSettings(contentSettings, 'referer', primaryPattern, '*', 'allow')
contentSettings = addContentSettings(contentSettings, 'plugins', primaryPattern, '*', 'allow')
contentSettings = addContentSettings(contentSettings, 'autoplay', primaryPattern, '*', 'allow')
}
})
return contentSettings
Expand Down
Loading

0 comments on commit 1e35477

Please sign in to comment.