New issue
Advanced search Search tips

Issue 619868 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Task

Blocked on:
issue 623488
issue 628669

Blocking:
issue 617602



Sign in to add a comment

Implement password viewing on Android (code changes)

Project Member Reported by vabr@chromium.org, Jun 14 2016

Issue description

This tracks code changes made to implement password viewing on Android.

For launch approvals, see bug 617602.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 14 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4630cceb0e4cd565ff9b5b95d841587fda1814f5

commit 4630cceb0e4cd565ff9b5b95d841587fda1814f5
Author: dozsa <dozsa@google.com>
Date: Tue Jun 14 10:39:12 2016

Add base::Feature to protect password viewing on Android

This CL adds a base::Feature "view-passwords" to guard the ability to view and copy passwords in Chrome on Android settings.

Note: There is no associated about::flags entry because it is crucial that the feature not be enabled by non-developers before it is complete.

BUG= 619868 

Review-Url: https://codereview.chromium.org/2069553002
Cr-Commit-Position: refs/heads/master@{#399679}

[modify] https://crrev.com/4630cceb0e4cd565ff9b5b95d841587fda1814f5/components/password_manager/core/common/password_manager_features.cc
[modify] https://crrev.com/4630cceb0e4cd565ff9b5b95d841587fda1814f5/components/password_manager/core/common/password_manager_features.h

Project Member

Comment 2 by bugdroid1@chromium.org, Jun 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4630cceb0e4cd565ff9b5b95d841587fda1814f5

commit 4630cceb0e4cd565ff9b5b95d841587fda1814f5
Author: dozsa <dozsa@google.com>
Date: Tue Jun 14 10:39:12 2016

Add base::Feature to protect password viewing on Android

This CL adds a base::Feature "view-passwords" to guard the ability to view and copy passwords in Chrome on Android settings.

Note: There is no associated about::flags entry because it is crucial that the feature not be enabled by non-developers before it is complete.

BUG= 619868 

Review-Url: https://codereview.chromium.org/2069553002
Cr-Commit-Position: refs/heads/master@{#399679}

[modify] https://crrev.com/4630cceb0e4cd565ff9b5b95d841587fda1814f5/components/password_manager/core/common/password_manager_features.cc
[modify] https://crrev.com/4630cceb0e4cd565ff9b5b95d841587fda1814f5/components/password_manager/core/common/password_manager_features.h

Comment 3 by dozsa@google.com, Jun 23 2016

Screenshots for reauthentication button implementation.
password_entry_editor_with_reauthentication_button.png
56.1 KB View Download
password_entry_editor_device_lock_confirmation.png
58.7 KB View Download

Comment 4 by vabr@chromium.org, Jun 27 2016

Blockedon: 623488

Comment 5 by dozsa@google.com, Jul 15 2016

Blockedon: 628669
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 10 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/04e1ab8bc32ae548c07791a27a62c87eb6961097

commit 04e1ab8bc32ae548c07791a27a62c87eb6961097
Author: dozsa <dozsa@google.com>
Date: Wed Aug 10 10:10:47 2016

Modify PasswordEntryEditor layout to remove warnings

This CL removes two warnings generated by the changes made to PasswordEntryEditor:
chrome/android/java/res/layout/password_entry_editor.xml:68 Cancel button should be on the left (was "Delete | Cancel", should be "Cancel | Delete"): ButtonOrder [warning]
chrome/android/java/res/layout/password_entry_editor_interactive.xml:89 [Accessibility] Missing `contentDescription` attribute on image: ContentDescription [warning]

BUG= 619868 

Review-Url: https://codereview.chromium.org/2221373002
Cr-Commit-Position: refs/heads/master@{#411017}

[modify] https://crrev.com/04e1ab8bc32ae548c07791a27a62c87eb6961097/chrome/android/java/res/layout/password_entry_editor.xml
[modify] https://crrev.com/04e1ab8bc32ae548c07791a27a62c87eb6961097/chrome/android/java/res/layout/password_entry_editor_interactive.xml

Project Member

Comment 8 by sheriffbot@chromium.org, Feb 27 2017

Labels: Hotlist-Recharge-BouncingOwner
Owner: ----
Status: Untriaged (was: Started)
The assigned owner "dozsa@google.com" is not able to receive e-mails, please re-triage.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 9 by vabr@chromium.org, Feb 27 2017

Owner: vabr@chromium.org
Status: Assigned (was: Untriaged)
Assigning to vabr@ who currently owns the launch bug 617602. If you are interested in owning this, let vabr@ know.

Comment 10 by vabr@chromium.org, May 2 2017

Labels: -tracking_work Type-Task
Vaclav, do you mind if I'll try to take care of this.

I looked at the code at it is in a decent shape.

The important thing which is missing is deletion (see screenshot attached). But I think that if we'll just add the delete button similarly to the current state, this new UI and ability too view/copy password still will be massive improvement.
Screenshot_20170523-221049.png
60.5 KB View Download
Cc: kolos@chromium.org
Adding kolos@, since we've recently discussed this feature.
Screen shots without "Delete" button: https://folio.googleplex.com/copy-and-view-passwords-on-clank

Comment 14 by kolos@chromium.org, May 24 2017

To me, the link in #13 doesn't work. "Error: file not found."
Cc: melandory@chromium.org
Where's the mock you're implementing?  Also, the link in #13 is broken.
Please ping the review when you've addressed #16.

Comment 19 by vabr@chromium.org, Jun 4 2017

Ad #11 -- melandory@, please feel free to take this one! :)
Sorry for a late response, I have been OOO through 5 June (coming back on 6).
Project Member

Comment 20 by bugdroid1@chromium.org, Jun 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2a595fea49f5e6ed6ed9caae2b954d25798226ba

commit 2a595fea49f5e6ed6ed9caae2b954d25798226ba
Author: melandory <melandory@chromium.org>
Date: Tue Jun 13 08:15:41 2017

Copy and view saved passwords.

Implements the ability to copy and view saved passwords if the user has
a lock. Before allow view or copy of passwords user is prompted with the
lock. The functionality is unavailable if there is no device level
lock.

The implementation in this CL has following shortcomings:
* "Nice viewer" is not available for the users without lock, still old version is used.
* There is no way to delete the credentials in the new view.
* In case password was viewed (eye icon clicked), the app switcher thumbnail contains the password in plain text.
* Wrong lock screen message.
* UI is not aligned with mocks.

Screenshots are attached to the bug.

The original CL: https://codereview.chromium.org/2067323004/

BUG= 619868 

Review-Url: https://codereview.chromium.org/2900173002
Cr-Commit-Position: refs/heads/master@{#478929}

[add] https://crrev.com/2a595fea49f5e6ed6ed9caae2b954d25798226ba/chrome/android/java/res/drawable-hdpi/ic_lock.png
[add] https://crrev.com/2a595fea49f5e6ed6ed9caae2b954d25798226ba/chrome/android/java/res/drawable-hdpi/ic_visibility_off.png
[add] https://crrev.com/2a595fea49f5e6ed6ed9caae2b954d25798226ba/chrome/android/java/res/drawable-mdpi/ic_lock.png
[add] https://crrev.com/2a595fea49f5e6ed6ed9caae2b954d25798226ba/chrome/android/java/res/drawable-mdpi/ic_visibility_off.png
[add] https://crrev.com/2a595fea49f5e6ed6ed9caae2b954d25798226ba/chrome/android/java/res/drawable-xhdpi/ic_lock.png
[add] https://crrev.com/2a595fea49f5e6ed6ed9caae2b954d25798226ba/chrome/android/java/res/drawable-xhdpi/ic_visibility_off.png
[add] https://crrev.com/2a595fea49f5e6ed6ed9caae2b954d25798226ba/chrome/android/java/res/drawable-xxhdpi/ic_lock.png
[add] https://crrev.com/2a595fea49f5e6ed6ed9caae2b954d25798226ba/chrome/android/java/res/drawable-xxhdpi/ic_visibility_off.png
[add] https://crrev.com/2a595fea49f5e6ed6ed9caae2b954d25798226ba/chrome/android/java/res/drawable-xxxhdpi/ic_lock.png
[add] https://crrev.com/2a595fea49f5e6ed6ed9caae2b954d25798226ba/chrome/android/java/res/drawable-xxxhdpi/ic_visibility_off.png
[add] https://crrev.com/2a595fea49f5e6ed6ed9caae2b954d25798226ba/chrome/android/java/res/layout/fragment_lock_screen.xml
[modify] https://crrev.com/2a595fea49f5e6ed6ed9caae2b954d25798226ba/chrome/android/java/res/layout/password_entry_editor.xml
[modify] https://crrev.com/2a595fea49f5e6ed6ed9caae2b954d25798226ba/chrome/android/java/res/layout/password_entry_editor_interactive.xml
[modify] https://crrev.com/2a595fea49f5e6ed6ed9caae2b954d25798226ba/chrome/android/java/src/org/chromium/chrome/browser/PasswordUIView.java
[modify] https://crrev.com/2a595fea49f5e6ed6ed9caae2b954d25798226ba/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordEntryEditor.java
[add] https://crrev.com/2a595fea49f5e6ed6ed9caae2b954d25798226ba/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordReauthenticationFragment.java
[modify] https://crrev.com/2a595fea49f5e6ed6ed9caae2b954d25798226ba/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java
[modify] https://crrev.com/2a595fea49f5e6ed6ed9caae2b954d25798226ba/chrome/android/java/strings/android_chrome_strings.grd
[modify] https://crrev.com/2a595fea49f5e6ed6ed9caae2b954d25798226ba/chrome/android/java_sources.gni
[add] https://crrev.com/2a595fea49f5e6ed6ed9caae2b954d25798226ba/chrome/android/junit/src/org/chromium/chrome/browser/preferences/password/PasswordReauthenticationFragmentTest.java
[modify] https://crrev.com/2a595fea49f5e6ed6ed9caae2b954d25798226ba/chrome/browser/android/password_ui_view_android.cc

Comment 21 by vabr@chromium.org, Jun 13 2017

Cc: -melandory@chromium.org vabr@chromium.org
Owner: melandory@chromium.org
Moving to melandory@, who is working on this.
Screenshot_2017-07-07-16-30-24.png
34.2 KB View Download
Screenshot_2017-07-07-12-09-25.png
48.7 KB View Download
Project Member

Comment 24 by bugdroid1@chromium.org, Jul 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/150b9af0520f20c883316373da0b6cb87124e075

commit 150b9af0520f20c883316373da0b6cb87124e075
Author: Tatiana Gornak <melandory@chromium.org>
Date: Wed Jul 26 10:47:20 2017

Refactor new password manager viewer for Clank.

1. Reuse xmls more activelly.
2. Change strings and appearances, to make them closer to mocks.
3. Better separation between old and new views. This resulted into some code
duplication. But will make removal of old code simpler.

BUG= 619868 

Change-Id: Ied17c41e96606e07d8ebba64da6dba3ccfd627d7
Reviewed-on: https://chromium-review.googlesource.com/570398
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Tatiana Gornak <melandory@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489600}
[add] https://crrev.com/150b9af0520f20c883316373da0b6cb87124e075/chrome/android/java/res/layout/password_entry_editor_copyable_row.xml
[modify] https://crrev.com/150b9af0520f20c883316373da0b6cb87124e075/chrome/android/java/res/layout/password_entry_editor_interactive.xml
[add] https://crrev.com/150b9af0520f20c883316373da0b6cb87124e075/chrome/android/java/res/layout/password_entry_editor_row_title.xml
[delete] https://crrev.com/fcc7e00950ae4212bee0ba4967466683d38d1cbf/chrome/android/java/res/layout/password_entry_editor_site_row.xml
[modify] https://crrev.com/150b9af0520f20c883316373da0b6cb87124e075/chrome/android/java/res/layout/password_entry_exception.xml
[modify] https://crrev.com/150b9af0520f20c883316373da0b6cb87124e075/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordEntryEditor.java
[modify] https://crrev.com/150b9af0520f20c883316373da0b6cb87124e075/chrome/android/java/strings/android_chrome_strings.grd

Project Member

Comment 25 by bugdroid1@chromium.org, Jul 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/38ab356c22837d09c3abca5d8ac13cced2db2a51

commit 38ab356c22837d09c3abca5d8ac13cced2db2a51
Author: Tatiana Gornak <melandory@chromium.org>
Date: Fri Jul 28 11:04:18 2017

Add flag for passwords viewing and copying for Android.

This CL adds flag to chrome://flags for controlling copying and viewing
functionality for the password manager settings page.

BUG= 619868 

Change-Id: I7353c17d6e40d6e584780d033555fbe0073f1a3c
Reviewed-on: https://chromium-review.googlesource.com/586606
Reviewed-by: Maxim Kolosovskiy <kolos@chromium.org>
Commit-Queue: Tatiana Gornak <melandory@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490357}
[modify] https://crrev.com/38ab356c22837d09c3abca5d8ac13cced2db2a51/chrome/browser/about_flags.cc
[modify] https://crrev.com/38ab356c22837d09c3abca5d8ac13cced2db2a51/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/38ab356c22837d09c3abca5d8ac13cced2db2a51/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/38ab356c22837d09c3abca5d8ac13cced2db2a51/tools/metrics/histograms/enums.xml

Project Member

Comment 26 by bugdroid1@chromium.org, Aug 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6e6baebec871c08d520cab842a1b5f9b3bef155d

commit 6e6baebec871c08d520cab842a1b5f9b3bef155d
Author: Tatiana Gornak <melandory@chromium.org>
Date: Tue Aug 01 10:03:42 2017

Native UI for the password manager settings page for every user.

As per latest decision all users should use native password manager
setting page instead of being redirected to the passwords.google.com

BUG= 619868 

Change-Id: I668722d4be34b605517bd846e071779e56f61346
Reviewed-on: https://chromium-review.googlesource.com/587187
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Tatiana Gornak <melandory@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490933}
[modify] https://crrev.com/6e6baebec871c08d520cab842a1b5f9b3bef155d/chrome/android/java/src/org/chromium/chrome/browser/preferences/MainPreferences.java
[modify] https://crrev.com/6e6baebec871c08d520cab842a1b5f9b3bef155d/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/PasswordViewingTypeTest.java

Project Member

Comment 27 by bugdroid1@chromium.org, Aug 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8cbfd809286859cdb1d87509ba3780e39f094649

commit 8cbfd809286859cdb1d87509ba3780e39f094649
Author: Tatiana Gornak <melandory@chromium.org>
Date: Tue Aug 08 16:30:24 2017

Improve the privacy of displaying saved passwords.

After this CL:
1. If the user viewed the password, it will be not possible to take
screenshot of the page.
2. If the user viewed the password and entered app switcher, then the
password will be masked on a screenshot.

BUG= 619868 , 751084

Change-Id: I4c818639740d135f5a806a802a7769bce3c7d48c
Reviewed-on: https://chromium-review.googlesource.com/605808
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Tatiana Gornak <melandory@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492651}
[modify] https://crrev.com/8cbfd809286859cdb1d87509ba3780e39f094649/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordEntryEditor.java

Project Member

Comment 28 by bugdroid1@chromium.org, Aug 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b04a7d33fa79a8acf5872af2f5d003de163efb1d

commit b04a7d33fa79a8acf5872af2f5d003de163efb1d
Author: Tatiana Gornak <melandory@chromium.org>
Date: Wed Aug 09 12:20:55 2017

Enable new Android password viewer by default.

BUG= 619868 

Change-Id: I6d3eeeaca8b6b3f800a3c794e6efb252bb2c6ec0
Reviewed-on: https://chromium-review.googlesource.com/608027
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Tatiana Gornak <melandory@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492949}
[modify] https://crrev.com/b04a7d33fa79a8acf5872af2f5d003de163efb1d/components/password_manager/core/common/password_manager_features.cc

Project Member

Comment 29 by bugdroid1@chromium.org, Aug 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c46e99c246e9a2905507284d3f4eba28c4d4c2dd

commit c46e99c246e9a2905507284d3f4eba28c4d4c2dd
Author: Tatiana Gornak <melandory@chromium.org>
Date: Thu Aug 17 22:11:32 2017

Incorporate UI team feedback for the Password Manager credentials viewer.

* Make font for title of a settings page smaller
* Change spacing and alignment between elements

BUG= 619868 

Change-Id: I9215a3414bcdf2d364dcfdc1383061cfdfbb83c5
Reviewed-on: https://chromium-review.googlesource.com/618573
Commit-Queue: Tatiana Gornak <melandory@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Nicolas Dossou-Gbété <dgn@chromium.org>
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495344}
[modify] https://crrev.com/c46e99c246e9a2905507284d3f4eba28c4d4c2dd/chrome/android/java/res/layout/password_entry_editor_copyable_row.xml
[modify] https://crrev.com/c46e99c246e9a2905507284d3f4eba28c4d4c2dd/chrome/android/java/res/layout/password_entry_editor_interactive.xml
[delete] https://crrev.com/6be43e7f8e0806cf773ffdd1adb91811740c9ed3/chrome/android/java/res/layout/password_entry_editor_row_title.xml
[modify] https://crrev.com/c46e99c246e9a2905507284d3f4eba28c4d4c2dd/chrome/android/java/res/layout/password_entry_exception.xml
[modify] https://crrev.com/c46e99c246e9a2905507284d3f4eba28c4d4c2dd/chrome/android/java/res/values-v17/styles.xml
[modify] https://crrev.com/c46e99c246e9a2905507284d3f4eba28c4d4c2dd/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordEntryEditor.java
[modify] https://crrev.com/c46e99c246e9a2905507284d3f4eba28c4d4c2dd/chrome/android/java/strings/android_chrome_strings.grd

Project Member

Comment 30 by bugdroid1@chromium.org, Aug 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0ae1a5d38aa08895e579f1b87263fb2a13c19612

commit 0ae1a5d38aa08895e579f1b87263fb2a13c19612
Author: jdoerrie <jdoerrie@chromium.org>
Date: Fri Aug 25 06:12:52 2017

Add UMA Metrics to Android Password Entry Editor

This change adds UMA metrics for the new password viewing on Android
feature.

Bug:  619868 
Change-Id: I6c42e0462ff992deb2fffe7d46bb6e16ce4822a4
Reviewed-on: https://chromium-review.googlesource.com/623677
Reviewed-by: Theresa <twellington@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Tatiana Gornak <melandory@chromium.org>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497333}
[modify] https://crrev.com/0ae1a5d38aa08895e579f1b87263fb2a13c19612/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordEntryEditor.java
[modify] https://crrev.com/0ae1a5d38aa08895e579f1b87263fb2a13c19612/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/0ae1a5d38aa08895e579f1b87263fb2a13c19612/tools/metrics/histograms/histograms.xml

Project Member

Comment 31 by bugdroid1@chromium.org, Aug 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2aff59932c90f064866fe07b74d0d10effd29bdd

commit 2aff59932c90f064866fe07b74d0d10effd29bdd
Author: Tatiana Gornak <melandory@chromium.org>
Date: Thu Aug 31 23:00:59 2017

Correct content description for hiding/viewing passwords.

BUG=744986,  619868 

Change-Id: I84cb14db5bab5ef3886b318627d50956f35f53f6
Reviewed-on: https://chromium-review.googlesource.com/645267
Commit-Queue: Tatiana Gornak <melandory@chromium.org>
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499060}
[modify] https://crrev.com/2aff59932c90f064866fe07b74d0d10effd29bdd/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordEntryEditor.java
[modify] https://crrev.com/2aff59932c90f064866fe07b74d0d10effd29bdd/chrome/android/java/strings/android_chrome_strings.grd

Status: Fixed (was: Assigned)
Project Member

Comment 33 by bugdroid1@chromium.org, Nov 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0636ea130f56e8351de4c1e29f466b5ee28f5218

commit 0636ea130f56e8351de4c1e29f466b5ee28f5218
Author: Tatiana Gornak <melandory@chromium.org>
Date: Fri Nov 24 13:10:30 2017

Ditch the old code for displaying credentials in settings for Clank

Also adds recording for deletion histogram

BUG= 619868 

Change-Id: I6dc7ba9b596605ba0773a43240c82df7acf84b88
Reviewed-on: https://chromium-review.googlesource.com/666624
Commit-Queue: Tatiana Gornak <melandory@chromium.org>
Reviewed-by: Nicolas Dossou-Gbété <dgn@chromium.org>
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519096}
[delete] https://crrev.com/e247e4076e4539bd15874f59ec264c500b1ebca2/chrome/android/java/res/layout/password_entry_editor.xml
[modify] https://crrev.com/0636ea130f56e8351de4c1e29f466b5ee28f5218/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordEntryEditor.java

Cc: -vabr@chromium.org

Sign in to add a comment