jdoerrie@: will you have a chance to fix ugly Android credentials in new settings? You know MD settings well. This fix will probably be very easy for you. I am glad to explain backend code or current settings code, if you need.

Yes, I'm glad to have a look. I will probably contact you offline in case I have questions regarding the backend.

Thanks. Questions are welcome.

Screenshot for Code Review

Deleted: Android_Credential_MD_Settings.png 30.2 KB

	Android_Credential_MD_Settings.png 30.2 KB View Download

jdoerrie@: what's the status of this bug?

so the RTL screenshot posted doesn't look like the () are correct, and I think using https://developer.mozilla.org/en-US/docs/Web/CSS/unicode-bidi could probably fix

dbeam@:

I started working on this bug in http://crrev.com/2651663003, but ran into the Android issue screenshotted above. In the following discussion hcarmona@ suggested getting inspired by Autofill, similarly to this: http://screen/dQpS9auNZvR

I reached out to an UX designer who suggested the proposal shown in #c7, but I haven't followed up on it yet.

Furthermore, it would be nice to be able to display the PlayStore name of the Android app credentials are associated with. Currently we only support credentials that have |affiliated_web_realm| set (https://cs.chromium.org/chromium/src/components/autofill/core/common/password_form.h?l=121). To my understanding that currently only applies to apps that opted into Smartlock (https://developers.google.com/identity/smartlock-passwords/android/associate-apps-and-sites), for example Netflix.

Mapping app identifiers to PlayStore names requires architectural changes, which is why this wasn't done yet.

However, I agree that for the mean time we should probably at least have feature parity between the old settings and MD settings and therefore not wait for PlayStore names. This however raises the question how to communicate that Android credentials have an associated web realm with JavaScript. 

In the old settings we simply add the appropriate fields to the DictionaryValue that is shared with JS (https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/options/password_manager_handler.cc?l=257), but this is incompatible with the IDL used for MD settings (https://cs.chromium.org/chromium/src/chrome/common/extensions/api/passwords_private.idl) and would probably require changes there.

In http://crrev.com/2651663003 I so far simply modified |entry.login_pair.origin_url|, but I don't think we should go as far and add HTML markup such as '<bdo>' there. It would be much cleaner to do this in HTML and JS only, with the appropriate metadata passed through the IDL.

What do you think is the cleanest way here, dbeam@?

yeah, if we're dropping something by shipping the Chrome 59 version of settings this isn't good.

hcarmona@: please lead jdoerrie@ to a mergeable solution

 Issue 707296  has been merged into this issue.

Screenshots for Code Review.

Deleted: Android_RTL.png 49.4 KB

	Android_RTL.png 49.4 KB View Download

Deleted: Android_LTR.png 59.1 KB

	Android_LTR.png 59.1 KB View Download

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

commit f120a871d23619bd90c973bc25b0007ab834c0c1
Author: jdoerrie <jdoerrie@chromium.org>
Date: Mon Apr 24 16:53:24 2017

Show human readable origin for Android apps

Prior to this change Android credentials were displayed in a human
unfriendly way, e.g. "android://com.nytimes.android/". This change
addresses this issue by trying to obtain the affiliated name, while
still making clear it is an Android credential.

This requires a change to the extensions API. In order to have a user
friendly UI three URLs are transmitted from the backend:

- origin: This URL comes straight from the password store and contains
  implementation specific logic. This URL is never surfaced to the user
  and only serves to do logic such as editing or deleting passwords.

- shown: The string that is shown in the UI. It hides the scheme and
  common host prefixes (e.g. "www") and indicates explicitly if a
  credential corresponds to an Android app.

- link: The URL that is linked from the UI. This is mostly equivalent to
  origin for Desktop credentials, but differs for Android credentials.
  If possible, there is a link to an affiliated website, otherwise this
  contains a link to the PlayStore.

BUG= 679434 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

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

[modify] https://crrev.com/f120a871d23619bd90c973bc25b0007ab834c0c1/chrome/browser/extensions/api/passwords_private/passwords_private_api.cc
[modify] https://crrev.com/f120a871d23619bd90c973bc25b0007ab834c0c1/chrome/browser/extensions/api/passwords_private/passwords_private_api.h
[modify] https://crrev.com/f120a871d23619bd90c973bc25b0007ab834c0c1/chrome/browser/extensions/api/passwords_private/passwords_private_apitest.cc
[modify] https://crrev.com/f120a871d23619bd90c973bc25b0007ab834c0c1/chrome/browser/extensions/api/passwords_private/passwords_private_delegate.h
[modify] https://crrev.com/f120a871d23619bd90c973bc25b0007ab834c0c1/chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.cc
[modify] https://crrev.com/f120a871d23619bd90c973bc25b0007ab834c0c1/chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.h
[modify] https://crrev.com/f120a871d23619bd90c973bc25b0007ab834c0c1/chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl_unittest.cc
[modify] https://crrev.com/f120a871d23619bd90c973bc25b0007ab834c0c1/chrome/browser/extensions/api/passwords_private/passwords_private_event_router.cc
[modify] https://crrev.com/f120a871d23619bd90c973bc25b0007ab834c0c1/chrome/browser/extensions/api/passwords_private/passwords_private_event_router.h
[modify] https://crrev.com/f120a871d23619bd90c973bc25b0007ab834c0c1/chrome/browser/resources/settings/passwords_and_forms_page/password_edit_dialog.html
[modify] https://crrev.com/f120a871d23619bd90c973bc25b0007ab834c0c1/chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html
[modify] https://crrev.com/f120a871d23619bd90c973bc25b0007ab834c0c1/chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.js
[modify] https://crrev.com/f120a871d23619bd90c973bc25b0007ab834c0c1/chrome/common/extensions/api/passwords_private.idl
[modify] https://crrev.com/f120a871d23619bd90c973bc25b0007ab834c0c1/chrome/test/data/extensions/api_test/passwords_private/test.js
[modify] https://crrev.com/f120a871d23619bd90c973bc25b0007ab834c0c1/chrome/test/data/webui/settings/passwords_and_autofill_fake_data.js
[modify] https://crrev.com/f120a871d23619bd90c973bc25b0007ab834c0c1/chrome/test/data/webui/settings/settings_autofill_section_browsertest.js
[modify] https://crrev.com/f120a871d23619bd90c973bc25b0007ab834c0c1/chrome/test/data/webui/settings/settings_passwords_section_browsertest.js
[modify] https://crrev.com/f120a871d23619bd90c973bc25b0007ab834c0c1/third_party/closure_compiler/externs/passwords_private.js

I request merge of r466661 into branch 3071 (M59). 

Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

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

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

commit 38e6c61012dbcea5d8c640f635b584cc4f67ee1f
Author: jdoerrie <jdoerrie@chromium.org>
Date: Wed Apr 26 08:10:13 2017

Show human readable origin for Android apps

Prior to this change Android credentials were displayed in a human
unfriendly way, e.g. "android://com.nytimes.android/". This change
addresses this issue by trying to obtain the affiliated name, while
still making clear it is an Android credential.

This requires a change to the extensions API. In order to have a user
friendly UI three URLs are transmitted from the backend:

- origin: This URL comes straight from the password store and contains
  implementation specific logic. This URL is never surfaced to the user
  and only serves to do logic such as editing or deleting passwords.

- shown: The string that is shown in the UI. It hides the scheme and
  common host prefixes (e.g. "www") and indicates explicitly if a
  credential corresponds to an Android app.

- link: The URL that is linked from the UI. This is mostly equivalent to
  origin for Desktop credentials, but differs for Android credentials.
  If possible, there is a link to an affiliated website, otherwise this
  contains a link to the PlayStore.

BUG= 679434 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2651663003
Cr-Commit-Position: refs/heads/master@{#466661}
(cherry picked from commit f120a871d23619bd90c973bc25b0007ab834c0c1)

Review-Url: https://codereview.chromium.org/2845543002 .
Cr-Commit-Position: refs/branch-heads/3071@{#220}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/38e6c61012dbcea5d8c640f635b584cc4f67ee1f/chrome/browser/extensions/api/passwords_private/passwords_private_api.cc
[modify] https://crrev.com/38e6c61012dbcea5d8c640f635b584cc4f67ee1f/chrome/browser/extensions/api/passwords_private/passwords_private_api.h
[modify] https://crrev.com/38e6c61012dbcea5d8c640f635b584cc4f67ee1f/chrome/browser/extensions/api/passwords_private/passwords_private_apitest.cc
[modify] https://crrev.com/38e6c61012dbcea5d8c640f635b584cc4f67ee1f/chrome/browser/extensions/api/passwords_private/passwords_private_delegate.h
[modify] https://crrev.com/38e6c61012dbcea5d8c640f635b584cc4f67ee1f/chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.cc
[modify] https://crrev.com/38e6c61012dbcea5d8c640f635b584cc4f67ee1f/chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.h
[modify] https://crrev.com/38e6c61012dbcea5d8c640f635b584cc4f67ee1f/chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl_unittest.cc
[modify] https://crrev.com/38e6c61012dbcea5d8c640f635b584cc4f67ee1f/chrome/browser/extensions/api/passwords_private/passwords_private_event_router.cc
[modify] https://crrev.com/38e6c61012dbcea5d8c640f635b584cc4f67ee1f/chrome/browser/extensions/api/passwords_private/passwords_private_event_router.h
[modify] https://crrev.com/38e6c61012dbcea5d8c640f635b584cc4f67ee1f/chrome/browser/resources/settings/passwords_and_forms_page/password_edit_dialog.html
[modify] https://crrev.com/38e6c61012dbcea5d8c640f635b584cc4f67ee1f/chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html
[modify] https://crrev.com/38e6c61012dbcea5d8c640f635b584cc4f67ee1f/chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.js
[modify] https://crrev.com/38e6c61012dbcea5d8c640f635b584cc4f67ee1f/chrome/common/extensions/api/passwords_private.idl
[modify] https://crrev.com/38e6c61012dbcea5d8c640f635b584cc4f67ee1f/chrome/test/data/extensions/api_test/passwords_private/test.js
[modify] https://crrev.com/38e6c61012dbcea5d8c640f635b584cc4f67ee1f/chrome/test/data/webui/settings/passwords_and_autofill_fake_data.js
[modify] https://crrev.com/38e6c61012dbcea5d8c640f635b584cc4f67ee1f/chrome/test/data/webui/settings/settings_autofill_section_browsertest.js
[modify] https://crrev.com/38e6c61012dbcea5d8c640f635b584cc4f67ee1f/chrome/test/data/webui/settings/settings_passwords_section_browsertest.js
[modify] https://crrev.com/38e6c61012dbcea5d8c640f635b584cc4f67ee1f/third_party/closure_compiler/externs/passwords_private.js

CL got merged and beta bots stayed green, thus I will mark this as fixed now.

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

commit b9fc076120ab64a7ff2bfa46ce511830724a234f
Author: jdoerrie <jdoerrie@chromium.org>
Date: Fri Jul 14 10:39:30 2017

Add Support for App Display Names to GetShownOriginAndLinkUrl

This change adds support for app display names to Password UI Utils.
This is useful for the more human friendly display of Android
credentials, as this will display the Play Store name of the App, if
available.

Bug: 628988,  617094 ,  679434 
Change-Id: Ide7bc694805ce40cbbccf8e8496c32bcb677de25
Reviewed-on: https://chromium-review.googlesource.com/567936
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Hector Carmona <hcarmona@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486721}
[modify] https://crrev.com/b9fc076120ab64a7ff2bfa46ce511830724a234f/chrome/browser/android/password_ui_view_android.cc
[modify] https://crrev.com/b9fc076120ab64a7ff2bfa46ce511830724a234f/chrome/browser/android/password_ui_view_android.h
[modify] https://crrev.com/b9fc076120ab64a7ff2bfa46ce511830724a234f/chrome/browser/extensions/api/passwords_private/passwords_private_utils.cc
[modify] https://crrev.com/b9fc076120ab64a7ff2bfa46ce511830724a234f/chrome/browser/extensions/api/passwords_private/passwords_private_utils.h
[modify] https://crrev.com/b9fc076120ab64a7ff2bfa46ce511830724a234f/chrome/browser/extensions/api/passwords_private/passwords_private_utils_unittest.cc
[modify] https://crrev.com/b9fc076120ab64a7ff2bfa46ce511830724a234f/chrome/browser/ui/passwords/password_manager_presenter.cc
[modify] https://crrev.com/b9fc076120ab64a7ff2bfa46ce511830724a234f/chrome/browser/ui/passwords/password_manager_presenter_unittest.cc
[modify] https://crrev.com/b9fc076120ab64a7ff2bfa46ce511830724a234f/chrome/browser/ui/webui/options/password_manager_handler.cc
[modify] https://crrev.com/b9fc076120ab64a7ff2bfa46ce511830724a234f/components/password_manager/core/browser/password_ui_utils.cc
[modify] https://crrev.com/b9fc076120ab64a7ff2bfa46ce511830724a234f/components/password_manager/core/browser/password_ui_utils.h
[modify] https://crrev.com/b9fc076120ab64a7ff2bfa46ce511830724a234f/components/password_manager/core/browser/password_ui_utils_unittest.cc