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

Using temporarySiteSettings to store private tab preferences #4469

Merged
merged 3 commits into from
Oct 4, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion js/components/braveryPanel.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ class BraveryPanel extends ImmutableComponent {
get isFpShown () {
return this.props.braveryPanelDetail.get('expandFp')
}
get isPrivate () {
return this.props.frameProps.getIn(['isPrivate'])
}
get redirectedResources () {
return this.props.frameProps.get('httpsEverywhere')
}
Expand Down Expand Up @@ -144,7 +147,7 @@ class BraveryPanel extends ImmutableComponent {
if (setting !== 'noScript' && (parsedUrl.protocol === 'https:' || parsedUrl.protocol === 'http:')) {
ruleKey = `https?://${parsedUrl.host}`
}
appActions.changeSiteSetting(ruleKey, setting, e.target.value)
appActions.changeSiteSetting(ruleKey, setting, e.target.value, this.isPrivate)
this.onReload()
}
get displayHost () {
Expand Down
6 changes: 3 additions & 3 deletions js/components/frame.js
Original file line number Diff line number Diff line change
Expand Up @@ -210,11 +210,11 @@ class Frame extends ImmutableComponent {
}
if (typeof activeSiteSettings.get('flash') === 'number') {
if (activeSiteSettings.get('flash') < Date.now()) {
appActions.removeSiteSetting(origin, 'flash')
appActions.removeSiteSetting(origin, 'flash', this.props.isPrivate)
}
}
if (activeSiteSettings.get('noScript') === 0) {
appActions.removeSiteSetting(origin, 'noScript')
appActions.removeSiteSetting(origin, 'noScript', this.props.isPrivate)
}
}

Expand Down Expand Up @@ -314,7 +314,7 @@ class Frame extends ImmutableComponent {

get zoomLevel () {
const zoom = this.props.frameSiteSettings && this.props.frameSiteSettings.get('zoomLevel')
appActions.removeSiteSetting(this.origin, 'zoomLevel')
appActions.removeSiteSetting(this.origin, 'zoomLevel', this.props.isPrivate)
return zoom
}

Expand Down
9 changes: 4 additions & 5 deletions js/components/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,10 @@ class Main extends ImmutableComponent {
}

get allSiteSettings () {
const activeFrame = FrameStateUtil.getActiveFrame(this.props.windowState)
if (activeFrame && activeFrame.get('isPrivate')) {
return this.props.appState.get('temporarySiteSettings')
}
Copy link
Member

Choose a reason for hiding this comment

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

need to be careful here that the allSiteSettings frame property is consistent with what is being sent in js/state/contentSettings.js and also registerPermissionHandler in app/filtering.js. the expected behavior is that private context inheirit settings from regular contexts but not vice-versa, so I think what you want here is a merge of temporarySiteSettings and siteSettings for private frames.

Copy link
Member Author

Choose a reason for hiding this comment

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

Address in 7de0c05.

return this.props.appState.get('siteSettings')
}

Expand All @@ -741,11 +745,6 @@ class Main extends ImmutableComponent {
}

get braveShieldsDisabled () {
const activeFrame = FrameStateUtil.getActiveFrame(this.props.windowState)
if (activeFrame && activeFrame.get('isPrivate')) {
return true
}

const activeRequestedLocation = this.activeRequestedLocation
if (!activeRequestedLocation) {
return true
Expand Down
4 changes: 0 additions & 4 deletions js/components/siteInfo.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,6 @@ class SiteInfo extends ImmutableComponent {
</ul>
</li>
}
// Disable in private mode for now
if (this.isPrivate) {
runInsecureContentInfo = null
}

return <Dialog onHide={this.props.onHide} className='siteInfo' isClickDismiss>
<ul onClick={(e) => e.stopPropagation()}>
Expand Down
94 changes: 49 additions & 45 deletions js/state/contentSettings.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,53 @@ const getBlock3rdPartyStorage = (braveryDefaults) => {
}
}

const hostSettingsToContentSettings = (hostSettings, contentSettings) => {
// We do 2 passes for setting content settings. On the first pass we consider all shield types.
for (let hostPattern in hostSettings) {
let hostSetting = hostSettings[hostPattern]
if (['number', 'boolean'].includes(typeof hostSetting.noScript)) {
addContentSettings(contentSettings.javascript, hostPattern, '*',
hostSetting.noScript === true ? 'block' : 'allow')
}
if (typeof hostSetting.runInsecureContent === 'boolean') {
addContentSettings(contentSettings.runInsecureContent, hostPattern, '*',
hostSetting.runInsecureContent ? 'allow' : 'block')
}
if (hostSetting.cookieControl) {
if (hostSetting.cookieControl === 'block3rdPartyCookie') {
addContentSettings(contentSettings.cookies, hostPattern, '*', 'block')
addContentSettings(contentSettings.cookies, hostPattern, parseSiteSettingsPattern(hostPattern), 'allow')
addContentSettings(contentSettings.referer, hostPattern, '*', 'block')
cookieExceptions.forEach((exceptionPair) =>
addContentSettings(contentSettings.cookies, exceptionPair[0], exceptionPair[1], 'allow'))
} else {
addContentSettings(contentSettings.cookies, hostPattern, '*', 'allow')
addContentSettings(contentSettings.referer, hostPattern, '*', 'allow')
}
}
if (typeof hostSetting.fingerprintingProtection === 'boolean') {
addContentSettings(contentSettings.canvasFingerprinting, hostPattern, '*', hostSetting.fingerprintingProtection ? 'block' : 'allow')
}
if (hostSetting.adControl) {
addContentSettings(contentSettings.adInsertion, hostPattern, '*', hostSetting.adControl === 'showBraveAds' ? 'allow' : 'block')
}
if (typeof hostSetting.flash === 'number') {
addContentSettings(contentSettings.flashActive, hostPattern, '*', 'allow')
}
}
// On the second pass we consider only shieldsUp === false settings since we want those to take precedence.
for (let hostPattern in hostSettings) {
let hostSetting = hostSettings[hostPattern]
if (hostSetting.shieldsUp === false) {
addContentSettings(contentSettings.cookies, hostPattern, '*', 'allow')
addContentSettings(contentSettings.canvasFingerprinting, hostPattern, '*', 'allow')
addContentSettings(contentSettings.adInsertion, hostPattern, '*', 'block')
addContentSettings(contentSettings.javascript, hostPattern, '*', 'allow')
addContentSettings(contentSettings.referer, hostPattern, '*', 'allow')
}
}
}

const getContentSettingsFromSiteSettings = (appState) => {
let braveryDefaults = siteSettings.braveryDefaults(appState, appConfig)

Expand Down Expand Up @@ -126,51 +173,8 @@ const getContentSettingsFromSiteSettings = (appState) => {
}]
}

let hostSettings = appState.get('siteSettings').toJS()
// We do 2 passes for setting content settings. On the first pass we consider all shield types.
for (let hostPattern in hostSettings) {
let hostSetting = hostSettings[hostPattern]
if (['number', 'boolean'].includes(typeof hostSetting.noScript)) {
addContentSettings(contentSettings.javascript, hostPattern, '*',
hostSetting.noScript === true ? 'block' : 'allow')
}
if (typeof hostSetting.runInsecureContent === 'boolean') {
addContentSettings(contentSettings.runInsecureContent, hostPattern, '*',
hostSetting.runInsecureContent ? 'allow' : 'block')
}
if (hostSetting.cookieControl) {
if (hostSetting.cookieControl === 'block3rdPartyCookie') {
addContentSettings(contentSettings.cookies, hostPattern, '*', 'block')
addContentSettings(contentSettings.cookies, hostPattern, parseSiteSettingsPattern(hostPattern), 'allow')
addContentSettings(contentSettings.referer, hostPattern, '*', 'block')
cookieExceptions.forEach((exceptionPair) =>
addContentSettings(contentSettings.cookies, exceptionPair[0], exceptionPair[1], 'allow'))
} else {
addContentSettings(contentSettings.cookies, hostPattern, '*', 'allow')
addContentSettings(contentSettings.referer, hostPattern, '*', 'allow')
}
}
if (typeof hostSetting.fingerprintingProtection === 'boolean') {
addContentSettings(contentSettings.canvasFingerprinting, hostPattern, '*', hostSetting.fingerprintingProtection ? 'block' : 'allow')
}
if (hostSetting.adControl) {
addContentSettings(contentSettings.adInsertion, hostPattern, '*', hostSetting.adControl === 'showBraveAds' ? 'allow' : 'block')
}
if (typeof hostSetting.flash === 'number') {
addContentSettings(contentSettings.flashActive, hostPattern, '*', 'allow')
}
}
// On the second pass we consider only shieldsUp === false settings since we want those to take precedence.
for (let hostPattern in hostSettings) {
let hostSetting = hostSettings[hostPattern]
if (hostSetting.shieldsUp === false) {
addContentSettings(contentSettings.cookies, hostPattern, '*', 'allow')
addContentSettings(contentSettings.canvasFingerprinting, hostPattern, '*', 'allow')
addContentSettings(contentSettings.adInsertion, hostPattern, '*', 'block')
addContentSettings(contentSettings.javascript, hostPattern, '*', 'allow')
addContentSettings(contentSettings.referer, hostPattern, '*', 'allow')
}
}
hostSettingsToContentSettings(appState.get('siteSettings').toJS(), contentSettings)
hostSettingsToContentSettings(appState.get('temporarySiteSettings').toJS(), contentSettings)
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is going to send the private settings to the non-private tabs. We can't sync all the sessions, we have to send the temp + regular to the private session and just the regular to the regular session. Probably use setUserPref with the incognito flag instead of registerUserPrefs

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed in 7de0c05.

Copy link
Member

Choose a reason for hiding this comment

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

shouldn't temporarySiteSettings get added only if the content settings are being sent to a private frame?

Copy link
Member

Choose a reason for hiding this comment

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

nvm @bridiver made the same comment above


return { content_settings: contentSettings }
}
Expand Down