Skip to content
This repository has been archived by the owner. It is now read-only.

fix(client): Only add autocomplete=off when in sync or to text fields acting as passwords. #1989

Merged
merged 1 commit into from Dec 23, 2014

Conversation

@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Dec 15, 2014

fixes #1788


describe('setPasswordVisibility', function () {
it('goes', function () {
assert.isTrue(true);

This comment has been minimized.

@zaach

zaach Dec 16, 2014
Contributor

WIP?

This comment has been minimized.

@shane-tomlinson shane-tomlinson force-pushed the issue-1788-autocomplete-off branch 5 times, most recently from 65c4ed9 to 10704d4 Dec 16, 2014
@@ -123,6 +122,7 @@ function (_, Backbone, $, p, AuthErrors,
})
.then(_.bind(self.afterRender, self))
.then(function () {
self.trigger('afterRender');

This comment has been minimized.

@shane-tomlinson

shane-tomlinson Dec 17, 2014
Author Member

What's this?

@shane-tomlinson shane-tomlinson force-pushed the issue-1788-autocomplete-off branch from 10704d4 to f79d1fd Dec 17, 2014
})
.end()

.sleep(5000)

This comment has been minimized.

@shane-tomlinson

shane-tomlinson Dec 17, 2014
Author Member

need a sleep?

@@ -78,7 +78,7 @@ function (chai, jQuery, sinon, BaseView, p, Translator, EphemeralMessages, Metri

return view.render()
.then(function () {
jQuery('#container').append(view.el);

This comment has been minimized.

@shane-tomlinson

shane-tomlinson Dec 17, 2014
Author Member

jQuery -> $ is an unrelated change for consistency.
append->html ensures the container element is cleared between each test.

…ssword fields!

* Always add `autocomplete=off` when the relier is Sync.
* Add `autocomplete=off` to text fields acting as passwords, otherwise passwords are displayed in a form autocomplete dropdown. Yuck.

fixes #1788
@shane-tomlinson shane-tomlinson force-pushed the issue-1788-autocomplete-off branch from f79d1fd to 7df783f Dec 17, 2014
@@ -202,21 +202,6 @@ function (chai, sinon, p, AuthErrors, Metrics, FxaClient, InterTabChannel,
});
});

describe('updatePasswordVisibility', function () {

This comment has been minimized.

@shane-tomlinson

shane-tomlinson Dec 17, 2014
Author Member

These tests are moved to the mixins/password-mixin.js set of tests.

@@ -105,7 +105,7 @@ function (chai, _, $, moment, sinon, p, View, Session, AuthErrors, Metrics,

return view.render()
.then(function () {
$('#container').append(view.el);
$('#container').html(view.el);

This comment has been minimized.

@shane-tomlinson

shane-tomlinson Dec 17, 2014
Author Member

append->html ensures the container is cleared between each test.

@shane-tomlinson
Copy link
Member Author

@shane-tomlinson shane-tomlinson commented Dec 17, 2014

Removing the WIP - ready for review!

@shane-tomlinson shane-tomlinson changed the title WIP fix(client): Only add autocomplete=off when in sync or to text fields acting as passwords. fix(client): Only add autocomplete=off when in sync or to text fields acting as passwords. Dec 17, 2014
@zaach
Copy link
Contributor

@zaach zaach commented Dec 23, 2014

Excellent!

zaach added a commit that referenced this pull request Dec 23, 2014
fix(client): Only add `autocomplete=off` when in sync or to text fields acting as passwords.
@zaach zaach merged commit 9507003 into master Dec 23, 2014
1 check passed
1 check passed
@shane-tomlinson
continuous-integration/travis-ci The Travis CI build passed
Details
@zaach zaach deleted the issue-1788-autocomplete-off branch Dec 23, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants