diff --git a/CHANGES.md b/CHANGES.md index 3bb4dae02e..550d013ba3 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -23,6 +23,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) - Cross-Signing | Consider not using a spinner on the 'complete security' prompt (#1271) - Restart broken Olm sessions ([MSC1719](https://github.com/matrix-org/matrix-doc/pull/1719)) - Cross-Signing | Hide Use recovery key when 4S is not setup (#1007) diff --git a/vector/src/main/java/im/vector/riotx/core/utils/EvenBetterLinkMovementMethod.kt b/vector/src/main/java/im/vector/riotx/core/utils/EvenBetterLinkMovementMethod.kt new file mode 100644 index 0000000000..1e565c0f4b --- /dev/null +++ b/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) + } + } +} diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailFragment.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailFragment.kt index f34bc2076e..150c0d1979 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailFragment.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailFragment.kt @@ -39,6 +39,7 @@ import androidx.core.app.ActivityOptionsCompat import androidx.core.content.ContextCompat import androidx.core.net.toUri import androidx.core.text.buildSpannedString +import androidx.core.text.toSpannable import androidx.core.util.Pair import androidx.core.view.ViewCompat import androidx.core.view.forEach @@ -110,9 +111,11 @@ import im.vector.riotx.core.utils.PERMISSION_REQUEST_CODE_PICK_ATTACHMENT import im.vector.riotx.core.utils.TextUtils import im.vector.riotx.core.utils.allGranted import im.vector.riotx.core.utils.checkPermissions +import im.vector.riotx.core.utils.colorizeMatchingText 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 @@ -167,6 +170,7 @@ import org.billcarsonfr.jsonviewer.JSonViewerDialog import org.commonmark.parser.Parser import timber.log.Timber import java.io.File +import java.net.URL import java.util.concurrent.TimeUnit import javax.inject.Inject @@ -919,7 +923,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 +951,25 @@ 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() && url.isValidUrl() && URL(title).host != URL(url).host) { + AlertDialog.Builder(requireActivity()) + .setTitle(R.string.external_link_confirmation_title) + .setMessage( + getString(R.string.external_link_confirmation_message, title, url) + .toSpannable() + .colorizeMatchingText(url, colorProvider.getColorFromAttribute(android.R.attr.textColorLink)) + .colorizeMatchingText(title, colorProvider.getColorFromAttribute(android.R.attr.textColorLink)) + ) + .setPositiveButton(R.string._continue) { _, _ -> + openUrlInExternalBrowser(requireContext(), url) + } + .setNegativeButton(R.string.cancel, null) + .show() + .withColoredButton(DialogInterface.BUTTON_NEGATIVE) + } else { + // Open in external browser, in a new Tab + openUrlInExternalBrowser(requireContext(), url) + } } } .disposeOnDestroyView() @@ -957,7 +978,7 @@ class RoomDetailFragment @Inject constructor( } override fun onUrlLongClicked(url: String): Boolean { - if (url != getString(R.string.edited_suffix)) { + if (url != getString(R.string.edited_suffix) && url.isValidUrl()) { // Copy the url to the clipboard copyToClipboard(requireContext(), url, true, R.string.link_copied_to_clipboard) } @@ -1263,7 +1284,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) diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/TimelineEventController.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/TimelineEventController.kt index 48e92ca438..addbfab43c 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/TimelineEventController.kt +++ b/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 } diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/action/EventSharedAction.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/action/EventSharedAction.kt index f362b7939c..b141e9a8cd 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/action/EventSharedAction.kt +++ b/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 diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/action/MessageActionsBottomSheet.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/action/MessageActionsBottomSheet.kt index 1336c61b68..eeb090092c 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/action/MessageActionsBottomSheet.kt +++ b/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 } diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/tools/EventRenderingTools.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/tools/EventRenderingTools.kt index 4e9959eda6..097a95bb27 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/tools/EventRenderingTools.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/timeline/tools/EventRenderingTools.kt @@ -17,19 +17,20 @@ 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.isValidUrl +import im.vector.riotx.core.utils.EvenBetterLinkMovementMethod import im.vector.riotx.features.home.room.detail.timeline.TimelineEventController import im.vector.riotx.features.html.PillImageSpan 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 +43,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,18 +56,17 @@ 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() +fun createLinkMovementMethod(urlClickCallback: TimelineEventController.UrlClickCallback?): EvenBetterLinkMovementMethod { + return EvenBetterLinkMovementMethod(object : EvenBetterLinkMovementMethod.OnLinkClickListener { + override fun onLinkClicked(textView: TextView, span: ClickableSpan, url: String, actualText: String): Boolean { + return urlClickCallback?.onUrlClicked(url, actualText) == true + } + }) .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) { + if (urlClickCallback?.onUrlLongClicked(url) == true) { tv.dispatchTouchEvent(MotionEvent.obtain(0, 0, MotionEvent.ACTION_CANCEL, 0f, 0f, 0)) true } else { diff --git a/vector/src/main/res/values/strings_riotX.xml b/vector/src/main/res/values/strings_riotX.xml index 56456aaf5d..8b675ee8c1 100644 --- a/vector/src/main/res/values/strings_riotX.xml +++ b/vector/src/main/res/values/strings_riotX.xml @@ -34,7 +34,8 @@ - + Double-check this link + The link %1$s is taking you to another site: %2$s.\n\nAre you sure you want to continue?