Closed Bug 1560032 Opened 1 year ago Closed 1 year ago

Support cutting and copying from unmasked password fields

Categories

(Core :: DOM: Editor, enhancement, P1)

68 Branch
Desktop
All
enhancement

Tracking

()

VERIFIED FIXED
mozilla70
Tracking Status
firefox70 --- verified

People

(Reporter: MattN, Assigned: masayuki)

References

Details

(Whiteboard: [passwords:generation] [skyline])

Attachments

(3 files)

Follow-up to bug 1560029 to support cutting/copying from unmasked password fields.

Blocks: 1548389
No longer blocks: 1560029
Whiteboard: [passwords:generation] [skyline]

Assigning for tracking purposes

Assignee: nobody → masayuki
Status: NEW → ASSIGNED

Looks like that this is not so difficult to fix once I fix bug 1548389.

No longer blocks: 1548389
Depends on: 1548389
Flags: qe-verify+

It does not make sense to copy masked password with mask characters.
Therefore, we should allow users to copy/cut in password fields only when
selected range is in unmasked range.

Note that for web-compat, copy/cut are always enabled in HTML/XHTML document
in content. Therefore this patch changes the behavior only in chrome's
password fields.

Additionally, only the test uses nsIEditor.canDelete(). Therefore, this
removes it and make the test use nsIDocShell.isCommandEnabled() instead.
Unfortunately, nsIEditor.canCopy() and nsIEditor.canCut() are used by
BlueGriffon, therefore, we cannot get rid of them for now.

First, we need to make nsCopySupport::FireClipboardEvent() keep handling
eCopy and eCut event even in password field, only if TextEditor allows
them.

Then, we need to make nsPlainTextSerializer::AppendText() not expose
masked password for making users safer. Although TextEditor does not allow
eCopy nor eCut when selection is not in unmasked range. Fortunately,
retrieving masked and unmasked password from nsTextFragment has already
been implemented in ContentEventHandler.cpp. This patch moves it into
EditorUtils and makes ContentEventHandler.cpp and nsPlaintextSerializer
share it.

If copying selected password is allowed by TextEditor, we should allow to
drag it too because web apps usually request to input new password twice,
but if users used password generator, they may want to use drag and drop the
new password rather than copy and paste.

Oh, Mirko is also PTO. I'll redirect review to smaug because he will be back sooner.

Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/4a1e400e7077
part 1: Make `TextEditor` for password allow to copy password when selected range is in unmasked range r=m_kato
https://hg.mozilla.org/integration/autoland/rev/8b92e575e1c3
part 2: Make cut/copy in password field available r=m_kato,smaug
https://hg.mozilla.org/integration/autoland/rev/584b03dfe25e
part 3: Make `EventStateManager` allow to drag password if copying selected password is allowed r=smaug

Verified - Fixed on latest Nightly 70.0a1 (2019-07-29) (64-bit) on Windows 10, MacOS 10.14 and Ubuntu 18.04.
The unmasked password from the form and doorhanger can be:

  • copy/pasted
  • cut
  • drag&drop
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Blocks: 1563522
You need to log in before you can comment on or make changes to this bug.