user-profile.js causing Javascript error in user-new.php

[pbiron]

​Line 53 of user-profile.dev.js is causing a Javascript error in user-new.php.

The problem is a result of the fact that user-new.php does NOT include include a "nickname" field. Hence, ​Line 40 results in inputs['display_nickname'] being undefined...and as a result ​Line 53

var val = value.replace(/<\/?[a-z][^>]*>/gi, '');

causes a JS error since value is undefined when id == 'display_nickname'.

I can think of at least 3 possible fixes:

1. add a "nickname" field to user-new.php
2. replace ​Line 40 with something like display_nickname : $('#nickname').val() ? $('#nickname').val () : '',
3. wrap lines 53-62 of user-profile.dev.js in if (undefined != value) {}

The 3rd solution seems the most general (and best) to me, since it future-proofs user-profile.js for contexts where any of the fields might not be present. However, I'd really like a 2nd opinion on this before submitting a patch.

[SergeyBiryukov]

I guess I would add

if ( !value )
	return;

before line 53 (similar to point 3).

[pbiron]

​@sergey: that would work as well, and is probably easier to read.

[azaozz]

In [20407]:

Don't try to append "Display Name" options when on the Add New User screen (the <select> doesn't exist there), props pbiron, SergeyBiryukov, fixes #20392

[nacin]

We should probably avoid this to begin with: $('#first_name, #last_name, #nickname').blur( if we're not on the page for it. How much in user-profile.js is the same?

[azaozz]

Replying to nacin:

Yes, can check if that <select> exists first. The pass strength JS is in the same file and is used on the Add New User screen.

[SergeyBiryukov]

return seems a bit more readable than a long if block.

20392.2.patch​ also includes some whitespace fixes as per the coding standards.

[azaozz]

Replying to SergeyBiryukov:

return seems a bit more readable than a long if block.

Yes but this is in an anonymous wrapper function that does several things. If it's ever extended to include another jQuery chain, we will have to be careful to add it above or below that return depending where it should run. That seems clearer with an if() block imho.

[SergeyBiryukov]

Replying to azaozz:

If it's ever extended to include another jQuery chain, we will have to be careful to add it above or below that return depending where it should run.

I see, thanks for the explanation. 20392.3.patch​ builds on 20392.patch​ with just some minor whitespace fixes, while we're at it.

[azaozz]

In [20431]:

Add New User screen: don't bind to blur event on the user_name fields when the <select> doesn't exist, props SergeyBiryukov, fixes #20392