+bettes@ for design advice.

We also need to update the password detail dialog.

+bbergher

Bruno, could you chime in. On https://codereview.chromium.org/2805683002/ I got a suggestion to elide the text and I'm not sure it's a good idea.

I see these as possible solutions, and would love bette@'s thoughts:

- Replace 'with <federated_domain>' with a constant string, like "Via website" or something, to ensure it always fits. The use should be able to click/hover to see the full domain.
- Clip the URL at the column width and add a marquee-like effect on hover (from https://developer.android.com/reference/android/widget/TextView.html#attr_android:ellipsize, example https://i.stack.imgur.com/r2YOR.gif). I thins can look weird and is actually deprecated behavior.
- Left-Elide the URL (with ...s.federated.com) and show the full URL on hover/click. I think this is the way to go, but would check with Enamel folks to make sure that's the preferred way to elide domains in this context.

Are there any other alternatives?

I prefer #3. Note that currently there are no hardcoded column sizes. Thus, I need also clarification on that.

Assigning to bettes@ to provide design advice.

Discussed with hcarmona@ and bbergher@ about the following: 

- use a constant string, "from website", to avoid misalignment in the layout
- use "info_outline" as an entry point for a tooltips containing URLs

Tooltips have the real estate to accommodate short and long URLs, accessible through mouse_hover and tap (for convertibles)

I'm not sure if our tooltips operate on max-width or fixed-width, but this implementation should mirror whatever it is we do today for policy-indicators. 

For dialogs:

- remove the "eye-visibility" icon
- place the URL in the disabled text-field. (url is copyable)

Deleted: Artboard.png 194 KB

	Artboard.png 194 KB View Download

Deleted: spec.png 18.0 KB

	spec.png 18.0 KB View Download

Can we look for a better string than "from website"?

- "website" is the title of the first column but this is referencing the identity provider.
- the third column is called "password" and we are not using the password from the website mentioned by the (i).

How about "via identity provider (i)"?

But frankly, I would prefer the screenshot of comment #1 with a faded URL that shows in a hover menu.

Agree with Comment #8. "from website" is both confusing and not informative enough.
We (the privacy and password manager) team would not like to sacrifice the content.

I created a separate  Issue 713743  for column misalignment. It's an existing problem unrelated to the federated credentials.

If it was the only concern against landing https://codereview.chromium.org/2805683002/ then I propose to land it now. It fixes the privacy issue and improves the current state. Any objections?

SGTM. In  Issue 679434  comment #7 has a proposed design for having 2 columns: Website and Credentials. Looks like it provides more space for the "from federated.com" text.

I'm ok landing as is and fixing alignment issues separately with possible re-layout.

if this was in the old options page but isn't in the new MD settings page, we need to do this and merge

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

commit 7a0ea2253643c495276d4f5953871318f706509a
Author: vasilii <vasilii@chromium.org>
Date: Fri Apr 21 18:49:04 2017

Show "with <something>" for federated credentials in the MD settings.

A screenshot is attached to the bug.

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

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

[modify] https://crrev.com/7a0ea2253643c495276d4f5953871318f706509a/chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html

Now we have to do something with the dialog. According to Issue 714140 it's not clear to me if we need that dialog at all.

I defer to the UX designers.

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

commit a7695ef873dfa0530d2ea4c1088e2fb67719607a
Author: vasilii <vasilii@chromium.org>
Date: Wed Apr 26 09:38:26 2017

Minor comment on the chrome://settings/passwords

This is a follow-up to https://codereview.chromium.org/2805683002/

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

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

[modify] https://crrev.com/a7695ef873dfa0530d2ea4c1088e2fb67719607a/chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html

Can we merge this to 59?

I think updating the dialog for 59 seems like a smaller change than removing it. vasilii@, what do you think?

What is so special for M59? MD settings are release or what?

yes, Material Design settings ships in M59

perhaps said in another way: if we don't merge this, Chrome users will not see "with federated.com" in settings for ~6 weeks

I can make the changes to the dialog quickly

I want to merge r466400 to M59.

hcarmona@ you can proceed with the dialog.

Adding screenshot of dialog

Deleted: with federated.png 16.1 KB

	with federated.png 16.1 KB View Download

Has this change baked in canary yet, tested, and confirmed? Is there enough automated unit testing coverage for this change?

r466400 is in Canary, I just looked at Version 60.0.3080.5 (Official Build) canary (64-bit) on MacOS and the change is there. A site with a password works and a site w/ federated text displays as "with federitedSite.com". That's a very low risk merge as it only affects HTML, IMO.

Just noticed that in the screenshot on #16, the columns are not aligned w/ the headings

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/+/97528cb013f79356e813919e35c4c2d9509d69f2

commit 97528cb013f79356e813919e35c4c2d9509d69f2
Author: Vasilii Sukhanov <vasilii@chromium.org>
Date: Thu Apr 27 14:22:26 2017

Show "with <something>" for federated credentials in the MD settings.

A screenshot is attached to the bug.

BUG= 706310 
TBR=hcarmona@chromium.org

Review-Url: https://codereview.chromium.org/2805683002
Cr-Commit-Position: refs/heads/master@{#466400}
(cherry picked from commit 7a0ea2253643c495276d4f5953871318f706509a)
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

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

[modify] https://crrev.com/97528cb013f79356e813919e35c4c2d9509d69f2/chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html

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

commit 119717035c2dc30d8602465cbc1428eb72072c0d
Author: hcarmona <hcarmona@chromium.org>
Date: Fri Apr 28 16:22:07 2017

MD-Settings - Add test for federated password dialog.

Followup for http://crrev.com/2844553003

R=dbeam@chromium.org
BUG= 706310 

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

[modify] https://crrev.com/119717035c2dc30d8602465cbc1428eb72072c0d/chrome/test/data/webui/settings/settings_passwords_section_browsertest.js

Pri-0 bugs are critical regressions or serious emergencies, and this bug has not been updated in three days. Could you please provide an update, or adjust the priority to a more appropriate level if applicable?

If a fix is in active development, please set the status to Started.

Thanks for your time! To disable nags, add the Disable-Nags label.

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

Verified this issue on Win 10, Linux Ubuntu 14.04 and Mac OS 10.12.4 using chrome latest beta M59 #59.0.3071.36 by following steps mentioned in the original comment. 

Observed the text on the password field. Hence adding the TE- Verified label

Please refer the screen cast

Deleted: 706310.mp4 1.8 MB

	706310.mp4 1.8 MB View Download