Bug 1486954 - Part II, Remove OSKeyStore.isEnabled. r=MattN

Given that the new store is always considered enabled, the not-enabled code
is now dead code. This patch removes them.

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

--HG--
extra : rebase_source : d2940f77e26ba9f5c808eada5083df293d314214
extra : histedit_source : 29741eb6014489fcd179e90ee18ff876227baaf5
This commit is contained in:
Matthew Noorenberghe 2018-10-22 22:57:28 -07:00
parent b2023e958b
commit 001567e898
10 changed files with 8 additions and 74 deletions

View File

@ -317,9 +317,9 @@ FormAutofillParent.prototype = {
return;
}
let isCCAndMPEnabled = collectionName == CREDITCARDS_COLLECTION_NAME && OSKeyStore.isEnabled;
// We don't filter "cc-number" when OSKeyStore is set.
if (isCCAndMPEnabled && info.fieldName == "cc-number") {
let isCC = collectionName == CREDITCARDS_COLLECTION_NAME;
// We don't filter "cc-number"
if (isCC && info.fieldName == "cc-number") {
recordsInCollection = recordsInCollection.filter(record => !!record["cc-number"]);
target.sendAsyncMessage("FormAutofill:Records", recordsInCollection);
return;
@ -334,17 +334,6 @@ FormAutofillParent.prototype = {
continue;
}
// Cache the decrypted "cc-number" in each record for content to preview
// when OSKeyStore isn't set.
if (!isCCAndMPEnabled && record["cc-number-encrypted"]) {
record["cc-number-decrypted"] = await OSKeyStore.decrypt(record["cc-number-encrypted"]);
}
// Filter "cc-number" based on the decrypted one.
if (info.fieldName == "cc-number") {
fieldValue = record["cc-number-decrypted"];
}
if (collectionName == ADDRESSES_COLLECTION_NAME && record.country
&& !FormAutofill.supportedCountries.includes(record.country)) {
// Address autofill isn't supported for the record's country so we don't

View File

@ -1692,13 +1692,10 @@ class CreditCards extends AutofillRecords {
return !creditCard[field];
}
if (field == "cc-number" && creditCard[field]) {
if (OSKeyStore.isEnabled) {
// Compare the masked numbers instead when decryption requires a password
// because we don't want to leak the credit card number.
return CreditCard.getLongMaskedNumber(clonedTargetCreditCard[field]) == creditCard[field];
}
return (clonedTargetCreditCard[field] == await OSKeyStore.decrypt(creditCard["cc-number-encrypted"]));
}
return clonedTargetCreditCard[field] == creditCard[field];
})).then(fieldResults => fieldResults.every(result => result));
if (isDuplicate) {

View File

@ -17,7 +17,7 @@ const EDIT_ADDRESS_KEYWORDS = [
"givenName", "additionalName", "familyName", "organization2", "streetAddress",
"state", "province", "city", "country", "zip", "postalCode", "email", "tel",
];
const MANAGE_CREDITCARDS_KEYWORDS = ["manageCreditCardsTitle", "addNewCreditCardTitle", "showCreditCardsBtnLabel"];
const MANAGE_CREDITCARDS_KEYWORDS = ["manageCreditCardsTitle", "addNewCreditCardTitle"];
const EDIT_CREDITCARD_KEYWORDS = ["cardNumber", "nameOnCard", "cardExpiresMonth", "cardExpiresYear", "cardNetwork"];
const FIELD_STATES = {
NORMAL: "NORMAL",

View File

@ -38,17 +38,6 @@ var OSKeyStore = {
_pendingUnlockPromise: null,
/**
* @returns {boolean} Always considered enabled because OS store is always
* protected via OS user login password.
* TODO: Figure out if the affacted behaviors
* (e.g. like bug 1486954 or confirming payment transaction)
* is correct or not.
*/
get isEnabled() {
return true;
},
/**
* @returns {boolean} True if logged in (i.e. decrypt(reauth = false) will
* not retrigger a dialog) and false if not.

View File

@ -20,7 +20,6 @@
</fieldset>
<div id="controls-container">
<button id="remove" disabled="disabled" data-localization="removeBtnLabel"/>
<button id="show-hide-credit-cards" data-localization="showCreditCardsBtnLabel"/>
<!-- Wrapper is used to properly compute the search tooltip position -->
<div>
<button id="add" data-localization="addBtnLabel"/>
@ -34,7 +33,6 @@
records: document.getElementById("credit-cards"),
controlsContainer: document.getElementById("controls-container"),
remove: document.getElementById("remove"),
showHideCreditCards: document.getElementById("show-hide-credit-cards"),
add: document.getElementById("add"),
edit: document.getElementById("edit"),
});

View File

@ -317,11 +317,7 @@ class ManageCreditCards extends ManageRecords {
elements.add.setAttribute("searchkeywords", FormAutofillUtils.EDIT_CREDITCARD_KEYWORDS
.map(key => FormAutofillUtils.stringBundle.GetStringFromName(key))
.join("\n"));
this._hasOSKeyStore = OSKeyStore.isEnabled;
this._isDecrypted = false;
if (this._hasOSKeyStore) {
elements.showHideCreditCards.setAttribute("hidden", true);
}
}
/**
@ -331,7 +327,7 @@ class ManageCreditCards extends ManageRecords {
*/
async openEditDialog(creditCard) {
// Ask for reauth if user is trying to edit an existing credit card.
if (!creditCard || !this._hasOSKeyStore || await OSKeyStore.ensureLoggedIn(true)) {
if (!creditCard || await OSKeyStore.ensureLoggedIn(true)) {
let decryptedCCNumObj = {};
if (creditCard) {
decryptedCCNumObj["cc-number"] = await OSKeyStore.decrypt(creditCard["cc-number-encrypted"]);
@ -362,12 +358,6 @@ class ManageCreditCards extends ManageRecords {
return cardObj.getLabel({showNumbers: showCreditCards});
}
async toggleShowHideCards(options) {
this._isDecrypted = !this._isDecrypted;
this.updateShowHideButtonState();
await this.updateLabels(options, this._isDecrypted);
}
async updateLabels(options, isDecrypted) {
for (let option of options) {
option.text = await this.getLabel(option.record, isDecrypted);
@ -395,25 +385,10 @@ class ManageCreditCards extends ManageRecords {
}
updateButtonsStates(selectedCount) {
this.updateShowHideButtonState();
super.updateButtonsStates(selectedCount);
}
updateShowHideButtonState() {
if (this._elements.records.length) {
this._elements.showHideCreditCards.removeAttribute("disabled");
} else {
this._elements.showHideCreditCards.setAttribute("disabled", true);
}
this._elements.showHideCreditCards.textContent =
this._isDecrypted ? FormAutofillUtils.stringBundle.GetStringFromName("hideCreditCardsBtnLabel") :
FormAutofillUtils.stringBundle.GetStringFromName("showCreditCardsBtnLabel");
}
handleClick(event) {
if (event.target == this._elements.showHideCreditCards) {
this.toggleShowHideCards(this._elements.records.options);
}
super.handleClick(event);
}
}

View File

@ -104,8 +104,6 @@ manageCreditCardsTitle = Saved Credit Cards
# in browser preferences.
addressesListHeader = Addresses
creditCardsListHeader = Credit Cards
showCreditCardsBtnLabel = Show Credit Cards
hideCreditCardsBtnLabel = Hide Credit Cards
removeBtnLabel = Remove
addBtnLabel = Add…
editBtnLabel = Edit…

View File

@ -3,7 +3,6 @@
const TEST_SELECTORS = {
selRecords: "#credit-cards",
btnRemove: "#remove",
btnShowHideCreditCards: "#show-hide-credit-cards",
btnAdd: "#add",
btnEdit: "#edit",
};
@ -15,13 +14,11 @@ add_task(async function test_manageCreditCardsInitialState() {
await ContentTask.spawn(browser, TEST_SELECTORS, (args) => {
let selRecords = content.document.querySelector(args.selRecords);
let btnRemove = content.document.querySelector(args.btnRemove);
let btnShowHideCreditCards = content.document.querySelector(args.btnShowHideCreditCards);
let btnAdd = content.document.querySelector(args.btnAdd);
let btnEdit = content.document.querySelector(args.btnEdit);
is(selRecords.length, 0, "No credit card");
is(btnRemove.disabled, true, "Remove button disabled");
is(btnShowHideCreditCards.disabled, true, "Show Credit Cards button disabled");
is(btnAdd.disabled, false, "Add button enabled");
is(btnEdit.disabled, true, "Edit button disabled");
});
@ -155,12 +152,9 @@ add_task(async function test_hasEditLoginPrompt() {
let selRecords = win.document.querySelector(TEST_SELECTORS.selRecords);
let btnRemove = win.document.querySelector(TEST_SELECTORS.btnRemove);
let btnShowHideCreditCards = win.document.querySelector(TEST_SELECTORS.btnShowHideCreditCards);
let btnAdd = win.document.querySelector(TEST_SELECTORS.btnAdd);
// let btnEdit = win.document.querySelector(TEST_SELECTORS.btnEdit);
is(btnShowHideCreditCards.hidden, true, "Show credit cards button is hidden");
EventUtils.synthesizeMouseAtCenter(selRecords.children[0], {}, win);
// Login dialog should show when trying to edit a credit card record.

View File

@ -70,8 +70,6 @@ const testText = "test string";
let cipherText;
add_task(async function test_encrypt_decrypt() {
Assert.equal(OSKeyStore.isEnabled, true);
Assert.equal(await OSKeyStore.ensureLoggedIn(), true, "Started logged in.");
cipherText = await OSKeyStore.encrypt(testText);

View File

@ -12,12 +12,9 @@ let gFakeLoggedIn = true;
add_task(function setup() {
OSKeyStoreTestUtils.setup();
oldGetters.isEnabled = Object.getOwnPropertyDescriptor(OSKeyStore, "isEnabled").get;
oldGetters.isLoggedIn = Object.getOwnPropertyDescriptor(OSKeyStore, "isLoggedIn").get;
OSKeyStore.__defineGetter__("isEnabled", () => true);
OSKeyStore.__defineGetter__("isLoggedIn", () => gFakeLoggedIn);
registerCleanupFunction(async () => {
OSKeyStore.__defineGetter__("isEnabled", oldGetters.isEnabled);
OSKeyStore.__defineGetter__("isLoggedIn", oldGetters.isLoggedIn);
await OSKeyStoreTestUtils.cleanup();
@ -31,7 +28,6 @@ add_task(function setup() {
});
add_task(async function test_getLabel_withOSKeyStore() {
ok(OSKeyStore.isEnabled, "Confirm that OSKeyStore is faked and thinks it is enabled");
ok(OSKeyStore.isLoggedIn, "Confirm that OSKeyStore is faked and thinks it is logged in");
const ccNumber = "4111111111111111";