zxcvbn() not defined (and making it impossible to edit an existing user)

[wp-plugin@…]

This site is only using the Akismet plugin.

Uncaught ReferenceError: zxcvbn is not defined

wp.passwordStrength.meter @ load-scripts.php?c=0&load[]=hoverIntent,common,admin-bar,password-strength-meter,underscore,wp-util…:4
f @ load-scripts.php?c=0&load[]=hoverIntent,common,admin-bar,password-strength-meter,underscore,wp-util…:12
m.event.dispatch @ load-scripts.php?c=0&load[]=jquery-core,jquery-migrate,utils,zxcvbn-async&ver=4.3.1:4
r.handle @ load-scripts.php?c=0&load[]=jquery-core,jquery-migrate,utils,zxcvbn-async&ver=4.3.1:4

2015-12-07 17:01:57.903 load-scripts.php?c=0&load[]=hoverIntent,common,admin-bar,password-strength-meter,underscore,wp-util…:4

Uncaught ReferenceError: zxcvbn is not defined

wp.passwordStrength.meter @ load-scripts.php?c=0&load[]=hoverIntent,common,admin-bar,password-strength-meter,underscore,wp-util…:4
f @ load-scripts.php?c=0&load[]=hoverIntent,common,admin-bar,password-strength-meter,underscore,wp-util…:12
m.event.dispatch @ load-scripts.php?c=0&load[]=jquery-core,jquery-migrate,utils,zxcvbn-async&ver=4.3.1:4
r.handle @ load-scripts.php?c=0&load[]=jquery-core,jquery-migrate,utils,zxcvbn-async&ver=4.3.1:4
m.event.trigger @ load-scripts.php?c=0&load[]=jquery-core,jquery-migrate,utils,zxcvbn-async&ver=4.3.1:4
e.event.trigger @ load-scripts.php?c=0&load[]=jquery-core,jquery-migrate,utils,zxcvbn-async&ver=4.3.1:8
(anonymous function) @ load-scripts.php?c=0&load[]=jquery-core,jquery-migrate,utils,zxcvbn-async&ver=4.3.1:4
m.extend.each @ load-scripts.php?c=0&load[]=jquery-core,jquery-migrate,utils,zxcvbn-async&ver=4.3.1:2
m.fn.m.each @ load-scripts.php?c=0&load[]=jquery-core,jquery-migrate,utils,zxcvbn-async&ver=4.3.1:2
m.fn.extend.trigger @ load-scripts.php?c=0&load[]=jquery-core,jquery-migrate,utils,zxcvbn-async&ver=4.3.1:4
(anonymous function) @ load-scripts.php?c=0&load[]=hoverIntent,common,admin-bar,password-strength-meter,underscore,wp-util…:12
m.event.dispatch @ load-scripts.php?c=0&load[]=jquery-core,jquery-migrate,utils,zxcvbn-async&ver=4.3.1:4
r.handle @ load-scripts.php?c=0&load[]=jquery-core,jquery-migrate,utils,zxcvbn-async&ver=4.3.1:4

2015-12-07 17:01:57.938 load-scripts.php?c=0&l

[peterwilsoncc]

Hi and welcome to trac.

Are you able to provide the following information to help recreate the bug

• steps to recreate the issue
• browser/browsers you are getting the error in

I will look into the problem but the info above will help if I am unable to recreate it locally.

Thanks

[wp-plugin@…]

Chrome ( on Mac OSX )

the user being edited has existed for a long time.

These error show up no matter which user I try to edit.

[peterwilsoncc]

Able to recreate

• In developer tools enable network throttling to emulate a poor network (2G)
• Edit a user
• Before the images/assets have loaded, click the generate password button
• The generated password may remain blank if the JavaScript in the HTML footer is still loading
• Start typing a password
• zxcvbn is undefined and an error will be thrown in the browser console.

Using Chrome 47.0/OSX 10.11.1 the browser recovers once zxcvbn is available and the strength meter starts working.

PROPOSED SOLUTION: wp.passwordStrength.meter fails gracefully if zxcvbn is undefined.

• returns -1 for strength unknown (new string required)
• users need to accept the possibly weak password (string change/new string may be required)

[adamsilverstein]

Replying to peterwilsoncc:

PROPOSED SOLUTION: wp.passwordStrength.meter fails gracefully if zxcvbn is undefined.

Could we have the Generate Password button load disabled, then enable when zxcvbn is ready? That would make it clear to users the button wasn't clickable. Also, I believe we already hide the generate password button if JavaScript is disabled.

[peterwilsoncc]

Replying to adamsilverstein:

Could we have the Generate Password button load disabled, then enable when zxcvbn is ready? That would make it clear to users the button wasn't clickable. Also, I believe we already hide the generate password button if JavaScript is disabled.

Allowing passwords to be entered while zxcvbn loads improves performance. At 323K (gzipped), I think this important for users on low-speed mobile networks.

I'll trial both approaches to allow others to test.

[peterwilsoncc]

This slipped my mind way back when, here's a first pass at solving the race condition on slow networks.

In 34905.diff​:

• wp.passwordStrength.meter fails gracefully if zxcvbn is undefined.
• Password strength indicator warns password strength is unknown
• user needs to accept the use of a potentially week password
• If the user enters a new password prior to the library loading:
  • no password is generated
  • password strength indicator updates once the library is ready

To test, open the network tab in Chrome dev tools, disable caching and throttle the network speed to 2G.

[ocean90]

Maybe related to #36584.

[peterwilsoncc]

#36584 was marked as a duplicate.

[adamsilverstein]

Hi @peterwilsoncc; thanks for the patch.

I tested this using the steps you described and was able to reproduce the issue and verify it is fixed nicely by your patch. I think your approach is sound, however I only tested the patch in Chrome; before this is committed, I think we should perform some additional testing across browsers/devices (including ie8 :( )

Here is a screencast showing the console errors when interacting before the patch:

​http://cl.ly/122c1y1x2w3O

Here is a screencast showing the process after the patch is applied; the field still works and there is no error:

​http://cl.ly/123E1h3M0S3E

[adamsilverstein]

34905.2.diff​ Add a bit of whitespace as per WordPress JS coding standards.

[peterwilsoncc]

In 34905.3.diff​

Shuffles the functionality for showing/hiding the checkbox for confirming weak passwords. In previous editions of the patch the weak password check would not update following the JavaScript loading.

Tested in the following with network link conditioner enabled.

Chrome/OSX
FF/OSX

(Modern.ie)
IE8/Win 7
IE11/Win 7

(Browser Stack)
IE9/Win 7
IE10/Win 7
IE10/Win 8
IE11/Win8.1
Edge 12/Win10
Edge 13/Win10
FF46/Win10

[peterwilsoncc]

In 34905.4.diff​, uses a function call to show or hide the weak password checkbox rather than triggering an event. Checked against IE8.

[markjaquith]

I think this is a good approach. As before, JS just errored, I'm not too worried about the new -1 return value. On any reasonably fast connection, you'll never encounter this. And I like that we're saying "potentially weak" and requiring confirmation when the strength is unknown.

[adamsilverstein]

@peterwilsoncc This patch looks good.

Here is a screencast of the patch in action on chrome, over a throttled 2G connection:

​http://cl.ly/0l172S1x1R0T

Lets get this committed in beta so more people can test it out.

[adamsilverstein]

34905.5.diff​:

• Add a period at the end of comment block
• Add space before comment blocks

[pento]

I think the JS coding standards handbook page is wrong.

"...should always be preceded by a blank line." should really be "...should always be preceded by a blank line, except for at the start of an indented block."

[peterwilsoncc]

34905.5.diff​ wasn't applying cleanly, 34905.6.diff​ is same but refreshed.

I agree with @pento, the line break hinders comprehension at the start of the control structure. The coding standards include examples both with and without a preceding empty line. What fun.

[netweb]

If you were to make any JS coding standards decisions, it would be great if said decision was somewhat based on the ESLint rule that applies to lines around comments: ​http://eslint.org/docs/rules/lines-around-comment

To which I'm thinking based on 34905.7.diff​ would be both of the following:

• "beforeBlockComment": false disallows an empty line before block comments
• "beforeLineComment": true requires an empty line before line comments

There's lots of ESLint rule configs to decide on for #31823

[pento]

It seems that the appropriate ESLint rules would be:

beforeBlockComment: true
beforeLineComment: true
allowBlockStart: true

Anyway, 34905.7.diff​ is good to commit.

[peterwilsoncc]

In 37940:

Users: Check zxcvbn is defined before calling.

Prevents JavaScript errors by checking zxcvbn is defined before calling.

Changes wp.passwordStrength.meter() to return -1 if the strength of the password is unknown.

On the user profile screen, generatePassword() checks if the user has entered the password before setting the value of the password input box.

Props peterwilsoncc, adamsilverstein.
Fixes #34905.

[netweb]

Replying to pento:

It seems that the appropriate ESLint rules would be:

beforeBlockComment: true
beforeLineComment: true
allowBlockStart: true

Thanks @pento :+1