fix(client): Hide all visible passwords on form submit. (#3969)
* fix(password): save passwords as passwords, not text Fixes #3799 * fix(client): Hide all visible passwords on form submit. This builds on @TDA's work. Before the form is submit, convert all visible passwords from [type=text] to [type=password] so that they are not saved as form data. In the process, I noticed two problems that are fixed: * The cursor was not being correctly replaced to its original position once a password field was converted. * If `setPasswordVisibility` was called programatically to toggle the state of the password field, the corresponding "Show" checkbox was not kept in sync. fixes #3799 * chore(client): `wekbit` => `webkit` r=@vladikoff
This commit is contained in:
Shane Tomlinson
GitHub
Родитель
b271bceb16
Коммит
a6c848cc9f
|
|
@ -670,18 +670,14 @@ define(function (require, exports, module) {
|
|||
/**
|
||||
* Safely focus an element
|
||||
*/
|
||||
focus: function (which) {
|
||||
focus (which) {
|
||||
try {
|
||||
var focusEl = this.$(which);
|
||||
const focusEl = this.$(which);
|
||||
// place the cursor at the end of the input when the
|
||||
// element is focused.
|
||||
var self = this;
|
||||
focusEl.one('focus', function () {
|
||||
try {
|
||||
this.selectionStart = this.selectionEnd = this.value.length;
|
||||
} catch (e) {
|
||||
// This can blow up on password fields in Chrome. Drop the error on
|
||||
// the ground, for whatever reason, it still behaves as we expect.
|
||||
}
|
||||
self.placeCursorAt(this, this.value.length);
|
||||
});
|
||||
focusEl.get(0).focus();
|
||||
} catch (e) {
|
||||
|
|
@ -689,6 +685,25 @@ define(function (require, exports, module) {
|
|||
}
|
||||
},
|
||||
|
||||
/**
|
||||
* Place the cursor at the given position within the input element
|
||||
*
|
||||
* @param {selector | element} which
|
||||
* @param {number} selectionStart - defaults to 0.
|
||||
* @param {number} selectionEnd - defaults to selectionStart.
|
||||
*/
|
||||
placeCursorAt (which, selectionStart = 0, selectionEnd = selectionStart) {
|
||||
const el = $(which).get(0);
|
||||
|
||||
try {
|
||||
el.selectionStart = selectionStart;
|
||||
el.selectionEnd = selectionEnd;
|
||||
} catch (e) {
|
||||
// This can blow up on password fields in Chrome. Drop the error on
|
||||
// the ground, for whatever reason, it still behaves as we expect.
|
||||
}
|
||||
},
|
||||
|
||||
/**
|
||||
* Invoke the specified handler with the given event. Handler
|
||||
* can either be a function or a string. If a string, looks for
|
||||
|
|
|
|||
|
|
@ -172,6 +172,8 @@ define(function (require, exports, module) {
|
|||
event.stopImmediatePropagation();
|
||||
}
|
||||
|
||||
this.trigger('submitStart');
|
||||
|
||||
return p()
|
||||
.then(function () {
|
||||
if (self.isHalted()) {
|
||||
|
|
|
|||
|
|
@ -7,7 +7,8 @@
|
|||
define(function (require, exports, module) {
|
||||
'use strict';
|
||||
|
||||
var Constants = require('lib/constants');
|
||||
const $ = require('jquery');
|
||||
const Constants = require('lib/constants');
|
||||
|
||||
module.exports = {
|
||||
events: {
|
||||
|
|
@ -15,7 +16,15 @@ define(function (require, exports, module) {
|
|||
'keyup input.password': 'onPasswordKeyUp'
|
||||
},
|
||||
|
||||
afterVisible: function () {
|
||||
initialize () {
|
||||
// An internal submitStart event is listened for instead of
|
||||
// the `submit` DOM event because form.js already binds a submit
|
||||
// event. Because of the way Cocktail wraps colliding functions,
|
||||
// the form is always submit if a second event handler is added.
|
||||
this.on('submitStart', () => this.hideVisiblePasswords());
|
||||
},
|
||||
|
||||
afterVisible () {
|
||||
if (this.isInExperiment && this.isInExperiment('showPassword')) {
|
||||
this.notifier.trigger('showPassword.triggered');
|
||||
|
||||
|
|
@ -25,7 +34,7 @@ define(function (require, exports, module) {
|
|||
}
|
||||
},
|
||||
|
||||
onPasswordVisibilityChange: function (event) {
|
||||
onPasswordVisibilityChange (event) {
|
||||
var target = this.$(event.target);
|
||||
this.setPasswordVisibilityFromButton(target);
|
||||
|
||||
|
|
@ -39,39 +48,80 @@ define(function (require, exports, module) {
|
|||
this.focus(controlsSelector);
|
||||
},
|
||||
|
||||
setPasswordVisibilityFromButton: function (button) {
|
||||
setPasswordVisibilityFromButton (button) {
|
||||
var isVisible = this.$(button).is(':checked');
|
||||
var targets = this.getAffectedPasswordInputs(button);
|
||||
this.setPasswordVisibility(isVisible, targets);
|
||||
this.setPasswordVisibility(targets, isVisible);
|
||||
},
|
||||
|
||||
getAffectedPasswordInputs: function (button) {
|
||||
var passwordField = this.$(button).siblings('.password');
|
||||
getAffectedPasswordInputs (button) {
|
||||
var $passwordEl = this.$(button).siblings('.password');
|
||||
if (this.$(button).data('synchronizeShow')) {
|
||||
passwordField = this.$('.password');
|
||||
$passwordEl = this.$('.password');
|
||||
}
|
||||
return passwordField;
|
||||
return $passwordEl;
|
||||
},
|
||||
|
||||
setPasswordVisibility: function (isVisible, passwordField) {
|
||||
/**
|
||||
* Set the password visibility for an element. Ensure the "show" button's
|
||||
* state is synchronized.
|
||||
*
|
||||
* @param {selector | element} which
|
||||
* @param {boolean} isVisible
|
||||
*/
|
||||
setPasswordVisibility (which, isVisible) {
|
||||
const $passwordEl = $(which);
|
||||
const $showPasswordEl = $passwordEl.siblings('.show-password');
|
||||
|
||||
// Store the cursor position before updating the element type
|
||||
// or else the cursor is set to before the first character.
|
||||
|
||||
// `which` can match more than one element. If this is the case
|
||||
// find the element that is focused.
|
||||
let $focusedEl = $passwordEl.filter(':focus');
|
||||
let selectionStart;
|
||||
let selectionEnd;
|
||||
|
||||
if ($focusedEl.length) {
|
||||
const focusedEl = $focusedEl.get(0);
|
||||
selectionStart = focusedEl.selectionStart;
|
||||
selectionEnd = focusedEl.selectionEnd;
|
||||
}
|
||||
|
||||
try {
|
||||
if (isVisible) {
|
||||
passwordField.attr('type', 'text').attr('autocomplete', 'off')
|
||||
$passwordEl.attr('type', 'text').attr('autocomplete', 'off')
|
||||
.attr('autocorrect', 'off').attr('autocapitalize', 'off');
|
||||
$showPasswordEl.attr('checked', true);
|
||||
this.logViewEvent('password.visible');
|
||||
} else {
|
||||
passwordField.attr('type', 'password');
|
||||
passwordField.removeAttr('autocomplete')
|
||||
.removeAttr('autocorrect').removeAttr('autocapitalize');
|
||||
$passwordEl.attr('type', 'password').removeAttr('autocomplete')
|
||||
.removeAttr('autocorrect').removeAttr('autocapitalize');
|
||||
$showPasswordEl.removeAttr('checked');
|
||||
this.logViewEvent('password.hidden');
|
||||
}
|
||||
} catch (e) {
|
||||
// IE8 blows up when changing the type of the input field. Other
|
||||
// browsers might too. Ignore the error.
|
||||
}
|
||||
|
||||
// Restore the cursor position if the element is in focus.
|
||||
if ($focusedEl.length) {
|
||||
this.placeCursorAt($focusedEl, selectionStart, selectionEnd);
|
||||
}
|
||||
},
|
||||
|
||||
onPasswordKeyUp: function () {
|
||||
/**
|
||||
* Hide all visible passwords to prevent passwords from being saved
|
||||
* by the browser as text form data.
|
||||
*/
|
||||
hideVisiblePasswords () {
|
||||
this.$el.find('.password[type=text]').each((index, el) => {
|
||||
this.setPasswordVisibility(el, false);
|
||||
});
|
||||
},
|
||||
|
||||
onPasswordKeyUp () {
|
||||
var values = [];
|
||||
|
||||
// Values contains all password classes length
|
||||
|
|
@ -88,11 +138,11 @@ define(function (require, exports, module) {
|
|||
}
|
||||
},
|
||||
|
||||
showPasswordHelper: function () {
|
||||
showPasswordHelper () {
|
||||
this.$('.input-help').css('opacity', '1');
|
||||
},
|
||||
|
||||
hidePasswordHelper: function () {
|
||||
hidePasswordHelper () {
|
||||
// Hide all input-help classes except input-help-forgot-pw
|
||||
this.$('.input-help:not(.input-help-forgot-pw)').css('opacity', '0');
|
||||
}
|
||||
|
|
|
|||
|
|
@ -11,6 +11,23 @@ define(function (require, exports, module) {
|
|||
var ProfileMock = require('../mocks/profile.js');
|
||||
var sinon = require('sinon');
|
||||
|
||||
function noOp () {}
|
||||
|
||||
function ifDocumentFocused(callback, done = noOp) {
|
||||
if (document.hasFocus && document.hasFocus()) {
|
||||
callback();
|
||||
} else {
|
||||
const message =
|
||||
'Cannot check for focus - document does not have focus.\n' +
|
||||
'If this is in Travis-CI, Sauce Labs, or Opera, this is expected.\n' +
|
||||
'Otherwise, try focusing the test document instead of \n' +
|
||||
'another window or dev tools.';
|
||||
|
||||
console.warn(message);
|
||||
done();
|
||||
}
|
||||
}
|
||||
|
||||
function requiresFocus(callback, done) {
|
||||
// Give the document focus
|
||||
window.focus();
|
||||
|
|
@ -20,20 +37,7 @@ define(function (require, exports, module) {
|
|||
document.activeElement.blur();
|
||||
}
|
||||
|
||||
if (document.hasFocus && document.hasFocus()) {
|
||||
callback();
|
||||
} else {
|
||||
var message =
|
||||
'Cannot check for focus - document does not have focus.\n' +
|
||||
'If this is in Travis-CI, Sauce Labs, or Opera, this is expected.\n' +
|
||||
'Otherwise, try focusing the test document instead of \n' +
|
||||
'another window or dev tools.';
|
||||
|
||||
console.warn(message);
|
||||
if (done) {
|
||||
done();
|
||||
}
|
||||
}
|
||||
ifDocumentFocused(callback, done);
|
||||
}
|
||||
|
||||
function addFxaClientSpy(fxaClient) {
|
||||
|
|
@ -164,6 +168,7 @@ define(function (require, exports, module) {
|
|||
createUid: createUid,
|
||||
emailToUser: emailToUser,
|
||||
getValueLabel: getValueLabel,
|
||||
ifDocumentFocused: ifDocumentFocused,
|
||||
indexOfEvent: indexOfEvent,
|
||||
isErrorLogged: isErrorLogged,
|
||||
isEventLogged: isEventLogged,
|
||||
|
|
|
|||
|
|
@ -361,7 +361,7 @@ define(function (require, exports, module) {
|
|||
it('focuses descendent element containing `autofocus` if html has `no-touch` class', function (done) {
|
||||
requiresFocus(function () {
|
||||
$('html').addClass('no-touch');
|
||||
// wekbit fails unless focusing another element first.
|
||||
// webkit fails unless focusing another element first.
|
||||
$('#otherElement').focus();
|
||||
|
||||
var handlerCalled = false;
|
||||
|
|
@ -589,20 +589,54 @@ define(function (require, exports, module) {
|
|||
});
|
||||
});
|
||||
|
||||
describe('focus', function () {
|
||||
it('focuses an element', function (done) {
|
||||
requiresFocus(function () {
|
||||
// wekbit fails unless focusing another element first.
|
||||
describe('focus', () => {
|
||||
it('focuses an element, sets the cursor position', (done) => {
|
||||
requiresFocus(() => {
|
||||
// webkit fails unless focusing another element first.
|
||||
$('#otherElement').focus();
|
||||
|
||||
view.$('#focusMe').one('focus', function () {
|
||||
const elText = 'some text';
|
||||
const $focusEl = view.$('#focusMe').val(elText);
|
||||
|
||||
// Use setTimeout because the selection is set in a `focus` handler
|
||||
// and this `focus` handler is run first.
|
||||
view.$('#focusMe').one('focus', () => setTimeout(() => {
|
||||
const focusEl = $focusEl.get(0);
|
||||
assert.equal(focusEl.selectionStart, elText.length);
|
||||
assert.equal(focusEl.selectionEnd, elText.length);
|
||||
done();
|
||||
});
|
||||
}, 0));
|
||||
|
||||
view.focus('#focusMe');
|
||||
}, done);
|
||||
});
|
||||
});
|
||||
|
||||
describe('placeCursorAt', () => {
|
||||
const elText = 'some text';
|
||||
let $focusEl;
|
||||
let focusEl;
|
||||
|
||||
beforeEach(() => {
|
||||
$focusEl = view.$('#focusMe').val(elText);
|
||||
focusEl = $focusEl.get(0);
|
||||
});
|
||||
|
||||
it('places the cursor at the specified position if only start given', () => {
|
||||
view.placeCursorAt('#focusMe', elText.length);
|
||||
|
||||
assert.equal(focusEl.selectionStart, elText.length);
|
||||
assert.equal(focusEl.selectionEnd, elText.length);
|
||||
});
|
||||
|
||||
it('places the cursor at the specified selection if both given', () => {
|
||||
view.placeCursorAt('#focusMe', elText.length - 2, elText.length);
|
||||
|
||||
assert.equal(focusEl.selectionStart, elText.length - 2);
|
||||
assert.equal(focusEl.selectionEnd, elText.length);
|
||||
});
|
||||
});
|
||||
|
||||
describe('trackChildView/untrackChildView', function () {
|
||||
it('trackChildView tracks a childView, untrackChildView untracks the childView', function () {
|
||||
var childView = new BaseView();
|
||||
|
|
|
|||
|
|
@ -164,6 +164,12 @@ define(function (require, exports, module) {
|
|||
});
|
||||
|
||||
describe('validateAndSubmit', function () {
|
||||
it('triggers a `submitStart` event', (done) => {
|
||||
view.on('submitStart', () => done());
|
||||
|
||||
view.validateAndSubmit();
|
||||
});
|
||||
|
||||
it('submits form if isValid returns true', function () {
|
||||
view.formIsValid = true;
|
||||
view.enableSubmitIfValid();
|
||||
|
|
|
|||
|
|
@ -5,24 +5,23 @@
|
|||
define(function (require, exports, module) {
|
||||
'use strict';
|
||||
|
||||
var $ = require('jquery');
|
||||
var BaseView = require('views/base');
|
||||
var chai = require('chai');
|
||||
var Cocktail = require('cocktail');
|
||||
var ExperimentMixin = require('views/mixins/experiment-mixin');
|
||||
var Metrics = require('lib/metrics');
|
||||
var Notifier = require('lib/channels/notifier');
|
||||
var PasswordMixin = require('views/mixins/password-mixin');
|
||||
var Relier = require('models/reliers/relier');
|
||||
var sinon = require('sinon');
|
||||
var TestHelpers = require('../../../lib/helpers');
|
||||
var TestTemplate = require('stache!templates/test_template');
|
||||
const $ = require('jquery');
|
||||
const assert = require('chai').assert;
|
||||
const BaseView = require('views/base');
|
||||
const Cocktail = require('cocktail');
|
||||
const ExperimentMixin = require('views/mixins/experiment-mixin');
|
||||
const Metrics = require('lib/metrics');
|
||||
const Notifier = require('lib/channels/notifier');
|
||||
const PasswordMixin = require('views/mixins/password-mixin');
|
||||
const Relier = require('models/reliers/relier');
|
||||
const sinon = require('sinon');
|
||||
const TestHelpers = require('../../../lib/helpers');
|
||||
const TestTemplate = require('stache!templates/test_template');
|
||||
|
||||
var assert = chai.assert;
|
||||
|
||||
var PasswordView = BaseView.extend({
|
||||
const PasswordView = BaseView.extend({
|
||||
template: TestTemplate
|
||||
});
|
||||
|
||||
Cocktail.mixin(
|
||||
PasswordView,
|
||||
PasswordMixin,
|
||||
|
|
@ -30,9 +29,9 @@ define(function (require, exports, module) {
|
|||
);
|
||||
|
||||
describe('views/mixins/password-mixin', function () {
|
||||
var view;
|
||||
var relier;
|
||||
var metrics;
|
||||
let metrics;
|
||||
let relier;
|
||||
let view;
|
||||
|
||||
beforeEach(function () {
|
||||
relier = new Relier();
|
||||
|
|
@ -94,7 +93,6 @@ define(function (require, exports, module) {
|
|||
view.afterVisible();
|
||||
assert.isFalse(view.$('.show-password-label').is(':hidden'));
|
||||
});
|
||||
|
||||
});
|
||||
|
||||
describe('onPasswordVisibilityChange', function () {
|
||||
|
|
@ -114,27 +112,69 @@ define(function (require, exports, module) {
|
|||
});
|
||||
});
|
||||
|
||||
describe('setPasswordVisibility', () => {
|
||||
it('to visible', () => {
|
||||
const $passwordEl = view.$('#password');
|
||||
$passwordEl.val('password').focus();
|
||||
|
||||
const selectionStart = 1;
|
||||
const selectionEnd = 2;
|
||||
|
||||
const passwordEl = $passwordEl.get(0);
|
||||
passwordEl.selectionStart = selectionStart;
|
||||
passwordEl.selectionEnd = selectionEnd;
|
||||
|
||||
view.setPasswordVisibility('#password', true);
|
||||
|
||||
assert.equal($passwordEl.attr('type'), 'text');
|
||||
assert.equal($passwordEl.attr('autocapitalize'), 'off');
|
||||
assert.equal($passwordEl.attr('autocorrect'), 'off');
|
||||
|
||||
// Ensure the cursor is replaced to its original position.
|
||||
// Only make the check if the document has focus, otherwise
|
||||
// the test fails. Document may not have focus if the dev
|
||||
// clicks out of the test window or when run in PhantomJS.
|
||||
TestHelpers.ifDocumentFocused(() => {
|
||||
assert.equal(passwordEl.selectionStart, selectionStart);
|
||||
assert.equal(passwordEl.selectionEnd, selectionEnd);
|
||||
});
|
||||
|
||||
// Ensure the show password state stays in sync
|
||||
const $showPasswordEl = $passwordEl.siblings('.show-password');
|
||||
assert.isTrue($showPasswordEl.is(':checked'));
|
||||
});
|
||||
|
||||
it('to hidden', () => {
|
||||
view.setPasswordVisibility('#password', false);
|
||||
|
||||
const $passwordEl = view.$('#password');
|
||||
assert.equal($passwordEl.attr('autocomplete'), null);
|
||||
assert.equal($passwordEl.attr('autocapitalize'), null);
|
||||
assert.equal($passwordEl.attr('autocorrect'), null);
|
||||
|
||||
// Ensure the show password state stays in sync
|
||||
const $showPasswordEl = $passwordEl.siblings('.show-password');
|
||||
assert.isFalse($showPasswordEl.is(':checked'));
|
||||
});
|
||||
});
|
||||
|
||||
describe('setPasswordVisibilityFromButton', function () {
|
||||
it('sets the password field to type=text if button is checked', function () {
|
||||
view.$('#show-password').attr('checked', 'checked');
|
||||
view.setPasswordVisibilityFromButton('#show-password');
|
||||
assert.equal(view.$('#password').attr('type'), 'text');
|
||||
assert.equal(view.$('#password').attr('autocapitalize'), 'off');
|
||||
assert.equal(view.$('#password').attr('autocorrect'), 'off');
|
||||
});
|
||||
|
||||
it('sets the password field to type=password if button is unchecked', function () {
|
||||
view.$('#show-password').removeAttr('checked');
|
||||
view.setPasswordVisibilityFromButton('#show-password');
|
||||
assert.equal(view.$('#password').attr('type'), 'password');
|
||||
assert.isUndefined(view.$('#password').attr('autocapitalize'));
|
||||
assert.isUndefined(view.$('#password').attr('autocorrect'));
|
||||
});
|
||||
});
|
||||
|
||||
describe('clicking on unsynched/synched show buttons', function () {
|
||||
it('gets pwd inputs to be shown', function () {
|
||||
var targets = view.getAffectedPasswordInputs('#show-password');
|
||||
it('gets password inputs to be shown', function () {
|
||||
let targets = view.getAffectedPasswordInputs('#show-password');
|
||||
assert.equal(targets.length, 1);
|
||||
|
||||
view.$('#show-password').data('synchronize-show', 'true');
|
||||
|
|
@ -147,26 +187,14 @@ define(function (require, exports, module) {
|
|||
it('pw field set to text when clicked', function () {
|
||||
view.$('.show-password').click();
|
||||
assert.equal(view.$('#password').attr('type'), 'text');
|
||||
assert.equal(view.$('#password').attr('autocomplete'), 'off');
|
||||
assert.equal(view.$('#password').attr('autocapitalize'), 'off');
|
||||
assert.equal(view.$('#password').attr('autocorrect'), 'off');
|
||||
assert.equal(view.$('#vpassword').attr('type'), 'text');
|
||||
assert.equal(view.$('#vpassword').attr('autocomplete'), 'off');
|
||||
assert.equal(view.$('#vpassword').attr('autocomplete'), 'off');
|
||||
assert.equal(view.$('#vpassword').attr('autocapitalize'), 'off');
|
||||
});
|
||||
|
||||
it('pw field set to password when clicked again', function () {
|
||||
view.$('.show-password').click();
|
||||
view.$('.show-password').click();
|
||||
assert.equal(view.$('#password').attr('type'), 'password');
|
||||
assert.equal(view.$('#password').attr('autocomplete'), null);
|
||||
assert.equal(view.$('#password').attr('autocapitalize'), null);
|
||||
assert.equal(view.$('#password').attr('autocorrect'), null);
|
||||
assert.equal($('#vpassword').attr('type'), 'password');
|
||||
assert.equal($('#vpassword').attr('autocomplete'), null);
|
||||
assert.equal($('#vpassword').attr('autocapitalize'), null);
|
||||
assert.equal($('#vpassword').attr('autocorrect'), null);
|
||||
assert.equal(view.$('#vpassword').attr('type'), 'password');
|
||||
});
|
||||
|
||||
it('logs whether the password is shown or hidden', function () {
|
||||
|
|
@ -206,5 +234,36 @@ define(function (require, exports, module) {
|
|||
assert.equal(view.$('.input-help-forgot-pw').css('opacity'), '1');
|
||||
});
|
||||
});
|
||||
|
||||
describe('hideVisiblePasswords', () => {
|
||||
it('sets all password fields to type `password`', () => {
|
||||
const $passwordEls = view.$('.password');
|
||||
|
||||
assert.equal($passwordEls.length, 2);
|
||||
|
||||
$passwordEls.each((index, el) => {
|
||||
view.setPasswordVisibility(el, true);
|
||||
assert.equal(el.type, 'text');
|
||||
});
|
||||
|
||||
view.hideVisiblePasswords();
|
||||
|
||||
$passwordEls.each((i, el) => {
|
||||
assert.equal(el.type, 'password');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('submitStart event', () => {
|
||||
beforeEach(() => {
|
||||
sinon.spy(view, 'hideVisiblePasswords');
|
||||
});
|
||||
|
||||
it('hides all visible passwords', () => {
|
||||
assert.equal(view.hideVisiblePasswords.callCount, 0);
|
||||
view.trigger('submitStart');
|
||||
assert.equal(view.hideVisiblePasswords.callCount, 1);
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче