Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[iOS] - Fix URL-Bar LTR-RTL Clipping #26360

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ios/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import("//brave/ios/browser/api/query_filter/headers.gni")
import("//brave/ios/browser/api/session_restore/headers.gni")
import("//brave/ios/browser/api/skus/headers.gni")
import("//brave/ios/browser/api/storekit_receipt/headers.gni")
import("//brave/ios/browser/api/string/headers.gni")
import("//brave/ios/browser/api/url/headers.gni")
import("//brave/ios/browser/api/url_sanitizer/headers.gni")
import("//brave/ios/browser/api/web/ui/headers.gni")
Expand Down Expand Up @@ -127,6 +128,7 @@ brave_core_public_headers += credential_provider_public_headers
brave_core_public_headers += developer_options_code_public_headers
brave_core_public_headers += browser_api_storekit_receipt_public_headers
brave_core_public_headers += webcompat_reporter_public_headers
brave_core_public_headers += browser_api_string_public_headers

action("brave_core_umbrella_header") {
script = "//build/config/ios/generate_umbrella_header.py"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,10 @@ struct HistoryItemView: View {
forDisplayOmitSchemePathAndTrivialSubdomains: url.absoluteString
)
)
.truncationMode(.tail)
.font(.footnote)
.frame(maxWidth: .infinity, alignment: .leading)
.fixedSize(horizontal: false, vertical: true)
.foregroundStyle(Color(braveSystemName: .textSecondary))
.environment(\.layoutDirection, .leftToRight)
.flipsForRightToLeftLayoutDirection(false)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
// file, You can obtain one at https://mozilla.org/MPL/2.0/.

import BraveCore
import BraveShared
import BraveUI
import Foundation
import Shared
Expand Down Expand Up @@ -63,9 +64,12 @@ struct PageSecurityView: View {
var body: some View {
VStack(alignment: .leading, spacing: 0) {
VStack(alignment: .leading, spacing: 16) {
Text(displayURL)
URLElidedText(text: displayURL)
.font(.headline)
.foregroundStyle(Color(braveSystemName: .textPrimary))
.frame(maxWidth: .infinity, alignment: .leading)
.fixedSize(horizontal: false, vertical: true)

HStack(alignment: .firstTextBaseline) {
warningIcon
VStack(alignment: .leading, spacing: 4) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// file, You can obtain one at https://mozilla.org/MPL/2.0/.

import BraveCore
import BraveShared
import BraveStrings
import Combine
import DesignSystem
Expand Down Expand Up @@ -456,7 +457,7 @@ class TabLocationView: UIView {

if let url = url {
if let internalURL = InternalURL(url), internalURL.isBasicAuthURL {
urlDisplayLabel.isWebScheme = false
urlDisplayLabel.isLeftToRight = true
urlDisplayLabel.text = Strings.PageSecurityView.signIntoWebsiteURLBarTitle
} else {
// Matches LocationBarModelImpl::GetFormattedURL in Chromium (except for omitHTTP)
Expand All @@ -466,21 +467,42 @@ class TabLocationView: UIView {
// If we can't parse the origin and the URL can't be classified via AutoCompleteClassifier
// the URL is likely a broken deceptive URL. Example: `about:blank#https://apple.com`
if URLOrigin(url: url).url == nil && URIFixup.getURL(url.absoluteString) == nil {
urlDisplayLabel.isWebScheme = false
urlDisplayLabel.isLeftToRight = true
urlDisplayLabel.text = ""
} else {
urlDisplayLabel.isWebScheme = ["http", "https"].contains(url.scheme ?? "")
urlDisplayLabel.text = URLFormatter.formatURL(
URLOrigin(url: url).url?.absoluteString ?? url.absoluteString,
formatTypes: [
.trimAfterHost, .omitHTTPS, .omitTrivialSubdomains,
.omitDefaults, .trimAfterHost, .omitHTTPS, .omitTrivialSubdomains,
],
unescapeOptions: .normal
)

// The direction the string will be rendered (this happens regardless of locale!!!)
// In a LTR environment, Arabic will always render RTL
// The dominant charset based on unicode's bidirection algorithm
// with strong L and strong R determines how a string will be rendered
let isLTRRendered = url.isLTRRendered

// Determine if the URL has BOTH LTR and RTL characters.
// Very likely a malicious URL, but not always!
// URLs like: m5155.xn--mgbaiqly6b2eg.xn--ngbc5azd/ are innocent
let isMixedCharset = url.hasMixedBidiDirection

var isLTR = isLTRRendered && !isMixedCharset
if isMixedCharset {
// FORCE LTR - ETLD are always the right most portion after the dot.
// We will force render the URL in LTR mode, so we should force clip on the left (sub-domain).
// RTL rendered domains will clip from the right side (sub-domain) and the ETLD will be rendered on the left.
isLTR = true
}

urlDisplayLabel.isLeftToRight =
!["http", "https"].contains(url.scheme ?? "") || !isLTR
}
}
} else {
urlDisplayLabel.isWebScheme = false
urlDisplayLabel.isLeftToRight = true
urlDisplayLabel.text = ""
}

Expand Down Expand Up @@ -548,9 +570,10 @@ private class DisplayURLLabel: UILabel {
}

private var textSize: CGSize = .zero
private var isRightToLeft: Bool = false
fileprivate var isWebScheme: Bool = false {
fileprivate var isLeftToRight: Bool = true {
didSet {
updateText()
updateTextSize()
updateClippingDirection()
setNeedsLayout()
setNeedsDisplay()
Expand All @@ -570,13 +593,24 @@ private class DisplayURLLabel: UILabel {
if oldValue != text {
updateText()
updateTextSize()
detectLanguageForNaturalDirectionClipping()
updateClippingDirection()
}
setNeedsDisplay()
}
}

private func updateTextSize() {
textSize = attributedText?.size() ?? .zero
setNeedsLayout()
setNeedsDisplay()
}

private func updateClippingDirection() {
// Update clipping fade direction
clippingFade.gradientLayer.startPoint = .init(x: isLeftToRight ? 1 : 0, y: 0.5)
clippingFade.gradientLayer.endPoint = .init(x: isLeftToRight ? 0 : 1, y: 0.5)
}

private func updateText() {
if let text = text {
// Without attributed string, the label will always render RTL characters even if you force LTR layout.
Expand All @@ -597,28 +631,6 @@ private class DisplayURLLabel: UILabel {
}
}

private func updateTextSize() {
textSize = attributedText?.size() ?? .zero
setNeedsLayout()
setNeedsDisplay()
}

private func detectLanguageForNaturalDirectionClipping() {
guard let text, let language = NLLanguageRecognizer.dominantLanguage(for: text) else { return }
switch language {
case .arabic, .hebrew, .persian, .urdu:
isRightToLeft = true
default:
isRightToLeft = false
}
}

private func updateClippingDirection() {
// Update clipping fade direction
clippingFade.gradientLayer.startPoint = .init(x: isRightToLeft || !isWebScheme ? 1 : 0, y: 0.5)
clippingFade.gradientLayer.endPoint = .init(x: isRightToLeft || !isWebScheme ? 0 : 1, y: 0.5)
}

@available(*, unavailable)
required init(coder: NSCoder) {
fatalError()
Expand All @@ -637,7 +649,7 @@ private class DisplayURLLabel: UILabel {
super.layoutSubviews()

clippingFade.frame = .init(
x: isRightToLeft || !isWebScheme ? bounds.width - 20 : 0,
x: isLeftToRight ? bounds.width - 20 : 0,
y: 0,
width: 20,
height: bounds.height
Expand All @@ -651,7 +663,7 @@ private class DisplayURLLabel: UILabel {
var rect = rect
if textSize.width > bounds.width {
let delta = (textSize.width - bounds.width)
if !isRightToLeft && isWebScheme {
if !isLeftToRight {
rect.origin.x -= delta
rect.size.width += delta
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,56 @@ extension URL {

return URL(string: "\(baseURL)?\(InternalURL.Param.url.rawValue)=\(encodedURL)")
}

public var hasMixedBidiDirection: Bool {
// First format the URL which will decode the puny-coding
let scheme = scheme ?? "http"
var renderedString = URLFormatter.formatURL(
absoluteString,
formatTypes: [.omitDefaults, .omitTrivialSubdomains, .omitTrailingSlashOnBareHostname],
unescapeOptions: .normal
)

// Strip prefixes
if let range = renderedString.range(of: "^(www|mobile|m)\\.", options: .regularExpression) {
renderedString.replaceSubrange(range, with: "")
}

// Strip scheme
if let range = renderedString.range(
of: "^(\(scheme)://|\(scheme):)",
options: .regularExpression
) {
renderedString.replaceSubrange(range, with: "")
}

return renderedString.bidiDirection == .MIXED
}

public var isLTRRendered: Bool {
// First format the URL which will decode the puny-coding
let scheme = scheme ?? "http"
var renderedString = URLFormatter.formatURL(
absoluteString,
formatTypes: [.omitDefaults, .omitTrivialSubdomains, .omitTrailingSlashOnBareHostname],
unescapeOptions: .normal
)

// Strip prefixes
if let range = renderedString.range(of: "^(www|mobile|m)\\.", options: .regularExpression) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't omitTrivialSubdomains above do the same?

renderedString.replaceSubrange(range, with: "")
}

// Strip scheme
if let range = renderedString.range(
of: "^(\(scheme)://|\(scheme):)",
options: .regularExpression
) {
renderedString.replaceSubrange(range, with: "")
}

return renderedString.bidiBaseDirection == .LTR
}
}

extension InternalURL {
Expand Down
3 changes: 3 additions & 0 deletions ios/brave-ios/Sources/BraveShared/URLElidedTextView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,8 @@ public struct URLElidedText: View {
attributes: .init([.font: font ?? .body, .paragraphStyle: paragraphStyle])
)
)
.truncationMode(.tail)
.environment(\.layoutDirection, .leftToRight)
.flipsForRightToLeftLayoutDirection(false)
}
}
Loading
Loading