Bug 1618696 - Prevent overlapping show/hide of the ConfirmationHint. r=MattN

* Track and clear a timerID for the ConfirmationHint to avoid callbacks from one show() call interfering with a subsequent call.
* Tighten up waiting for and verifying the confirmation hint in browser_doorhanger_generated_password.js
* Pass in the correct browser when retrieving the anchorNode for the confirmation hint.

Differential Revision: https://phabricator.services.mozilla.com/D67398

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Sam Foster 2020-03-18 23:05:57 +00:00
Родитель 97296cac1e
Коммит 08d2b6aa6d
4 изменённых файлов: 127 добавлений и 69 удалений

Просмотреть файл

@ -9081,6 +9081,8 @@ TabModalPromptBox.prototype = {
};
var ConfirmationHint = {
_timerID: null,
/**
* Shows a transient, non-interactive confirmation hint anchored to an
* element, usually used in response to a user action to reaffirm that it was
@ -9103,6 +9105,8 @@ var ConfirmationHint = {
*
*/
show(anchor, messageId, options = {}) {
this._reset();
this._message.textContent = gBrowserBundle.GetStringFromName(
`confirmationHint.${messageId}.label`
);
@ -9130,8 +9134,7 @@ var ConfirmationHint = {
"popupshown",
() => {
this._animationBox.setAttribute("animate", "true");
setTimeout(() => {
this._timerID = setTimeout(() => {
this._panel.hidePopup(true);
}, DURATION + 120);
},
@ -9141,8 +9144,8 @@ var ConfirmationHint = {
this._panel.addEventListener(
"popuphidden",
() => {
this._panel.removeAttribute("hidearrow");
this._animationBox.removeAttribute("animate");
// reset the timerId in case our timeout wasn't the cause of the popup being hidden
this._reset();
},
{ once: true }
);
@ -9154,6 +9157,15 @@ var ConfirmationHint = {
});
},
_reset() {
if (this._timerID) {
clearTimeout(this._timerID);
this._timerID = null;
}
this._panel.removeAttribute("hidearrow");
this._animationBox.removeAttribute("animate");
},
get _panel() {
delete this._panel;
return (this._panel = document.getElementById("confirmation-hint"));

Просмотреть файл

@ -439,7 +439,7 @@ class LoginManagerPrompter {
showOptions.dismissed && showOptions.extraAttr == "attention"
? ATTENTION_NOTIFICATION_TIMEOUT_MS
: NOTIFICATION_TIMEOUT_MS;
PopupNotifications.show(
let notification = PopupNotifications.show(
browser,
notificationID,
promptMsg,
@ -540,8 +540,8 @@ class LoginManagerPrompter {
);
if (notifySaved) {
let notification = PopupNotifications.getNotification(notificationID);
let anchor = notification.anchorElement;
log.debug("Showing the ConfirmationHint");
anchor.ownerGlobal.ConfirmationHint.show(anchor, "passwordSaved");
}
}

Просмотреть файл

@ -13,23 +13,27 @@ const FORM_PAGE_PATH =
const passwordInputSelector = "#form-basic-password";
const usernameInputSelector = "#form-basic-username";
async function setup_withOneLogin(username = "username", password = "pass1") {
// Reset to a single, known login
async function task_setup() {
Services.logins.removeAllLogins();
LoginTestUtils.resetGeneratedPasswordsCache();
await cleanupPasswordNotifications();
}
async function setup_withOneLogin(username = "username", password = "pass1") {
// Reset to a single, known login
await task_setup();
let login = await LoginTestUtils.addLogin({ username, password });
return login;
}
async function setup_withNoLogins() {
// Reset to a single, known login
Services.logins.removeAllLogins();
await task_setup();
is(
Services.logins.getAllLogins().length,
0,
"0 logins at the start of the test"
);
LoginTestUtils.resetGeneratedPasswordsCache();
}
async function fillGeneratedPasswordFromACPopup(
@ -93,17 +97,33 @@ async function verifyGeneratedPasswordWasFilled(
);
}
async function verifyConfirmationHint(hintElem) {
info("verifyConfirmationHint");
info("verifyConfirmationHint, hintPromiseShown resolved");
is(
hintElem.anchorNode.id,
"password-notification-icon",
"Hint should be anchored on the password notification icon"
);
info("verifyConfirmationHint, assertion ok, wait for poopuphidden");
await BrowserTestUtils.waitForEvent(hintElem, "popuphidden");
info("verifyConfirmationHint, /popuphidden");
async function verifyConfirmationHint(browser, forceClose) {
let hintElem = browser.ownerDocument.getElementById("confirmation-hint");
await BrowserTestUtils.waitForPopupEvent(hintElem, "shown");
try {
is(hintElem.state, "open", "hint popup is open");
ok(
BrowserTestUtils.is_visible(hintElem.anchorNode),
"hint anchorNode is visible"
);
is(
hintElem.anchorNode.id,
"password-notification-icon",
"Hint should be anchored on the password notification icon"
);
info("verifyConfirmationHint, hint is shown and has its anchorNode");
if (forceClose) {
await closePopup(hintElem);
} else {
info("verifyConfirmationHint, assertion ok, wait for poopuphidden");
await BrowserTestUtils.waitForPopupEvent(hintElem, "hidden");
info("verifyConfirmationHint, hintElem poup is hidden");
}
} catch (ex) {
ok(false, "Confirmation hint not shown: " + ex.message);
} finally {
info("verifyConfirmationHint promise finalized");
}
}
async function openFormInNewTab(url, formValues, taskFn) {
@ -191,6 +211,9 @@ async function openFormInNewTab(url, formValues, taskFn) {
}
await taskFn(browser);
await closePopup(
browser.ownerDocument.getElementById("confirmation-hint")
);
}
);
}
@ -281,17 +304,20 @@ add_task(async function autocomplete_generated_password_auto_saved() {
"passwordmgr-storage-changed",
(_, data) => data == "addLogin"
);
let confirmationHint = document.getElementById("confirmation-hint");
let hintPromiseShown = BrowserTestUtils.waitForEvent(
confirmationHint,
"popupshown"
// Let the hint hide itself this first time
let forceClosePopup = false;
let hintShownAndVerified = verifyConfirmationHint(
browser,
forceClosePopup
);
await fillGeneratedPasswordFromACPopup(browser, passwordInputSelector);
let [{ username, password }] = await storageChangedPromise;
await verifyGeneratedPasswordWasFilled(browser, passwordInputSelector);
// Make sure confirmation hint was shown
await hintPromiseShown;
await verifyConfirmationHint(confirmationHint);
info("waiting for verifyConfirmationHint");
await hintShownAndVerified;
// Check properties of the newly auto-saved login
is(username, "", "Saved login should have no username");
@ -426,18 +452,22 @@ add_task(async function autocomplete_generated_password_saved_username() {
"passwordmgr-storage-changed",
(_, data) => data == "addLogin"
);
let confirmationHint = document.getElementById("confirmation-hint");
let hintPromiseShown = BrowserTestUtils.waitForEvent(
confirmationHint,
"popupshown"
// We don't need to wait to confirm the hint hides itelf every time
let forceClosePopup = true;
let hintShownAndVerified = verifyConfirmationHint(
browser,
forceClosePopup
);
await fillGeneratedPasswordFromACPopup(browser, passwordInputSelector);
// Make sure confirmation hint was shown
info("waiting for verifyConfirmationHint");
await hintShownAndVerified;
info("waiting for addLogin");
await storageChangedPromise;
await verifyGeneratedPasswordWasFilled(browser, passwordInputSelector);
// Make sure confirmation hint was shown
await hintPromiseShown;
await verifyConfirmationHint(confirmationHint);
// Check properties of the newly auto-saved login
let [user1LoginSnapshot, autoSavedLogin] = verifyLogins([
@ -769,11 +799,6 @@ add_task(async function contextmenu_fill_generated_password_and_set_username() {
"passwordmgr-storage-changed",
(_, data) => data == "addLogin"
);
let confirmationHint = document.getElementById("confirmation-hint");
let hintPromiseShown = BrowserTestUtils.waitForEvent(
confirmationHint,
"popupshown"
);
await SpecialPowers.spawn(
browser,
[[passwordInputSelector, usernameInputSelector]],
@ -785,16 +810,24 @@ add_task(async function contextmenu_fill_generated_password_and_set_username() {
);
}
);
// Let the hint hide itself this first time
let forceClosePopup = false;
let hintShownAndVerified = verifyConfirmationHint(
browser,
forceClosePopup
);
info("waiting to fill generated password using context menu");
await doFillGeneratedPasswordContextMenuItem(
browser,
passwordInputSelector
);
info("waiting for verifyConfirmationHint");
await hintShownAndVerified;
info("waiting for dismissed password-change notification");
await waitForDoorhanger(browser, "password-change");
// Make sure confirmation hint was shown
await hintPromiseShown;
await verifyConfirmationHint(confirmationHint);
info("waiting for addLogin");
await storageChangedPromise;
@ -878,10 +911,12 @@ add_task(async function contextmenu_password_change_form_without_username() {
"passwordmgr-storage-changed",
(_, data) => data == "addLogin"
);
let confirmationHint = document.getElementById("confirmation-hint");
let hintPromiseShown = BrowserTestUtils.waitForEvent(
confirmationHint,
"popupshown"
// We don't need to wait to confirm the hint hides itelf every time
let forceClosePopup = true;
let hintShownAndVerified = verifyConfirmationHint(
browser,
forceClosePopup
);
// Make the 2nd field use a generated password
@ -893,9 +928,10 @@ add_task(async function contextmenu_password_change_form_without_username() {
info("waiting for dismissed password-change notification");
await waitForDoorhanger(browser, "password-change");
// Make sure confirmation hint was shown
await hintPromiseShown;
await verifyConfirmationHint(confirmationHint);
info("waiting for verifyConfirmationHint");
await hintShownAndVerified;
info("waiting for addLogin");
await storageChangedPromise;
@ -982,10 +1018,11 @@ add_task(
"passwordmgr-storage-changed",
(_, data) => data == "addLogin"
);
let confirmationHint = document.getElementById("confirmation-hint");
let hintPromiseShown = BrowserTestUtils.waitForEvent(
confirmationHint,
"popupshown"
// We don't need to wait to confirm the hint hides itelf every time
let forceClosePopup = true;
let hintShownAndVerified = verifyConfirmationHint(
browser,
forceClosePopup
);
info("waiting to fill generated password using context menu");
@ -996,9 +1033,10 @@ add_task(
info("waiting for dismissed password-change notification");
await waitForDoorhanger(browser, "password-change");
// Make sure confirmation hint was shown
await hintPromiseShown;
await verifyConfirmationHint(confirmationHint);
info("waiting for verifyConfirmationHint");
await hintShownAndVerified;
info("waiting for addLogin");
await storageChangedPromise;
@ -1157,10 +1195,11 @@ add_task(async function autosaved_login_updated_to_existing_login_onsubmit() {
"passwordmgr-storage-changed",
(_, data) => data == "addLogin"
);
let confirmationHint = document.getElementById("confirmation-hint");
let hintPromiseShown = BrowserTestUtils.waitForEvent(
confirmationHint,
"popupshown"
// We don't need to wait to confirm the hint hides itelf every time
let forceClosePopup = true;
let hintShownAndVerified = verifyConfirmationHint(
browser,
forceClosePopup
);
info("waiting to fill generated password using context menu");
@ -1171,9 +1210,10 @@ add_task(async function autosaved_login_updated_to_existing_login_onsubmit() {
info("waiting for dismissed password-change notification");
await waitForDoorhanger(browser, "password-change");
// Make sure confirmation hint was shown
await hintPromiseShown;
await verifyConfirmationHint(confirmationHint);
info("waiting for verifyConfirmationHint");
await hintShownAndVerified;
info("waiting for addLogin");
await storageChangedPromise;
@ -1349,10 +1389,11 @@ add_task(async function form_change_from_autosaved_login_to_existing_login() {
"passwordmgr-storage-changed",
(_, data) => data == "addLogin"
);
let confirmationHint = document.getElementById("confirmation-hint");
let hintPromiseShown = BrowserTestUtils.waitForEvent(
confirmationHint,
"popupshown"
// We don't need to wait to confirm the hint hides itelf every time
let forceClosePopup = true;
let hintShownAndVerified = verifyConfirmationHint(
browser,
forceClosePopup
);
info("Filling generated password from AC menu");
@ -1360,9 +1401,10 @@ add_task(async function form_change_from_autosaved_login_to_existing_login() {
info("waiting for dismissed password-change notification");
await waitForDoorhanger(browser, "password-change");
// Make sure confirmation hint was shown
await hintPromiseShown;
await verifyConfirmationHint(confirmationHint);
info("waiting for verifyConfirmationHint");
await hintShownAndVerified;
info("waiting for addLogin");
await storageChangedPromise;
@ -1427,8 +1469,8 @@ add_task(async function form_change_from_autosaved_login_to_existing_login() {
"password-change"
);
let hintDidShow = false;
hintPromiseShown = BrowserTestUtils.waitForPopupEvent(
confirmationHint,
let hintPromiseShown = BrowserTestUtils.waitForPopupEvent(
document.getElementById("confirmation-hint"),
"shown"
);
hintPromiseShown.then(() => (hintDidShow = true));

Просмотреть файл

@ -330,7 +330,11 @@ async function waitForDoorhanger(browser, type) {
await TestUtils.waitForCondition(() => {
notif = PopupNotifications.getNotification("password", browser);
if (notif && type !== "any") {
return notif.options.passwordNotificationType == type;
return (
notif.options.passwordNotificationType == type &&
notif.anchorElement &&
BrowserTestUtils.is_visible(notif.anchorElement)
);
}
return notif;
}, `Waiting for a ${type} notification`);