Ask the user to confirm urls which contain unicode direction overrides #6164
Conversation
… them to confirm the url
| @@ -1922,30 +1925,36 @@ class TimelineFragment @Inject constructor( | |||
| } | |||
| }) | |||
| if (!isManaged) { | |||
| if (title.isValidUrl() && url.isValidUrl() && URL(title).host != URL(url).host) { | |||
| MaterialAlertDialogBuilder(requireActivity(), R.style.ThemeOverlay_Vector_MaterialAlertDialog_NegativeDestructive) | |||
There was a problem hiding this comment.
the dialog displaying has been extracted to its own function
| openUrlInExternalBrowser(requireContext(), url) | ||
| when { | ||
| url.containsRtLOverride() || (title.isValidUrl() && url.isValidUrl() && URL(title).host != URL(url).host) -> { | ||
| displayUrlConfirmationDialog(title.ensureEndsLeftToRight(), url.filterDirectionOverrides()) |
There was a problem hiding this comment.
we have to ensure the text returns to LtR at the end with title.ensureEndsLeftToRight(), otherwise the override character will cause the rest of the string resource to also become inverted
| WITHOUT LTR OVERRIDE |
|---|
 |
There was a problem hiding this comment.
url.filterDirectionOverrides() will also be used to open the url, if the user accepts. Will it work fine? Or considerer adding url as a new param.
There was a problem hiding this comment.
ah! no it should be a separate parameter, will update 👍
ouchadam
requested review from
a team,
ganfra and
Florian14
and removed request for
a team
ouchadam
added
the
Z-NextRelease
| openUrlInExternalBrowser(requireContext(), url) | ||
| when { | ||
| url.containsRtLOverride() || (title.isValidUrl() && url.isValidUrl() && URL(title).host != URL(url).host) -> { | ||
| displayUrlConfirmationDialog(title.ensureEndsLeftToRight(), url.filterDirectionOverrides()) |
There was a problem hiding this comment.
url.filterDirectionOverrides() will also be used to open the url, if the user accepts. Will it work fine? Or considerer adding url as a new param.
| // Open in external browser, in a new Tab | ||
| openUrlInExternalBrowser(requireContext(), url) | ||
| when { | ||
| url.containsRtLOverride() || (title.isValidUrl() && url.isValidUrl() && URL(title).host != URL(url).host) -> { |
There was a problem hiding this comment.
Maybe split into 2 cases rather than using || for clarity?
kind of~ after looking into the logic, the url detection only takes into account urls with a protocol, for example |
Matrix SDKIntegration Tests Results:
|
Type of change
Content
Checks for the presence of Right to Left unicode directional overrides when tapping on a URL, if one is found we'll show the user the URL with the overrides disabled and ask them to confirm they want to continue.
Motivation and context
Fixes #6163
Newer versions of Android (31+) seemingly handle the oddness out of the box by avoiding marking the URLs as clickable links, however previous versions will still attempt to link out to them, although the formatting is often broken.
Screenshots / GIFs
Tests
\u202ETested devices