Better wp namespace in password-strength-meter.js

[afercia]

Currently, password-strength-meter.js uses this pattern to pass the wp object as argument:

window.wp = window.wp || {};

var passwordStrength;
(function($){
	wp.passwordStrength = {
...

})(jQuery);

so wp inside the IIFE is, technically, undefined, even if WordPress considers wp a global object. Also the first line is pointless if then window.wp is not passed as argument.

[andg]

Invoking a typeof on the wp variable in the IIFE actually returns "object", not "undefined", since any variable not otherwise found is attributed to the global window object by default by the browser.

But yeah, passing it as a second argument would remove all uncertainties, and would probably be a better approach overall.

[afercia]

See 38806#comment:2.

[lemacarl]

@afercia Removed window.wp = window.wp || {}; since its pointless

[audrasjb]

I tested the patch and it still applies cleanly so I'm removing the needs-refresh keyword, but I'm adding the needs-testing keyword as we need to test this thoroughly to make sure it doesn't introduce any regression.

[Boniu91]

@audrasjb In this ticket, we should only test regressions related to password strength meter feature, right?

[dkotter]

Tested the following scenarios today with this patch and everything worked as expected:

• Setting a password during the install steps
• Changing the password of my user account
• Setting a password when adding a new user
• Changing the password of that user after they've been added
• Changing passwords of my account and another user account on a multisite install

[audrasjb]

As per today's bug scrub:
The patch still applies cleanly against trunk.
Given it was tested successfully, it looks like it safe to go to the next step: get a review from a committer then commit. @davidbaumwald kindly proposed to review the proposed patch.

[davidbaumwald]

After looking through the codebase a bit at similar admin scripts, the window.wp = window.wp || {}; pattern is common. However, the wp variable is aliased inside the IIFE.

Rather than removing the the window.wp = window.wp || {}; line, I think it's probably better to just pass wp to the IIFE. Updated patch incoming.

Adding 2nd-opinion here to see if this is the right approach, or should all other instances of this pattern be revisited.

[davidbaumwald]

Define wp in the IIFE

[hellofromTonya]

window.wp is a global,wp is available within the IIFE (as IIFEs can access global variables). ​See it in action here. I'm not seeing a bug with this implementation, though readability is not the best IMO.

There are multiple types of implementations in the core scripts including the design pattern in the password strength script, injecting window into the IIFE as another param (along with jQuery), and setting the global within the IIFE.

It would be nice to have a consistent design approach across all of these scripts, though outside the scope of this specific ticket. Maybe a new ticket can be opened to explore that type of enhancement.

As the IIFE works with global variables, marking this ticket as a close candidate and moving to Future Release.