Merge #7705

7705: For #7700 - Use the website favicon in save login dialog r=Amejia481 a=BranescuMihai



Co-authored-by: Mihai Branescu <branescu.mihai@gmail.com>

components/feature/prompts/src/main/java/mozilla/components/feature/prompts/PromptFeature.kt

@@ -475,7 +475,8 @@ class PromptFeature private constructor(
                     sessionId = session.id,
                     hint = promptRequest.hint,
                     // For v1, we only handle a single login and drop all others on the floor
-                    login = promptRequest.logins[0]
+                    login = promptRequest.logins[0],
+                    icon = session.content.icon
                 )
             }
 

components/feature/prompts/src/main/java/mozilla/components/feature/prompts/dialog/SaveLoginDialogFragment.kt

@@ -6,6 +6,8 @@ package mozilla.components.feature.prompts.dialog
 
 import android.app.Dialog
 import android.content.DialogInterface
+import android.content.res.ColorStateList
+import android.graphics.Bitmap
 import android.os.Bundle
 import android.text.Editable
 import android.text.TextWatcher
@@ -14,8 +16,11 @@ import android.view.View
 import android.view.ViewGroup
 import android.widget.Button
 import android.widget.FrameLayout
+import android.widget.ImageView
 import androidx.annotation.VisibleForTesting
 import androidx.appcompat.widget.AppCompatTextView
+import androidx.core.content.ContextCompat
+import androidx.core.widget.ImageViewCompat
 import com.google.android.material.bottomsheet.BottomSheetBehavior
 import com.google.android.material.bottomsheet.BottomSheetDialog
 import com.google.android.material.button.MaterialButton
@@ -36,6 +41,7 @@ import mozilla.components.concept.storage.Login
 import mozilla.components.concept.storage.LoginValidationDelegate.Result
 import mozilla.components.feature.prompts.R
 import mozilla.components.feature.prompts.ext.onDone
+import mozilla.components.support.ktx.android.content.res.resolveAttribute
 import mozilla.components.support.ktx.android.view.hideKeyboard
 import mozilla.components.support.ktx.android.view.toScope
 import java.util.concurrent.CopyOnWriteArrayList
@@ -49,6 +55,7 @@ private const val KEY_LOGIN_GUID = "KEY_LOGIN_GUID"
 private const val KEY_LOGIN_ORIGIN = "KEY_LOGIN_ORIGIN"
 private const val KEY_LOGIN_FORM_ACTION_ORIGIN = "KEY_LOGIN_FORM_ACTION_ORIGIN"
 private const val KEY_LOGIN_HTTP_REALM = "KEY_LOGIN_HTTP_REALM"
+@VisibleForTesting internal const val KEY_LOGIN_ICON = "KEY_LOGIN_ICON"
 
 /**
  * [android.support.v4.app.DialogFragment] implementation to display a
@@ -70,9 +77,13 @@ internal class SaveLoginDialogFragment : PromptDialogFragment() {
     private val origin by lazy { safeArguments.getString(KEY_LOGIN_ORIGIN)!! }
     private val formActionOrigin by lazy { safeArguments.getString(KEY_LOGIN_FORM_ACTION_ORIGIN) }
     private val httpRealm by lazy { safeArguments.getString(KEY_LOGIN_HTTP_REALM) }
+    @VisibleForTesting
+    internal val icon by lazy { safeArguments.getParcelable<Bitmap>(KEY_LOGIN_ICON) }
 
-    private var username by SafeArgString(KEY_LOGIN_USERNAME)
-    private var password by SafeArgString(KEY_LOGIN_PASSWORD)
+    @VisibleForTesting
+    internal var username by SafeArgString(KEY_LOGIN_USERNAME)
+    @VisibleForTesting
+    internal var password by SafeArgString(KEY_LOGIN_PASSWORD)
 
     private var validateStateUpdate: Job? = null
 
@@ -122,7 +133,7 @@ internal class SaveLoginDialogFragment : PromptDialogFragment() {
             }
         }
 
-        return inflateRootView(container)
+        return setupRootView(container)
     }
 
     override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
@@ -174,15 +185,22 @@ internal class SaveLoginDialogFragment : PromptDialogFragment() {
         dismiss()
     }
 
-    private fun inflateRootView(container: ViewGroup? = null): View {
-        val rootView = LayoutInflater.from(requireContext()).inflate(
+    @VisibleForTesting
+    internal fun setupRootView(container: ViewGroup? = null): View {
+        val rootView = inflateRootView(container)
+        bindUsername(rootView)
+        bindPassword(rootView)
+        bindIcon(rootView)
+        return rootView
+    }
+
+    @VisibleForTesting
+    internal fun inflateRootView(container: ViewGroup? = null): View {
+        return LayoutInflater.from(requireContext()).inflate(
             R.layout.mozac_feature_prompt_save_login_prompt,
             container,
             false
         )
-        bindUsername(rootView)
-        bindPassword(rootView)
-        return rootView
     }
 
     private fun bindUsername(view: View) {
@@ -251,6 +269,24 @@ internal class SaveLoginDialogFragment : PromptDialogFragment() {
         }
     }
 
+    private fun bindIcon(view: View) {
+        val iconView = view.findViewById<ImageView>(R.id.host_icon)
+        if (icon != null) {
+            iconView.setImageBitmap(icon)
+        } else {
+            setImageViewTint(iconView)
+        }
+    }
+
+    @VisibleForTesting
+    internal fun setImageViewTint(imageView: ImageView) {
+        val tintColor = ContextCompat.getColor(
+            requireContext(),
+            requireContext().theme.resolveAttribute(android.R.attr.textColorPrimary)
+        )
+        ImageViewCompat.setImageTintList(imageView, ColorStateList.valueOf(tintColor))
+    }
+
     /**
      * Check current state then update view state to match.
      */
@@ -357,7 +393,8 @@ internal class SaveLoginDialogFragment : PromptDialogFragment() {
         fun newInstance(
             sessionId: String,
             hint: Int,
-            login: Login
+            login: Login,
+            icon: Bitmap? = null
         ): SaveLoginDialogFragment {
 
             val fragment = SaveLoginDialogFragment()
@@ -372,6 +409,7 @@ internal class SaveLoginDialogFragment : PromptDialogFragment() {
                 putString(KEY_LOGIN_ORIGIN, login.origin)
                 putString(KEY_LOGIN_FORM_ACTION_ORIGIN, login.formActionOrigin)
                 putString(KEY_LOGIN_HTTP_REALM, login.httpRealm)
+                putParcelable(KEY_LOGIN_ICON, icon)
             }
 
             fragment.arguments = arguments

components/feature/prompts/src/main/res/layout/mozac_feature_prompt_save_login_prompt.xml

@@ -15,19 +15,27 @@
     android:paddingBottom="16dp"
     tools:ignore="Overdraw">
 
+    <androidx.appcompat.widget.AppCompatImageView
+        android:id="@+id/host_icon"
+        android:layout_width="24dp"
+        android:layout_height="24dp"
+        app:layout_constraintStart_toStartOf="parent"
+        app:layout_constraintTop_toTopOf="parent"
+        app:srcCompat="@drawable/mozac_ic_globe"
+        android:importantForAccessibility="no" />
+
     <androidx.appcompat.widget.AppCompatTextView
         android:id="@+id/host_name"
         android:layout_width="0dp"
         android:layout_height="wrap_content"
-        android:drawablePadding="16dp"
         android:focusable="true"
         android:focusableInTouchMode="true"
         android:gravity="center_vertical"
         android:textColor="?android:textColorPrimary"
         android:textSize="16sp"
-        app:drawableStartCompat="@drawable/mozac_ic_globe"
-        app:drawableTint="?android:textColorPrimary"
-        app:layout_constraintStart_toStartOf="parent"
+        android:layout_marginStart="12dp"
+        android:layout_marginEnd="12dp"
+        app:layout_constraintStart_toEndOf="@+id/host_icon"
         app:layout_constraintTop_toTopOf="parent"
         tools:text="host.com" />
 

components/feature/prompts/src/test/java/mozilla/components/feature/prompts/PromptFeatureTest.kt

@@ -10,6 +10,7 @@ import android.app.Activity.RESULT_OK
 import android.content.ClipData
 import android.content.Context
 import android.content.Intent
+import android.graphics.Bitmap
 import android.net.Uri
 import androidx.fragment.app.Fragment
 import androidx.fragment.app.FragmentManager
@@ -62,6 +63,7 @@ import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.mockito.Mockito.`when`
 import org.mockito.Mockito.doReturn
 import org.mockito.Mockito.never
 import org.mockito.Mockito.spy
@@ -1118,6 +1120,43 @@ class PromptFeatureTest {
         verify(fragment, times(1)).dismiss()
     }
 
+    @Test
+    fun `prompt will always start the save login dialog with an icon`() {
+        val feature = PromptFeature(
+            activity = mock(),
+            store = store,
+            fragmentManager = fragmentManager,
+            shareDelegate = mock(),
+            isSaveLoginEnabled = { true },
+            loginValidationDelegate = mock()
+        ) { }
+        val loginUsername = "username"
+        val loginPassword = "password"
+        val login: Login = mock()
+        `when`(login.username).thenReturn(loginUsername)
+        `when`(login.password).thenReturn(loginPassword)
+        val loginsPrompt = PromptRequest.SaveLoginPrompt(2, listOf(login), { }, { })
+        val websiteIcon: Bitmap = mock()
+        val contentState: ContentState = mock()
+        val session: TabSessionState = mock()
+        val sessionId = "sessionId"
+        `when`(contentState.icon).thenReturn(websiteIcon)
+        `when`(session.content).thenReturn(contentState)
+        `when`(session.id).thenReturn(sessionId)
+
+        feature.handleDialogsRequest(
+            loginsPrompt, session
+        )
+
+        // Only interested in the icon, but it doesn't hurt to be sure we show a properly configured dialog.
+        assertTrue(feature.activePrompt!!.get() is SaveLoginDialogFragment)
+        val dialogFragment = feature.activePrompt!!.get() as SaveLoginDialogFragment
+        assertEquals(loginUsername, dialogFragment.username)
+        assertEquals(loginPassword, dialogFragment.password)
+        assertEquals(websiteIcon, dialogFragment.icon)
+        assertEquals(sessionId, dialogFragment.sessionId)
+    }
+
     @Test
     fun `save login dialog will not be dismissed on page load`() {
         val feature = spy(

components/feature/prompts/src/test/java/mozilla/components/feature/prompts/dialog/SaveLoginDialogFragmentTest.kt

+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package mozilla.components.feature.prompts.dialog
+
+import android.graphics.Bitmap
+import android.graphics.drawable.BitmapDrawable
+import android.widget.FrameLayout
+import android.widget.ImageView
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import com.google.android.material.textfield.TextInputEditText
+import junit.framework.TestCase
+import mozilla.components.concept.storage.Login
+import mozilla.components.feature.prompts.R
+import mozilla.components.support.test.any
+import mozilla.components.support.test.mock
+import mozilla.ext.appCompatContext
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.mockito.Mockito.`when`
+import org.mockito.Mockito.doAnswer
+import org.mockito.Mockito.doReturn
+import org.mockito.Mockito.spy
+import org.mockito.Mockito.times
+import org.mockito.Mockito.verify
+import org.robolectric.Shadows
+
+@RunWith(AndroidJUnit4::class)
+class SaveLoginDialogFragmentTest : TestCase() {
+    @Test
+    fun `dialog should always set the website icon if it is available`() {
+        val sessionId = "sessionId"
+        val hint = 42
+        val loginUsername = "username"
+        val loginPassword = "password"
+        val login: Login = mock() // valid image to be used as favicon
+        `when`(login.username).thenReturn(loginUsername)
+        `when`(login.password).thenReturn(loginPassword)
+        val icon: Bitmap = mock()
+        val fragment = spy(SaveLoginDialogFragment.newInstance(
+            sessionId, hint, login, icon
+        ))
+        doReturn(appCompatContext).`when`(fragment).requireContext()
+        doAnswer {
+            FrameLayout(appCompatContext).apply {
+                addView(TextInputEditText(appCompatContext).apply { id = R.id.username_field })
+                addView(TextInputEditText(appCompatContext).apply { id = R.id.password_field })
+                addView(ImageView(appCompatContext).apply { id = R.id.host_icon })
+            }
+        }.`when`(fragment).inflateRootView(any())
+
+        val fragmentView = fragment.onCreateView(mock(), mock(), mock())
+
+        verify(fragment).inflateRootView(any())
+        verify(fragment).setupRootView(any())
+        assertEquals(sessionId, fragment.sessionId)
+        // Using assertTrue since assertEquals / assertSame would fail here
+        assertTrue(loginUsername == fragmentView.findViewById<TextInputEditText>(R.id.username_field).text.toString())
+        assertTrue(loginPassword == fragmentView.findViewById<TextInputEditText>(R.id.password_field).text.toString())
+
+        // Actually verifying that the provided image is used
+        verify(fragment, times(0)).setImageViewTint(any())
+        assertSame(icon, (fragmentView.findViewById<ImageView>(R.id.host_icon).drawable as BitmapDrawable).bitmap)
+    }
+
+    @Test
+    fun `dialog should use a default tinted icon if favicon is not available`() {
+        val sessionId = "sessionId"
+        val hint = 42
+        val loginUsername = "username"
+        val loginPassword = "password"
+        val login: Login = mock()
+        `when`(login.username).thenReturn(loginUsername)
+        `when`(login.password).thenReturn(loginPassword)
+        val icon: Bitmap? = null // null favicon
+        val fragment = spy(SaveLoginDialogFragment.newInstance(
+            sessionId, hint, login, icon
+        ))
+        val defaultIconResource = R.drawable.mozac_ic_globe
+        doReturn(appCompatContext).`when`(fragment).requireContext()
+        doAnswer {
+            FrameLayout(appCompatContext).apply {
+                addView(TextInputEditText(appCompatContext).apply { id = R.id.username_field })
+                addView(TextInputEditText(appCompatContext).apply { id = R.id.password_field })
+                addView(ImageView(appCompatContext).apply {
+                    id = R.id.host_icon
+                    setImageResource(defaultIconResource)
+                })
+            }
+        }.`when`(fragment).inflateRootView(any())
+
+        val fragmentView = fragment.onCreateView(mock(), mock(), mock())
+
+        verify(fragment).inflateRootView(any())
+        verify(fragment).setupRootView(any())
+        assertEquals(sessionId, fragment.sessionId)
+        // Using assertTrue since assertEquals / assertSame would fail here
+        assertTrue(loginUsername == fragmentView.findViewById<TextInputEditText>(R.id.username_field).text.toString())
+        assertTrue(loginPassword == fragmentView.findViewById<TextInputEditText>(R.id.password_field).text.toString())
+
+        // Actually verifying that the tinted default image is used
+        val iconView = fragmentView.findViewById<ImageView>(R.id.host_icon)
+        verify(fragment).setImageViewTint(iconView)
+        assertNotNull(iconView.imageTintList)
+        // The icon sent was null, we want the default instead
+        assertNotNull(iconView.drawable)
+        assertEquals(defaultIconResource, Shadows.shadowOf(iconView.drawable).createdFromResId)
+    }
+}

docs/changelog.md

@@ -31,6 +31,9 @@ permalink: /changelog/
   * Added `ContainerToolbarFeature` to update the toolbar with the container page action whenever
     the selected tab changes.
 
+* **feature-prompts**
+  * Replaced generic icon in `LoginDialogFragment` with site icon (keep the generic one as fallback)
+
 # 56.0.0
 
 * [Commits](https://github.com/mozilla-mobile/android-components/compare/v55.0.0...v56.0.0)