Add Information Tooltip for Public Suffix List Matches

chrome/android/java/src/org/chromium/chrome/browser/password_manager/AccountChooserDialog.java

Index: chrome/android/java/src/org/chromium/chrome/browser/password_manager/AccountChooserDialog.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/password_manager/AccountChooserDialog.java b/chrome/android/java/src/org/chromium/chrome/browser/password_manager/AccountChooserDialog.java
index 4c41262fb1945784f7eb33a5ab13b333c6e4846c..d1b02c941625a04b55b6f6117f185f25c4817311 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/password_manager/AccountChooserDialog.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/password_manager/AccountChooserDialog.java
@@ -7,21 +7,27 @@ package org.chromium.chrome.browser.password_manager;
 import android.app.Activity;
 import android.content.Context;
 import android.content.DialogInterface;
+import android.content.res.Resources;
 import android.graphics.Bitmap;
+import android.graphics.Color;
 import android.support.v7.app.AlertDialog;
 import android.text.SpannableString;
 import android.text.Spanned;
 import android.text.TextUtils;
 import android.text.method.LinkMovementMethod;
 import android.text.style.ClickableSpan;
+import android.util.TypedValue;
+import android.view.Gravity;
 import android.view.LayoutInflater;
 import android.view.View;
 import android.view.ViewGroup;
 import android.widget.ArrayAdapter;
 import android.widget.ImageView;
 import android.widget.ListView;
+import android.widget.PopupWindow;
 import android.widget.TextView;
 
+import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.signin.AccountManagementFragment;
@@ -138,6 +144,65 @@ public class AccountChooserDialog
                     secondaryNameView.setVisibility(View.VISIBLE);
                 }
 
+                final String originUrl = credential.getOriginUrl();
+                if (!originUrl.isEmpty()) {
+                    ImageView pslInfoIconView =
+                            (ImageView) convertView.findViewById(R.id.psl_info_icon);
+                    pslInfoIconView.setImageResource(R.drawable.btn_info);

[gone, 2016/12/08 01:32:44]

Set the ImageResource in the XML file directly.

[jdoerrie, 2016/12/13 17:24:48]

Done.

+                    pslInfoIconView.setClickable(true);

[gone, 2016/12/08 01:32:44]

If you use the ImageButton, this should be automatically be clickable.  Ditto if
you set an OnClickListener.

[jdoerrie, 2016/12/13 17:24:48]

Acknowledged.

+                    pslInfoIconView.setOnClickListener(new View.OnClickListener() {
+                        @Override
+                        public void onClick(View view) {
+                            final ImageView iconView = (ImageView) view;

[gone, 2016/12/08 01:32:44]

Pull this all out into another function called prepareIconView or something. 
It's indented way too deeply.

[jdoerrie, 2016/12/13 17:24:48]

Done.

+                            Context context = iconView.getContext();
+                            Resources resources = context.getResources();
+                            iconView.setBackground(ApiCompatibilityUtils.getDrawable(
+                                    resources, R.drawable.psl_info_icon_background));
+
+                            TextView textView = new TextView(context);

[gone, 2016/12/08 01:32:44]

1) Can you apply the background for the tooltip to the TextView, instead? 
There's (currently) nothing else in the tooltip, so it should be fine.  It'd
also make it .

2) Pull all this TextView stuff into an XML layout file instead of doing it
here.  Makes it clearer to understand how you're styling the font and
dimensions.

3) Use a LayoutInflater.from(context) to inflate the TextView layout.

[jdoerrie, 2016/12/13 17:24:48]

Done.

+                            textView.setText(originUrl);
+                            textView.setTextColor(Color.WHITE);
+                            textView.setTextSize(TypedValue.COMPLEX_UNIT_PX, resources.getDimension(
+                                    R.dimen.psl_info_tooltip_text_size));
+                            int horizontalPadding = resources.getDimensionPixelSize(
+                                    R.dimen.psl_info_tooltip_horizontal_padding);
+                            textView.setPadding(horizontalPadding, 0, horizontalPadding, 0);
+                            textView.setHeight(resources.getDimensionPixelSize(
+                                    R.dimen.psl_info_tooltip_height));

[gone, 2016/12/08 01:32:44]

Don't set the height of the TextView directly.  People who have changed the
style or size of their fonts system-wide (for accessibility, e.g.) will bork
your UI.

[jdoerrie, 2016/12/13 17:24:48]

I would like to avoid this, however the official Material Design spec suggests a
fixed height of 32dp: 

https://material.google.com/components/tooltips.html#tooltips-tooltips-mobile

How is this done in other parts of the code base?

[gone, 2016/12/13 23:14:29]

A fixed-height on text is generally unenforceable; most places end up specifying
wrap_content to avoid clipping the text.  We roughly control the size of the
TextView by adjusting the font size.

[jdoerrie, 2016/12/14 16:04:01]

We could specify padding in the tooltip background to achieve a similar effect.
On my device a fontsize of 14sp combined with setting a top and bottom padding
of 6.5dp comes close to a total height of 32dp, but 6.5dp still seems arbitrary
to me. Is there a commonly used vertical padding in other parts of the codebase?

[gone, 2016/12/14 18:20:42]

Setting padding like that is totally arbitrary and won't apply in all situations
:/  You could try using
https://developer.android.com/reference/android/view/View.html#setMinimumHeig...
on the appropriate view so that the toast can expand as necessary.

[jdoerrie, 2016/12/15 13:27:18]

Done.

+                            textView.setGravity(Gravity.CENTER);
+
+                            PopupWindow tooltip = new PopupWindow(context);
+                            tooltip.setContentView(textView);
+                            tooltip.setOutsideTouchable(true);
+                            tooltip.setBackgroundDrawable(ApiCompatibilityUtils.getDrawable(
+                                    resources, R.drawable.psl_info_tooltip_background));
+                            tooltip.setOnDismissListener(new PopupWindow.OnDismissListener() {
+                                @Override
+                                public void onDismiss() {
+                                    iconView.setBackgroundResource(0);
+                                }
+                            });
+
+                            // Measure size of textView for correct offsets.
+                            textView.measure(0, 0);

[gone, 2016/12/08 01:32:44]

These should be MeasureSpecs instead of straight integer values.  Use
MeasureSpec#makeMeasureSpec() so that it's obvious what you're trying to do.

[jdoerrie, 2016/12/15 13:27:18]

Acknowledged.

+
+                            // Shift tooltip to the left by required amount, handle RTL correctly.
+                            int xOffset = ApiCompatibilityUtils.isLayoutRtl(iconView)
+                                    ? -iconView.getWidth()
+                                    : -(textView.getMeasuredWidth() - iconView.getWidth());
+
+                            // Shift tooltip up by required amount.

[gone, 2016/12/08 01:32:44]

What happens on small phones and long addresses?  You should:

1) Clamp the maximum size of the text view so that it doesn't run off the
screen.
2) Set it to single line mode, then ellipsize it so that the last part of the
domain remains readable, similarly to how we have to deal with URLs in other
places.

[jdoerrie, 2016/12/13 17:24:48]

Done.

+                            int tooltip_margin = resources.getDimensionPixelSize(
+                                    R.dimen.psl_info_tooltip_margin);
+                            int yOffset = -(textView.getMeasuredHeight() + iconView.getHeight()
+                                    + tooltip_margin);
+
+                            tooltip.showAsDropDown(iconView, xOffset, yOffset);
+                            textView.announceForAccessibility(textView.getText());
+                        }
+                    });
+                }
+
                 return convertView;
             }
         };