1
0
mirror of https://github.com/kremalicious/metamask-extension.git synced 2024-10-22 03:12:42 +02:00

Fix autolock field to accept decimals in Firefox (#19653)

The autolock field on the Settings screen — the field that allows users
to set the duration that MetaMask will wait for until automatically
locking — does not always accept decimal numbers. This breaks the e2e
test for this feature as it attempts to set this field to "0.1".

More specifically, the React component responsible for this field passes
whatever the user inputs through the `Number` function immediately and
then uses this to repopulate the input. Therefore, if the user enters
"3" followed by a ".", `Number("3.")` will be called. This evaluates to
the number 3, and "3" becomes the new value of the field. As a result,
the "." can never be typed.

Curiously, this behavior only happens in Firefox; Chrome seems to
keep the "." in the input field when it's typed. This happens because
`onChange` event doesn't seem to get fired until a number is typed
*after* the ".". This may be due to underlying differences in the DOM
between Chrome and Firefox.

Regardless, always passing the input through `Number` creates other odd
behavior, such as the fact that the input can never be cleared (because
`Number("")` evaluates to 0).

This commit solves these problems by saving the "raw" version of the
user's input as well as the normalized version. The raw version is
always used to populate the input, whereas the normalized version is
saved in state.
This commit is contained in:
Elliot Winkler 2023-06-22 09:29:24 -07:00 committed by GitHub
parent 16dad66da9
commit bd12ea733a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
28 changed files with 59 additions and 81 deletions

View File

@ -1863,9 +1863,6 @@
"lock": { "lock": {
"message": "Ausloggen" "message": "Ausloggen"
}, },
"lockTimeTooGreat": {
"message": "Sperrzeit ist zu groß"
},
"logo": { "logo": {
"message": "$1-Logo", "message": "$1-Logo",
"description": "$1 is the name of the ticker" "description": "$1 is the name of the ticker"

View File

@ -1863,9 +1863,6 @@
"lock": { "lock": {
"message": "Αποσύνδεση" "message": "Αποσύνδεση"
}, },
"lockTimeTooGreat": {
"message": "Ο χρόνος κλειδώματος είναι πολύ μεγάλος"
},
"logo": { "logo": {
"message": "Λογότυπο $1", "message": "Λογότυπο $1",
"description": "$1 is the name of the ticker" "description": "$1 is the name of the ticker"

View File

@ -2171,8 +2171,8 @@
"lockMetaMask": { "lockMetaMask": {
"message": "Lock MetaMask" "message": "Lock MetaMask"
}, },
"lockTimeTooGreat": { "lockTimeInvalid": {
"message": "Lock time is too great" "message": "Lock time must be a number between 0 and 10080"
}, },
"logo": { "logo": {
"message": "$1 logo", "message": "$1 logo",

View File

@ -1863,9 +1863,6 @@
"lock": { "lock": {
"message": "Bloquear" "message": "Bloquear"
}, },
"lockTimeTooGreat": {
"message": "El tiempo de bloqueo es demasiado largo"
},
"logo": { "logo": {
"message": "Logo de $1", "message": "Logo de $1",
"description": "$1 is the name of the ticker" "description": "$1 is the name of the ticker"

View File

@ -1269,9 +1269,6 @@
"lock": { "lock": {
"message": "Bloquear" "message": "Bloquear"
}, },
"lockTimeTooGreat": {
"message": "El tiempo de bloqueo es demasiado largo"
},
"low": { "low": {
"message": "Baja" "message": "Baja"
}, },

View File

@ -1863,9 +1863,6 @@
"lock": { "lock": {
"message": "Déconnexion" "message": "Déconnexion"
}, },
"lockTimeTooGreat": {
"message": "Le temps de verrouillage est trop important"
},
"logo": { "logo": {
"message": "Logo $1", "message": "Logo $1",
"description": "$1 is the name of the ticker" "description": "$1 is the name of the ticker"

View File

@ -1863,9 +1863,6 @@
"lock": { "lock": {
"message": "लॉक" "message": "लॉक"
}, },
"lockTimeTooGreat": {
"message": "लॉक समय बहुत अधिक है"
},
"logo": { "logo": {
"message": "$1 लोगो", "message": "$1 लोगो",
"description": "$1 is the name of the ticker" "description": "$1 is the name of the ticker"

View File

@ -1863,9 +1863,6 @@
"lock": { "lock": {
"message": "Kunci" "message": "Kunci"
}, },
"lockTimeTooGreat": {
"message": "Lock time terlalu besar"
},
"logo": { "logo": {
"message": "Logo $1", "message": "Logo $1",
"description": "$1 is the name of the ticker" "description": "$1 is the name of the ticker"

View File

@ -1117,9 +1117,6 @@
"lock": { "lock": {
"message": "Disconnetti" "message": "Disconnetti"
}, },
"lockTimeTooGreat": {
"message": "Tempo di inattività troppo lungo"
},
"mainnet": { "mainnet": {
"message": "Rete Ethereum Principale" "message": "Rete Ethereum Principale"
}, },

View File

@ -1863,9 +1863,6 @@
"lock": { "lock": {
"message": "ロック" "message": "ロック"
}, },
"lockTimeTooGreat": {
"message": "ロック時間が長すぎます"
},
"logo": { "logo": {
"message": "$1 ロゴ", "message": "$1 ロゴ",
"description": "$1 is the name of the ticker" "description": "$1 is the name of the ticker"

View File

@ -1863,9 +1863,6 @@
"lock": { "lock": {
"message": "잠금" "message": "잠금"
}, },
"lockTimeTooGreat": {
"message": "잠금 시간이 너무 깁니다"
},
"logo": { "logo": {
"message": "$1 로고", "message": "$1 로고",
"description": "$1 is the name of the ticker" "description": "$1 is the name of the ticker"

View File

@ -816,9 +816,6 @@
"lock": { "lock": {
"message": "I-lock" "message": "I-lock"
}, },
"lockTimeTooGreat": {
"message": "Masyadong matagal ang oras ng pag-lock"
},
"mainnet": { "mainnet": {
"message": "Ethereum Mainnet" "message": "Ethereum Mainnet"
}, },

View File

@ -1863,9 +1863,6 @@
"lock": { "lock": {
"message": "Sair" "message": "Sair"
}, },
"lockTimeTooGreat": {
"message": "O tempo de bloqueio é longo demais"
},
"logo": { "logo": {
"message": "Logotipo do $1", "message": "Logotipo do $1",
"description": "$1 is the name of the ticker" "description": "$1 is the name of the ticker"

View File

@ -1269,9 +1269,6 @@
"lock": { "lock": {
"message": "Bloquear" "message": "Bloquear"
}, },
"lockTimeTooGreat": {
"message": "O tempo de bloqueio é longo demais"
},
"low": { "low": {
"message": "Baixa" "message": "Baixa"
}, },

View File

@ -1863,9 +1863,6 @@
"lock": { "lock": {
"message": "Заблокировать" "message": "Заблокировать"
}, },
"lockTimeTooGreat": {
"message": "Время блокировки слишком велико"
},
"logo": { "logo": {
"message": "логотип $1", "message": "логотип $1",
"description": "$1 is the name of the ticker" "description": "$1 is the name of the ticker"

View File

@ -1863,9 +1863,6 @@
"lock": { "lock": {
"message": "I-lock" "message": "I-lock"
}, },
"lockTimeTooGreat": {
"message": "Masyadong matagal ang oras ng pag-lock"
},
"logo": { "logo": {
"message": "$1 logo", "message": "$1 logo",
"description": "$1 is the name of the ticker" "description": "$1 is the name of the ticker"

View File

@ -1863,9 +1863,6 @@
"lock": { "lock": {
"message": "Kilitle" "message": "Kilitle"
}, },
"lockTimeTooGreat": {
"message": "Kilitleme süresi çok fazla"
},
"logo": { "logo": {
"message": "$1 logosu", "message": "$1 logosu",
"description": "$1 is the name of the ticker" "description": "$1 is the name of the ticker"

View File

@ -1863,9 +1863,6 @@
"lock": { "lock": {
"message": "Khóa" "message": "Khóa"
}, },
"lockTimeTooGreat": {
"message": "Thời gian khóa quá lớn"
},
"logo": { "logo": {
"message": "Logo $1", "message": "Logo $1",
"description": "$1 is the name of the ticker" "description": "$1 is the name of the ticker"

View File

@ -1863,9 +1863,6 @@
"lock": { "lock": {
"message": "注销" "message": "注销"
}, },
"lockTimeTooGreat": {
"message": "锁定时间过长"
},
"logo": { "logo": {
"message": "$1标志", "message": "$1标志",
"description": "$1 is the name of the ticker" "description": "$1 is the name of the ticker"

View File

@ -821,9 +821,6 @@
"lock": { "lock": {
"message": "鎖定" "message": "鎖定"
}, },
"lockTimeTooGreat": {
"message": "鎖定時間過長"
},
"mainnet": { "mainnet": {
"message": "以太坊 主網路" "message": "以太坊 主網路"
}, },

View File

@ -13,6 +13,7 @@ import {
POLLING_TOKEN_ENVIRONMENT_TYPES, POLLING_TOKEN_ENVIRONMENT_TYPES,
ORIGIN_METAMASK, ORIGIN_METAMASK,
} from '../../../shared/constants/app'; } from '../../../shared/constants/app';
import { DEFAULT_AUTO_LOCK_TIME_LIMIT } from '../../../shared/constants/preferences';
export default class AppStateController extends EventEmitter { export default class AppStateController extends EventEmitter {
/** /**
@ -32,7 +33,7 @@ export default class AppStateController extends EventEmitter {
this.onInactiveTimeout = onInactiveTimeout || (() => undefined); this.onInactiveTimeout = onInactiveTimeout || (() => undefined);
this.store = new ObservableStore({ this.store = new ObservableStore({
timeoutMinutes: 0, timeoutMinutes: DEFAULT_AUTO_LOCK_TIME_LIMIT,
connectedStatusPopoverHasBeenShown: true, connectedStatusPopoverHasBeenShown: true,
defaultHomeActiveTabName: null, defaultHomeActiveTabName: null,
browserEnvironment: {}, browserEnvironment: {},

View File

@ -3,3 +3,5 @@ export enum ThemeType {
dark = 'dark', dark = 'dark',
os = 'os', os = 'os',
} }
export const DEFAULT_AUTO_LOCK_TIME_LIMIT = 0;

View File

@ -38,7 +38,7 @@ describe('Auto-Lock Timer', function () {
await autoLockTimerInput.fill(10081); await autoLockTimerInput.fill(10081);
await driver.waitForSelector({ await driver.waitForSelector({
css: '#autoTimeout-helper-text', css: '#autoTimeout-helper-text',
text: 'Lock time is too great', text: 'Lock time must be a number between 0 and 10080',
}); });
await autoLockTimerInput.fill(sixSecsInMins); await autoLockTimerInput.fill(sixSecsInMins);
await driver.assertElementNotPresent('#autoTimeout-helper-text'); await driver.assertElementNotPresent('#autoTimeout-helper-text');

View File

@ -15,6 +15,7 @@ import { updateTransactionGasFees } from '../../store/actions';
import { setCustomGasLimit, setCustomGasPrice } from '../gas/gas.duck'; import { setCustomGasLimit, setCustomGasPrice } from '../gas/gas.duck';
import { KeyringType } from '../../../shared/constants/keyring'; import { KeyringType } from '../../../shared/constants/keyring';
import { DEFAULT_AUTO_LOCK_TIME_LIMIT } from '../../../shared/constants/preferences';
import { isEqualCaseInsensitive } from '../../../shared/modules/string-utils'; import { isEqualCaseInsensitive } from '../../../shared/modules/string-utils';
import { stripHexPrefix } from '../../../shared/modules/hexstring-utils'; import { stripHexPrefix } from '../../../shared/modules/hexstring-utils';
import { decGWEIToHexWEI } from '../../../shared/modules/conversion.utils'; import { decGWEIToHexWEI } from '../../../shared/modules/conversion.utils';
@ -37,7 +38,7 @@ const initialState = {
currentLocale: '', currentLocale: '',
currentBlockGasLimit: '', currentBlockGasLimit: '',
preferences: { preferences: {
autoLockTimeLimit: undefined, autoLockTimeLimit: DEFAULT_AUTO_LOCK_TIME_LIMIT,
showFiatInTestnets: false, showFiatInTestnets: false,
showTestNetworks: false, showTestNetworks: false,
useNativeCurrencyAsPrimaryCurrency: true, useNativeCurrencyAsPrimaryCurrency: true,

View File

@ -25,12 +25,14 @@ import { pageChanged } from '../../ducks/history/history';
import { prepareToLeaveSwaps } from '../../ducks/swaps/swaps'; import { prepareToLeaveSwaps } from '../../ducks/swaps/swaps';
import { getSendStage } from '../../ducks/send'; import { getSendStage } from '../../ducks/send';
import { getProviderConfig } from '../../ducks/metamask/metamask'; import { getProviderConfig } from '../../ducks/metamask/metamask';
import { DEFAULT_AUTO_LOCK_TIME_LIMIT } from '../../../shared/constants/preferences';
import Routes from './routes.component'; import Routes from './routes.component';
function mapStateToProps(state) { function mapStateToProps(state) {
const { appState } = state; const { appState } = state;
const { alertOpen, alertMessage, isLoading, loadingMessage } = appState; const { alertOpen, alertMessage, isLoading, loadingMessage } = appState;
const { autoLockTimeLimit = 0 } = getPreferences(state); const { autoLockTimeLimit = DEFAULT_AUTO_LOCK_TIME_LIMIT } =
getPreferences(state);
const { completedOnboarding } = state.metamask; const { completedOnboarding } = state.metamask;
return { return {

View File

@ -22,6 +22,7 @@ import {
MetaMetricsEventCategory, MetaMetricsEventCategory,
MetaMetricsEventName, MetaMetricsEventName,
} from '../../../../shared/constants/metametrics'; } from '../../../../shared/constants/metametrics';
import { DEFAULT_AUTO_LOCK_TIME_LIMIT } from '../../../../shared/constants/preferences';
import { exportAsFile } from '../../../helpers/utils/export-utils'; import { exportAsFile } from '../../../helpers/utils/export-utils';
import ActionableMessage from '../../../components/ui/actionable-message'; import ActionableMessage from '../../../components/ui/actionable-message';
import ZENDESK_URLS from '../../../helpers/constants/zendesk-url'; import ZENDESK_URLS from '../../../helpers/constants/zendesk-url';
@ -72,6 +73,7 @@ export default class AdvancedTab extends PureComponent {
state = { state = {
autoLockTimeLimit: this.props.autoLockTimeLimit, autoLockTimeLimit: this.props.autoLockTimeLimit,
autoLockTimeLimitBeforeNormalization: this.props.autoLockTimeLimit,
lockTimeError: '', lockTimeError: '',
showLedgerTransportWarning: false, showLedgerTransportWarning: false,
showResultMessage: false, showResultMessage: false,
@ -379,11 +381,10 @@ export default class AdvancedTab extends PureComponent {
<div className="settings-page__content-item"> <div className="settings-page__content-item">
<div className="settings-page__content-item-col"> <div className="settings-page__content-item-col">
<TextField <TextField
type="number"
id="autoTimeout" id="autoTimeout"
data-testid="auto-lockout-time" data-testid="auto-lockout-time"
placeholder="5" placeholder="0"
value={this.state.autoLockTimeLimit} value={this.state.autoLockTimeLimitBeforeNormalization}
onChange={(e) => this.handleLockChange(e.target.value)} onChange={(e) => this.handleLockChange(e.target.value)}
error={lockTimeError} error={lockTimeError}
fullWidth fullWidth
@ -601,21 +602,41 @@ export default class AdvancedTab extends PureComponent {
); );
} }
handleLockChange(time) { handleLockChange(autoLockTimeLimitBeforeNormalization) {
const { t } = this.context; const { t } = this.context;
const autoLockTimeLimit = Math.max(Number(time), 0);
this.setState(() => { if (autoLockTimeLimitBeforeNormalization === '') {
let lockTimeError = ''; this.setState({
autoLockTimeLimitBeforeNormalization,
autoLockTimeLimit: DEFAULT_AUTO_LOCK_TIME_LIMIT.toString(),
lockTimeError: '',
});
return;
}
if (autoLockTimeLimit > 10080) { const autoLockTimeLimitAfterNormalization = Number(
lockTimeError = t('lockTimeTooGreat'); autoLockTimeLimitBeforeNormalization,
} );
return { if (
autoLockTimeLimit, Number.isNaN(autoLockTimeLimitAfterNormalization) ||
lockTimeError, autoLockTimeLimitAfterNormalization < 0 ||
}; autoLockTimeLimitAfterNormalization > 10080
) {
this.setState({
autoLockTimeLimitBeforeNormalization,
autoLockTimeLimit: null,
lockTimeError: t('lockTimeInvalid'),
});
return;
}
const autoLockTimeLimit = autoLockTimeLimitAfterNormalization;
this.setState({
autoLockTimeLimitBeforeNormalization,
autoLockTimeLimit,
lockTimeError: '',
}); });
} }

View File

@ -31,14 +31,21 @@ describe('AdvancedTab Component', () => {
expect(restoreFile).toBeInTheDocument(); expect(restoreFile).toBeInTheDocument();
}); });
it('should update autoLockTimeLimit', () => { it('should default the auto-lockout time to 0', () => {
const { queryByTestId } = renderWithProvider(<AdvancedTab />, mockStore);
const autoLockoutTime = queryByTestId('auto-lockout-time');
expect(autoLockoutTime).toHaveValue('0');
});
it('should update the auto-lockout time', () => {
const { queryByTestId } = renderWithProvider(<AdvancedTab />, mockStore); const { queryByTestId } = renderWithProvider(<AdvancedTab />, mockStore);
const autoLockoutTime = queryByTestId('auto-lockout-time'); const autoLockoutTime = queryByTestId('auto-lockout-time');
const autoLockoutButton = queryByTestId('auto-lockout-button'); const autoLockoutButton = queryByTestId('auto-lockout-button');
fireEvent.change(autoLockoutTime, { target: { value: 1440 } }); fireEvent.change(autoLockoutTime, { target: { value: '1440' } });
expect(autoLockoutTime).toHaveValue(1440); expect(autoLockoutTime).toHaveValue('1440');
fireEvent.click(autoLockoutButton); fireEvent.click(autoLockoutButton);

View File

@ -17,6 +17,7 @@ import {
} from '../../../store/actions'; } from '../../../store/actions';
import { getPreferences } from '../../../selectors'; import { getPreferences } from '../../../selectors';
import { doesUserHaveALedgerAccount } from '../../../ducks/metamask/metamask'; import { doesUserHaveALedgerAccount } from '../../../ducks/metamask/metamask';
import { DEFAULT_AUTO_LOCK_TIME_LIMIT } from '../../../../shared/constants/preferences';
import AdvancedTab from './advanced-tab.component'; import AdvancedTab from './advanced-tab.component';
export const mapStateToProps = (state) => { export const mapStateToProps = (state) => {
@ -37,7 +38,7 @@ export const mapStateToProps = (state) => {
const { const {
showFiatInTestnets, showFiatInTestnets,
showTestNetworks, showTestNetworks,
autoLockTimeLimit = 0, autoLockTimeLimit = DEFAULT_AUTO_LOCK_TIME_LIMIT,
} = getPreferences(state); } = getPreferences(state);
const userHasALedgerAccount = doesUserHaveALedgerAccount(state); const userHasALedgerAccount = doesUserHaveALedgerAccount(state);