mirror of
https://github.com/mozilla/gecko-dev.git
synced 2024-10-13 21:35:39 +00:00
Bug 1473671 - Don't store persistent block permission if ESC pressed while showing permission doorhanger. r=florian
While showing a doorhanger permisison prompt, if the user presses the ESC key we call the secondary action callback, passing in whether any checkbox on the popup notification was checked. In the case of an autoplay-media permission prompt, we have a "remember" checkbox, which is checked by default. So pressing ESC means the user will remember a "block" result for the current site. We believe that users don't expect pressing ESC to result in a remembered decision, they expect pressing ESC to avoid making a decision. So we want to ignore the checkbox when ESC is pressed for autoplay-media. So this patch adds a new PopupNotification behaviour which reports the source of event which caused the action callback to be called. This enables the ESC key press to ignore storing a permission. Note: the change here to not store a permission on ESC press applies to all permissions, not just autoplay-media. MozReview-Commit-ID: IUWFN8LG9VF --HG-- extra : rebase_source : b004bc211177a7bfb6dcea563d75db9a6750b214
This commit is contained in:
parent
2a45f8f49d
commit
4a6ff21ab5
@ -29,6 +29,8 @@ var tests = [
|
|||||||
ok(this.notifyObj.mainActionClicked, "mainAction was clicked");
|
ok(this.notifyObj.mainActionClicked, "mainAction was clicked");
|
||||||
ok(!this.notifyObj.dismissalCallbackTriggered, "dismissal callback wasn't triggered");
|
ok(!this.notifyObj.dismissalCallbackTriggered, "dismissal callback wasn't triggered");
|
||||||
ok(this.notifyObj.removedCallbackTriggered, "removed callback triggered");
|
ok(this.notifyObj.removedCallbackTriggered, "removed callback triggered");
|
||||||
|
is(this.notifyObj.mainActionSource, "button", "main action should have been triggered by button.");
|
||||||
|
is(this.notifyObj.secondaryActionSource, undefined, "shouldn't have a secondary action source.");
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
{ id: "Test#2",
|
{ id: "Test#2",
|
||||||
@ -44,6 +46,8 @@ var tests = [
|
|||||||
ok(this.notifyObj.secondaryActionClicked, "secondaryAction was clicked");
|
ok(this.notifyObj.secondaryActionClicked, "secondaryAction was clicked");
|
||||||
ok(!this.notifyObj.dismissalCallbackTriggered, "dismissal callback wasn't triggered");
|
ok(!this.notifyObj.dismissalCallbackTriggered, "dismissal callback wasn't triggered");
|
||||||
ok(this.notifyObj.removedCallbackTriggered, "removed callback triggered");
|
ok(this.notifyObj.removedCallbackTriggered, "removed callback triggered");
|
||||||
|
is(this.notifyObj.mainActionSource, undefined, "shouldn't have a main action source.");
|
||||||
|
is(this.notifyObj.secondaryActionSource, "button", "secondary action should have been triggered by button.");
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
{ id: "Test#2b",
|
{ id: "Test#2b",
|
||||||
|
@ -30,6 +30,8 @@ var tests = [
|
|||||||
ok(this.notifyObj.secondaryActionClicked, "secondaryAction was clicked");
|
ok(this.notifyObj.secondaryActionClicked, "secondaryAction was clicked");
|
||||||
ok(!this.notifyObj.dismissalCallbackTriggered, "dismissal callback wasn't triggered");
|
ok(!this.notifyObj.dismissalCallbackTriggered, "dismissal callback wasn't triggered");
|
||||||
ok(this.notifyObj.removedCallbackTriggered, "removed callback triggered");
|
ok(this.notifyObj.removedCallbackTriggered, "removed callback triggered");
|
||||||
|
is(this.notifyObj.mainActionSource, undefined, "shouldn't have a main action source.");
|
||||||
|
is(this.notifyObj.secondaryActionSource, "esc-press", "secondary action should be from ESC key press");
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
// Test that for non-persistent notifications, the escape key dismisses the notification.
|
// Test that for non-persistent notifications, the escape key dismisses the notification.
|
||||||
@ -48,6 +50,8 @@ var tests = [
|
|||||||
ok(!this.notifyObj.secondaryActionClicked, "secondaryAction was not clicked");
|
ok(!this.notifyObj.secondaryActionClicked, "secondaryAction was not clicked");
|
||||||
ok(this.notifyObj.dismissalCallbackTriggered, "dismissal callback triggered");
|
ok(this.notifyObj.dismissalCallbackTriggered, "dismissal callback triggered");
|
||||||
ok(!this.notifyObj.removedCallbackTriggered, "removed callback was not triggered");
|
ok(!this.notifyObj.removedCallbackTriggered, "removed callback was not triggered");
|
||||||
|
is(this.notifyObj.mainActionSource, undefined, "shouldn't have a main action source.");
|
||||||
|
is(this.notifyObj.secondaryActionSource, undefined, "shouldn't have a secondary action source.");
|
||||||
this.notification.remove();
|
this.notification.remove();
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
@ -114,13 +114,19 @@ function BasicNotification(testId) {
|
|||||||
this.mainAction = {
|
this.mainAction = {
|
||||||
label: "Main Action",
|
label: "Main Action",
|
||||||
accessKey: "M",
|
accessKey: "M",
|
||||||
callback: () => this.mainActionClicked = true
|
callback: ({source}) => {
|
||||||
|
this.mainActionClicked = true;
|
||||||
|
this.mainActionSource = source;
|
||||||
|
},
|
||||||
};
|
};
|
||||||
this.secondaryActions = [
|
this.secondaryActions = [
|
||||||
{
|
{
|
||||||
label: "Secondary Action",
|
label: "Secondary Action",
|
||||||
accessKey: "S",
|
accessKey: "S",
|
||||||
callback: () => this.secondaryActionClicked = true
|
callback: ({source}) => {
|
||||||
|
this.secondaryActionClicked = true;
|
||||||
|
this.secondaryActionSource = source;
|
||||||
|
},
|
||||||
}
|
}
|
||||||
];
|
];
|
||||||
this.options = {
|
this.options = {
|
||||||
|
@ -312,10 +312,9 @@ var PermissionPromptPrototype = {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (this.permissionKey) {
|
if (this.permissionKey) {
|
||||||
|
if ((state && state.checkboxChecked && state.source != "esc-press") ||
|
||||||
// Permanently store permission.
|
|
||||||
if ((state && state.checkboxChecked) ||
|
|
||||||
promptAction.scope == SitePermissions.SCOPE_PERSISTENT) {
|
promptAction.scope == SitePermissions.SCOPE_PERSISTENT) {
|
||||||
|
// Permanently store permission.
|
||||||
let scope = SitePermissions.SCOPE_PERSISTENT;
|
let scope = SitePermissions.SCOPE_PERSISTENT;
|
||||||
// Only remember permission for session if in PB mode.
|
// Only remember permission for session if in PB mode.
|
||||||
if (PrivateBrowsingUtils.isBrowserPrivate(this.browser)) {
|
if (PrivateBrowsingUtils.isBrowserPrivate(this.browser)) {
|
||||||
|
@ -81,9 +81,19 @@ add_task(async function testPermissionDeniedDismiss() {
|
|||||||
gBrowser.selectedBrowser.loadURI(testPageURL);
|
gBrowser.selectedBrowser.loadURI(testPageURL);
|
||||||
await waitForMessage(false, gBrowser);
|
await waitForMessage(false, gBrowser);
|
||||||
|
|
||||||
|
// Pressing ESC results in a temporary block permission on the browser object.
|
||||||
|
// So the global permission for the URL should still be unknown, but the browser
|
||||||
|
// should have a block permission with a temporary scope.
|
||||||
is(getPermission(testPageURL, "persistent-storage"),
|
is(getPermission(testPageURL, "persistent-storage"),
|
||||||
Ci.nsIPermissionManager.DENY_ACTION,
|
Ci.nsIPermissionManager.UNKNOWN_ACTION,
|
||||||
"Correct permission set");
|
"Correct permission set");
|
||||||
|
|
||||||
|
let tempBlock = SitePermissions.getAllForBrowser(gBrowser.selectedBrowser)
|
||||||
|
.find(p => p.id == "persistent-storage" &&
|
||||||
|
p.state == SitePermissions.BLOCK &&
|
||||||
|
p.scope == SitePermissions.SCOPE_TEMPORARY);
|
||||||
|
ok(tempBlock, "Should have a temporary block permission on active browser");
|
||||||
|
|
||||||
unregisterAllPopupEventHandlers();
|
unregisterAllPopupEventHandlers();
|
||||||
gBrowser.removeCurrentTab();
|
gBrowser.removeCurrentTab();
|
||||||
removePermission(testPageURL, "persistent-storage");
|
removePermission(testPageURL, "persistent-storage");
|
||||||
|
@ -253,7 +253,7 @@ function PopupNotifications(tabbrowser, panel,
|
|||||||
// Ignore focused elements inside the notification.
|
// Ignore focused elements inside the notification.
|
||||||
getNotificationFromElement(focusedElement) == notification ||
|
getNotificationFromElement(focusedElement) == notification ||
|
||||||
notification.contains(focusedElement)) {
|
notification.contains(focusedElement)) {
|
||||||
this._onButtonEvent(aEvent, "secondarybuttoncommand", notification);
|
this._onButtonEvent(aEvent, "secondarybuttoncommand", "esc-press", notification);
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
@ -345,6 +345,11 @@ PopupNotifications.prototype = {
|
|||||||
* - callback (function): a callback to be invoked when the button is
|
* - callback (function): a callback to be invoked when the button is
|
||||||
* pressed, is passed an object that contains the following fields:
|
* pressed, is passed an object that contains the following fields:
|
||||||
* - checkboxChecked: (boolean) If the optional checkbox is checked.
|
* - checkboxChecked: (boolean) If the optional checkbox is checked.
|
||||||
|
* - source: (string): the source of the action that initiated the
|
||||||
|
* callback, either:
|
||||||
|
* - "button" if popup buttons were directly activated, or
|
||||||
|
* - "esc-press" if the user pressed the escape key, or
|
||||||
|
* - "menucommand" if a menu was activated.
|
||||||
* - [optional] dismiss (boolean): If this is true, the notification
|
* - [optional] dismiss (boolean): If this is true, the notification
|
||||||
* will be dismissed instead of removed after running the callback.
|
* will be dismissed instead of removed after running the callback.
|
||||||
* - [optional] disableHighlight (boolean): If this is true, the button
|
* - [optional] disableHighlight (boolean): If this is true, the button
|
||||||
@ -809,7 +814,7 @@ PopupNotifications.prototype = {
|
|||||||
popupnotification.setAttribute("popupid", n.id);
|
popupnotification.setAttribute("popupid", n.id);
|
||||||
popupnotification.setAttribute("oncommand", "PopupNotifications._onCommand(event);");
|
popupnotification.setAttribute("oncommand", "PopupNotifications._onCommand(event);");
|
||||||
if (Services.prefs.getBoolPref("privacy.permissionPrompts.showCloseButton")) {
|
if (Services.prefs.getBoolPref("privacy.permissionPrompts.showCloseButton")) {
|
||||||
popupnotification.setAttribute("closebuttoncommand", "PopupNotifications._onButtonEvent(event, 'secondarybuttoncommand');");
|
popupnotification.setAttribute("closebuttoncommand", "PopupNotifications._onButtonEvent(event, 'secondarybuttoncommand', 'esc-press');");
|
||||||
} else {
|
} else {
|
||||||
popupnotification.setAttribute("closebuttoncommand", `PopupNotifications._dismiss(event, ${TELEMETRY_STAT_DISMISSAL_CLOSE_BUTTON});`);
|
popupnotification.setAttribute("closebuttoncommand", `PopupNotifications._dismiss(event, ${TELEMETRY_STAT_DISMISSAL_CLOSE_BUTTON});`);
|
||||||
}
|
}
|
||||||
@ -1477,7 +1482,7 @@ PopupNotifications.prototype = {
|
|||||||
this._setNotificationUIState(notificationEl);
|
this._setNotificationUIState(notificationEl);
|
||||||
},
|
},
|
||||||
|
|
||||||
_onButtonEvent(event, type, notificationEl = null) {
|
_onButtonEvent(event, type, source = "button", notificationEl = null) {
|
||||||
if (!notificationEl) {
|
if (!notificationEl) {
|
||||||
notificationEl = getNotificationFromElement(event.originalTarget);
|
notificationEl = getNotificationFromElement(event.originalTarget);
|
||||||
}
|
}
|
||||||
@ -1541,7 +1546,8 @@ PopupNotifications.prototype = {
|
|||||||
if (action) {
|
if (action) {
|
||||||
try {
|
try {
|
||||||
action.callback.call(undefined, {
|
action.callback.call(undefined, {
|
||||||
checkboxChecked: notificationEl.checkbox.checked
|
checkboxChecked: notificationEl.checkbox.checked,
|
||||||
|
source,
|
||||||
});
|
});
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
Cu.reportError(error);
|
Cu.reportError(error);
|
||||||
@ -1569,7 +1575,8 @@ PopupNotifications.prototype = {
|
|||||||
|
|
||||||
try {
|
try {
|
||||||
target.action.callback.call(undefined, {
|
target.action.callback.call(undefined, {
|
||||||
checkboxChecked: notificationEl.checkbox.checked
|
checkboxChecked: notificationEl.checkbox.checked,
|
||||||
|
source: "menucommand",
|
||||||
});
|
});
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
Cu.reportError(error);
|
Cu.reportError(error);
|
||||||
|
Loading…
Reference in New Issue
Block a user