Make WordPress Core

Opened 9 years ago

Closed 9 years ago

#33601 closed defect (bug) (duplicate)

Pasting a password on an iOS device doesn't work properly

Reported by: magicroundabout's profile magicroundabout Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.3
Component: Users Keywords:
Focuses: javascript Cc:

Description

I've noticed an issue with pasting a password into a user profile on an iOS device.

I've replicated this on 4.3 and the latest nightly (4.4-alpha-33636)

Here's a (slightly slow and awkward) video showing the problem: http://www.youtube.com/watch?v=dq1xt_ahbL4

The basic steps to replicate this are:

  • Edit a user profile on an iOS device
  • Copy a password from somewhere into the iOS clipboard
  • Tap the "generate password" button
  • Use the iOS Select All and Paste function to add your own password into the box
  • Click the "Update User" button
  • Open a new, logged-out browser window and attempt to log in as that user

This can be overcome by using the iOS keyboard to edit the password after you've pasted it in.

My guess was that a keyup or update event was triggering a fill of some hidden fields. This seems to be the case. The one displayed field has name "pass1-text". This populates two hidden fields called "pass1" and "pass2". The back-end code uses the values of pass1 and/or pass2 to actually update the password's value.

There's two issues here.

The first and most obvious is that the paste event isn't triggering the update of pass1 and pass2. This LOOKS like it could be fixed by changing line 50 of user-profile.js to be:

.on('keyup paste', function () {

BUT I tried this and it turns out the paste event fires before the input's .val() is updated. There's numerous solutions to this but they all feel pretty hacky (see http://stackoverflow.com/questions/2176861/javascript-get-clipboard-data-on-paste-event-cross-browser for example). I could have a go at a patch, but I suspect it needs input from someone who's better at JS than I am.

The second issue is that the password SEEMS to be changed in the back-end, event if the submitted pass1-text is different to the values of pass1 and pass2.

In the case of the failed paste, the POST'ed pass1 and pass2 have the value that was populated when I clicked "Generate password", but pass1-text is different - it has the value I pasted in. I don't know if you want an additional validation check here? Or maybe it's OK as long as we sort out the JS and population of pass1 and pass2?

Change History (12)

#1 @magicroundabout
9 years ago

So...just a quick instant update on this.

I did try to create a patch for this that involved adding the paste event handler. This fixed the issue on the iPad, but this got complicated because back in Chrome on my laptop, the event was firing TWICE: once for paste and once for keyup, and it was getting pretty confusing.

I took a look at #31226 which seemed to fix this for the old password strength meter in 4.1. In that ticket @helen mentioned the setTimeout needed to catch the value updating from the paste event, but it was ultimately fixed by @dipesh.kakadiya changing the event handler from keyup to input propertychange. I wonder if there's any reason we've reverted from that back to keyup?

#2 @magicroundabout
9 years ago

Oh. Ah.

Looks like the handler was reverted to keyup in #33385 just prior to 4.3 release because of issues with IE8.

Looks like this might need input from a cross-platform JS events guru. That's not me I'm afraid. :(

Last edited 9 years ago by magicroundabout (previous) (diff)

#3 @magicroundabout
9 years ago

Just confirmed that #33385 undid the work of #31226. Pasting a password using right-click -> paste doesn't work either. It doesn't even update the strength meter.

Simpler steps to replicate on a laptop without the fuss of iOS:

  • Copy a weak password to the clipboard
  • Edit a user
  • Click the "Generate password" button
  • Right-click and paste the password in
  • Note that the password strength meter doesn't update

I guess you could say that given the new password functionality, why would I want to paste a password in? But, well, I wanted to do exactly that (my use case was wanting to set a password that I already had stored in a password manager).

#4 @SergeyBiryukov
9 years ago

  • Milestone changed from Awaiting Review to 4.3.1

#5 @peterwilsoncc
9 years ago

  • Focuses javascript added
  • Keywords has-patch added
  • Version changed from trunk to 4.3

Feature detecting the best event to use, see 33398.1.diff:ticket:33398, fixes this for touch devices:input event is preferred over keyup.

#6 @peterwilsoncc
9 years ago

  • Keywords has-patch removed

This ticket was mentioned in Slack in #core-passwords by sam. View the logs.


9 years ago

#8 follow-up: @adamsilverstein
9 years ago

@magicroundabout:

Thanks for the bug report.

The latest patch on #33398 should resolve this issue. Can you git it a try and let us know if it fixes the issue?

Thanks!

This ticket was mentioned in Slack in #core-passwords by adamsilverstein. View the logs.


9 years ago

#10 in reply to: ↑ 8 @samuelsidler
9 years ago

Replying to adamsilverstein:

The latest patch on #33398 should resolve this issue. Can you git it a try and let us know if it fixes the issue?

Assuming we hear back that it does resolve this issue, we should dupe this ticket to #33398.

This ticket was mentioned in Slack in #core-passwords by sam. View the logs.


9 years ago

#12 @ocean90
9 years ago

  • Milestone 4.3.1 deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #33398.

Note: See TracTickets for help on using tickets.