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

prefs that require browser restart should show prompt for browser restart #2546

Closed
wants to merge 1 commit into from
Closed

prefs that require browser restart should show prompt for browser restart #2546

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions app/extensions/brave/locales/en-US/app.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,3 +149,7 @@ flashTitle=Flash Object Blocked
flashRightClick=Right-click to run Adobe Flash
flashSubtext=from {{source}} on {{site}}.
flashExpirationText=Approvals reset 7 days after last visit.
prefsRestart=Do you want to restart now?
yes=Yes
no=No
neverForThisSite=Never for this site
33 changes: 31 additions & 2 deletions app/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ let throttleKeytar = false
// Map of password notification bar messages to their callbacks
const passwordCallbacks = {}

const prefsRestartCallbacks = {}

/**
* Gets the master key for encrypting login credentials from the OS keyring.
*/
Expand Down Expand Up @@ -381,6 +383,26 @@ app.on('ready', () => {
app.quit()
})

ipcMain.on(messages.PREFS_RESTART, () => {
var message = locale.translation('prefsRestart')

appActions.showMessageBox({
buttons: [locale.translation('yes'), locale.translation('no')],
options: {
persist: false
},
message
})
prefsRestartCallbacks[message] = (buttonIndex, persist) => {
if (buttonIndex === 0) {
Copy link
Member

Choose a reason for hiding this comment

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

I think this is sometimes being called before the pref is actually saved. if i have flash enabled and disable in prefs then click 'yes' to restart, it is still enabled when Brave relaunches.

app.relaunch({args: process.argv.slice(1) + ['--relaunch']})
app.exit(0)
} else {
appActions.hideMessageBox(message)
}
}
})

ipcMain.on(messages.UPDATE_APP_MENU, (e, args) => {
Menu.init(AppStore.getState().get('settings'), args)
})
Expand Down Expand Up @@ -597,7 +619,11 @@ app.on('ready', () => {
if (!(message in passwordCallbacks)) {
// Notification not shown already
appActions.showMessageBox({
buttons: ['Yes', 'No', 'Never for this site'],
buttons: [
locale.translation('yes'),
locale.translation('no'),
locale.translation('neverForThisSite')
],
options: {
persist: false,
advancedText: '[Password settings]',
Expand Down Expand Up @@ -633,10 +659,13 @@ app.on('ready', () => {
}
})

ipcMain.on(messages.NOTIFICATION_RESPONSE, (e, message, buttonIndex) => {
ipcMain.on(messages.NOTIFICATION_RESPONSE, (e, message, buttonIndex, persist) => {
if (passwordCallbacks[message]) {
passwordCallbacks[message](buttonIndex)
}
if (prefsRestartCallbacks[message]) {
prefsRestartCallbacks[message](buttonIndex, persist)
}
})

// Setup the crash handling
Expand Down
6 changes: 5 additions & 1 deletion app/locale.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,11 @@ var rendererIdentifiers = function () {
'historySuggestionTitle',
'aboutPagesSuggestionTitle',
'searchSuggestionTitle',
'topSiteSuggestionTitle'
'topSiteSuggestionTitle',
'prefsRestart',
'yes',
'no',
'neverForThisSite'
]
}

Expand Down
5 changes: 5 additions & 0 deletions js/about/preferences.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,7 @@ class ShieldsTab extends ImmutableComponent {
class SecurityTab extends ImmutableComponent {
onToggleFlash (e) {
aboutActions.setResourceEnabled(flash, e.target.value)
ipc.send(messages.PREFS_RESTART)
}
render () {
return <div>
Expand Down Expand Up @@ -591,6 +592,10 @@ class AboutPreferences extends React.Component {
settings: this.state.settings.set(key, value)
})
aboutActions.changeSetting(key, value)
if (key === settings.DO_NOT_TRACK || key === settings.HARDWARE_ACCELERATION_ENABLED ||
key === settings.PDFJS_ENABLED) {
Copy link
Member

Choose a reason for hiding this comment

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

@bbondy was there a reason you added the restart requirement for PDFJS in e962ef9? it seems to work fine when i turn it on/off and don't restart

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure how to reproduce but sometimes even having the option on gives a save prompt. It's something related to flipping pref though. It works reliably with a restart without the bug.

ipc.send(messages.PREFS_RESTART)
}
}

render () {
Expand Down
1 change: 1 addition & 0 deletions js/constants/messages.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ const messages = {
SHORTCUT_PREV_TAB: _,
// Misc application events
QUIT_APPLICATION: _,
PREFS_RESTART: _,
UPDATE_APP_MENU: _, /** @arg {Object} args menu args to update */
CERT_ERROR: _, /** @arg {Object} details of certificate error */
LOGIN_REQUIRED: _, /** @arg {Object} details of the login required request */
Expand Down