Show a warning dialog if the text of the clicked link does not match the link target

Fixes #922

CHANGES.md

@@ -10,7 +10,7 @@ Features ✨:
 
 Improvements 🙌:
  - Verification DM / Handle concurrent .start after .ready (#794)
- - Reimplementation of multiple attachment picker
+ - Reimplementation of multiple attachment picker@
  - Cross-Signing | Update Shield Logic for DM (#963)
  - Cross-Signing | Complete security new session design update (#1135)
  - Cross-Signing | Setup key backup as part of SSSS bootstrapping (#1201)
@@ -22,6 +22,7 @@ Improvements 🙌:
  - Emoji Verification | It's not the same butterfly! (#1220)
  - Cross-Signing | Composer decoration: shields (#1077)
  - Cross-Signing | Migrate existing keybackup to cross signing with 4S from mobile (#1197)
+ - Show a warning dialog if the text of the clicked link does not match the link target (#922)
 
 Bugfix 🐛:
  - Fix summary notification staying after "mark as read"

vector/src/main/java/im/vector/riotx/core/utils/EvenBetterLinkMovementMethod.kt

@@ -0,0 +1,48 @@
+/*
+ * Copyright (c) 2020 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package im.vector.riotx.core.utils
+
+import android.text.Spanned
+import android.text.style.ClickableSpan
+import android.text.style.URLSpan
+import android.widget.TextView
+import me.saket.bettermovementmethod.BetterLinkMovementMethod
+
+class EvenBetterLinkMovementMethod(private val onLinkClickListener: OnLinkClickListener? = null) : BetterLinkMovementMethod() {
+
+    interface OnLinkClickListener {
+        /**
+         * @param textView   The TextView on which a click was registered.
+         * @param span       The ClickableSpan which is clicked on.
+         * @param url        The clicked URL.
+         * @param actualText The original text which is spanned. Can be used to compare actualText and target url to prevent misleading urls.
+         * @return true if this click was handled, false to let Android handle the URL.
+         */
+        fun onLinkClicked(textView: TextView, span: ClickableSpan, url: String, actualText: String): Boolean
+    }
+
+    override fun dispatchUrlClick(textView: TextView, clickableSpan: ClickableSpan) {
+        val spanned = textView.text as Spanned
+        val actualText = textView.text.subSequence(spanned.getSpanStart(clickableSpan), spanned.getSpanEnd(clickableSpan)).toString()
+        val url = (clickableSpan as? URLSpan)?.url ?: actualText
+
+        if (onLinkClickListener == null || !onLinkClickListener.onLinkClicked(textView, clickableSpan, url, actualText)) {
+            // Let Android handle this long click as a short-click.
+            clickableSpan.onClick(textView)
+        }
+    }
+}

vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailFragment.kt

@@ -113,6 +113,7 @@ import im.vector.riotx.core.utils.checkPermissions
 import im.vector.riotx.core.utils.copyToClipboard
 import im.vector.riotx.core.utils.createUIHandler
 import im.vector.riotx.core.utils.getColorFromUserId
+import im.vector.riotx.core.utils.isValidUrl
 import im.vector.riotx.core.utils.jsonViewerStyler
 import im.vector.riotx.core.utils.openUrlInExternalBrowser
 import im.vector.riotx.core.utils.saveMedia
@@ -919,7 +920,7 @@ class RoomDetailFragment @Inject constructor(
 
     // TimelineEventController.Callback ************************************************************
 
-    override fun onUrlClicked(url: String): Boolean {
+    override fun onUrlClicked(url: String, title: String): Boolean {
         permalinkHandler
                 .launch(requireActivity(), url, object : NavigationInterceptor {
                     override fun navToRoom(roomId: String?, eventId: String?): Boolean {
@@ -947,8 +948,20 @@ class RoomDetailFragment @Inject constructor(
                 .observeOn(AndroidSchedulers.mainThread())
                 .subscribe { managed ->
                     if (!managed) {
-                        // Open in external browser, in a new Tab
-                        openUrlInExternalBrowser(requireContext(), url)
+                        if (title.isValidUrl() && title != url) {
+                            AlertDialog.Builder(requireActivity())
+                                    .setTitle(R.string.external_link_confirmation_title)
+                                    .setMessage(getString(R.string.external_link_confirmation_message, title, url))
+                                    .setPositiveButton(R.string.external_link_confirmation_negative_button) { _, _ ->
+                                        openUrlInExternalBrowser(requireContext(), url)
+                                    }
+                                    .setNegativeButton(R.string.external_link_confirmation_positive_button, null)
+                                    .show()
+                                    .withColoredButton(DialogInterface.BUTTON_NEGATIVE)
+                        } else {
+                            // Open in external browser, in a new Tab
+                            openUrlInExternalBrowser(requireContext(), url)
+                        }
                     }
                 }
                 .disposeOnDestroyView()
@@ -1263,7 +1276,7 @@ class RoomDetailFragment @Inject constructor(
                 roomDetailViewModel.handle(RoomDetailAction.IgnoreUser(action.senderId))
             }
             is EventSharedAction.OnUrlClicked               -> {
-                onUrlClicked(action.url)
+                onUrlClicked(action.url, action.title)
             }
             is EventSharedAction.OnUrlLongClicked           -> {
                 onUrlLongClicked(action.url)

vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/TimelineEventController.kt

@@ -104,7 +104,7 @@ class TimelineEventController @Inject constructor(private val dateFormatter: Vec
     }
 
     interface UrlClickCallback {
-        fun onUrlClicked(url: String): Boolean
+        fun onUrlClicked(url: String, title: String): Boolean
         fun onUrlLongClicked(url: String): Boolean
     }
 

vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/action/EventSharedAction.kt

@@ -99,7 +99,7 @@ sealed class EventSharedAction(@StringRes val titleRes: Int,
             EventSharedAction(R.string.message_view_edit_history, R.drawable.ic_view_edit_history)
 
     // An url in the event preview has been clicked
-    data class OnUrlClicked(val url: String) :
+    data class OnUrlClicked(val url: String, val title: String) :
             EventSharedAction(0, 0)
 
     // An url in the event preview has been long clicked

vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/action/MessageActionsBottomSheet.kt

@@ -63,8 +63,8 @@ class MessageActionsBottomSheet : VectorBaseBottomSheetDialogFragment(), Message
         super.onDestroyView()
     }
 
-    override fun onUrlClicked(url: String): Boolean {
-        sharedActionViewModel.post(EventSharedAction.OnUrlClicked(url))
+    override fun onUrlClicked(url: String, title: String): Boolean {
+        sharedActionViewModel.post(EventSharedAction.OnUrlClicked(url, title))
         // Always consume
         return true
     }

vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/tools/EventRenderingTools.kt

@@ -17,11 +17,14 @@
 package im.vector.riotx.features.home.room.detail.timeline.tools
 
 import android.text.SpannableStringBuilder
+import android.text.style.ClickableSpan
 import android.view.MotionEvent
+import android.widget.TextView
 import androidx.core.text.toSpannable
 import im.vector.matrix.android.api.permalinks.MatrixLinkify
 import im.vector.matrix.android.api.permalinks.MatrixPermalinkSpan
 import im.vector.riotx.core.linkify.VectorLinkify
+import im.vector.riotx.core.utils.EvenBetterLinkMovementMethod
 import im.vector.riotx.core.utils.isValidUrl
 import im.vector.riotx.features.home.room.detail.timeline.TimelineEventController
 import im.vector.riotx.features.html.PillImageSpan
@@ -29,7 +32,6 @@ import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
-import me.saket.bettermovementmethod.BetterLinkMovementMethod
 
 fun CharSequence.findPillsAndProcess(scope: CoroutineScope, processBlock: (PillImageSpan) -> Unit) {
     scope.launch(Dispatchers.Main) {
@@ -42,10 +44,11 @@ fun CharSequence.findPillsAndProcess(scope: CoroutineScope, processBlock: (PillI
 }
 
 fun CharSequence.linkify(callback: TimelineEventController.UrlClickCallback?): CharSequence {
+    val text = this.toString()
     val spannable = SpannableStringBuilder(this)
     MatrixLinkify.addLinks(spannable, object : MatrixPermalinkSpan.Callback {
         override fun onUrlClicked(url: String) {
-            callback?.onUrlClicked(url)
+            callback?.onUrlClicked(url, text)
         }
     })
     VectorLinkify.addLinks(spannable, true)
@@ -54,23 +57,21 @@ fun CharSequence.linkify(callback: TimelineEventController.UrlClickCallback?): C
 
 // Better link movement methods fixes the issue when
 // long pressing to open the context menu on a TextView also triggers an autoLink click.
-fun createLinkMovementMethod(urlClickCallback: TimelineEventController.UrlClickCallback?): BetterLinkMovementMethod {
-    return BetterLinkMovementMethod.newInstance()
-            .apply {
-                setOnLinkClickListener { _, url ->
-                    // Return false to let android manage the click on the link, or true if the link is handled by the application
-                    url.isValidUrl() && urlClickCallback?.onUrlClicked(url) == true
-                }
-
-                // We need also to fix the case when long click on link will trigger long click on cell
-                setOnLinkLongClickListener { tv, url ->
-                    // Long clicks are handled by parent, return true to block android to do something with url
-                    if (url.isValidUrl() && urlClickCallback?.onUrlLongClicked(url) == true) {
-                        tv.dispatchTouchEvent(MotionEvent.obtain(0, 0, MotionEvent.ACTION_CANCEL, 0f, 0f, 0))
-                        true
-                    } else {
-                        false
-                    }
-                }
+fun createLinkMovementMethod(urlClickCallback: TimelineEventController.UrlClickCallback?): EvenBetterLinkMovementMethod {
+    return EvenBetterLinkMovementMethod(object : EvenBetterLinkMovementMethod.OnLinkClickListener {
+        override fun onLinkClicked(textView: TextView, span: ClickableSpan, url: String, actualText: String): Boolean {
+            return url.isValidUrl() && urlClickCallback?.onUrlClicked(url, actualText) == true
+        }
+    }).apply {
+        // We need also to fix the case when long click on link will trigger long click on cell
+        setOnLinkLongClickListener { tv, url ->
+            // Long clicks are handled by parent, return true to block android to do something with url
+            if (url.isValidUrl() && urlClickCallback?.onUrlLongClicked(url) == true) {
+                tv.dispatchTouchEvent(MotionEvent.obtain(0, 0, MotionEvent.ACTION_CANCEL, 0f, 0f, 0))
+                true
+            } else {
+                false
             }
+        }
+    }
 }

vector/src/main/res/values/strings_riotX.xml

@@ -21,7 +21,10 @@
 
 
     <!-- BEGIN Strings added by Onuray -->
-
+    <string name="external_link_confirmation_title">Double-check this link</string>
+    <string name="external_link_confirmation_message">The link %1$s is taking you to another site: %2$s. Are you sure you want to continue?</string>
+    <string name="external_link_confirmation_negative_button">Continue</string>
+    <string name="external_link_confirmation_positive_button">Cancel</string>
     <!-- END Strings added by Onuray -->