Don't include leading and trailing spaces in password strength

Here is patch added and fixed point: Length of indicator bar remains at about 25% and rating remains as Weak if either leading or trailing spaces is added. These extra spaces won't consider to calculate password strength.

Yes Charles, I will create a single function to manage both cases. I will upload the next patch soon.

I have reviewed my code once again and found only following code can help in password strength checker. Because we have trimmed password value before passing in Drupal.evaluatePasswordStrength() function by
$.trim(passwordInput.val()).

After applying this code, I test this by creating new users and also edit existing users and its working fine as required.

Say I created a user with name "test", when user enter leading&trailing spaces in password fields then password strength checker dint comes into place. And at the time of saving in database, these leading&trailing spaces automatically get trimmed through module file. It means during password saver we don't required any function to deal this. So can we go with this single line or I need to write some other logic. Please correct me if I am wrong.

// Check the password strength.
        var passwordCheck = function () {
          if (settings.password.showStrengthIndicator) {
            // Evaluate the password strength.
            var result = Drupal.evaluatePasswordStrength($.trim(passwordInput.val()), settings.password);

            // Update the suggestions for how to improve the password.
            if (passwordDescription.html() !== result.message) {
              passwordDescription.html(result.message);
            }

            // Only show the description box if a weakness exists in the password.
            passwordDescription.toggle(result.strength !== 100);

            // Adjust the length of the strength indicator.
            innerWrapper.find('.indicator')
              .css('width', result.strength + '%')
              .css('background-color', result.indicatorColor);

            // Update the strength indication text.
            innerWrapper.find('.password-strength-text').html(result.indicatorText);
          }

          // Check the value in the confirm input and show results.
          if (confirmInput.val()) {
            passwordCheckMatch(confirmInput.val());
            confirmResult.css({ visibility: 'visible' });
          }
          else {
            confirmResult.css({ visibility: 'hidden' });
          }
        };

Why not permit leading/trailing spaces in the password? Then there need be no special logic in the password-strength indicator, and users get the passwords they expect when they enter them.

I suppose some users might accidentally put spaces before or after their passwords, or assume that doing so has no effect. So it could also be confusing for some users if spaces are kept.

It seems it can be confusing no matter the password-strength indication (a problem probably more relevant to #1921576 than this issue).

Instead of changing this in two different places in the code I would just add this line in Drupal.evaluatePasswordStrength function:

…   
    var username = (usernameBox.length > 0) ? usernameBox.val() : translate.username;

    password = password.trim();

    // Lose 5 points for every character less than 6, plus a 30 point penalty.
    if (password.length < 6) {
      msg.push(translate.tooShort);
      strength -= ((6 - password.length) * 5) + 30;
    }
…

And we can use the native trim function instead of using jQuery.

Also for reference if any issue is going to touch a javascript file can you please add the JavaScript tag to it so that the JavaScript team can monitor and review issues? Otherwise those kind of issues get lost in the queue.

Working fine

indentation

There is an eslint error:

core/modules/user/user.js
  114:14  error  Multiple spaces found before '='  no-multi-spaces

✖ 1 problem (1 error, 0 warnings)

Also usually when we change parameters like that we put this sort of thing at the very top of the function, before anything else gets done.

Could you move the new line at the begining please?

Right before var indicatorText; around line 112.

Worked as expected Length of indicator bar remains at about 25% and rating remains as Weak.

Attaching screencast after applying patch.

Also usually when we change parameters like that we put this sort of thing at the very top of the function, before anything else gets done.

Do you know an example of this offhand? It seems a little odd to me to call a function before variable declarations, though I am not an experienced JavaScript developer.

I see the logic of the suggestion--you look at the function parameters and can immediately see that parameter is being trimmed.

In any case, here is an updated patch that moves the line as you suggest. But the patch in #21 seems fine to me too.

Edit: I see now that nod_ is Drupal JavaScript maintainer so I defer to his judgment on the placement.

Update to apply on latest code.

Is it a problem that trim() was introduced only in IE 9?

Edit: No, IE 9 or greater is required for Drupal 8 per https://www.drupal.org/node/61509.

Simple fix should not take such long time.

Back to RTBC per #28

Committed 4af9619 and pushed to 8.0.x. Thanks!

From the issue summary it looks like this was originally discovered as a Drupal 7 issue.

Drupal 7 requires IE 8 at minimum, but trim() was introduced in IE 9. So this patch uses replace() to accomplish nearly the same.*

*trim() actually removes some other obscure whitespace characters. See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global..., which gives polyfill code for trim(). I doubt we care about them, but I have not investigated.

D7 requires IE6+

jQuery.trim is your friend.

Thanks. I removed the comment since it is more obvious what the line is doing. Maybe it would be worth commenting why we are using jQuery.trim, though?

D7 requires IE6+

This "Browser Requirements" page seems to suggest IE 8 is the minimum:
https://www.drupal.org/node/61509

Do you know a source on 6 being the minimum supported IE version for Drupal 7? I don't think it matters for this issue, but it would be helpful to know.

It's officially dropped in D8:
https://www.drupal.org/node/1569578

Committed to 7.x - thanks!