Skip to content

Commit

Permalink
[iOS] - Fix URL-Bar LTR-RTL Clipping (#26360)
Browse files Browse the repository at this point in the history
- Fix URL-Bar LTR-RTL
- Fix display of URL in the Page Security View
- Use ICU in Chromium, to determine text direction and base direction
  • Loading branch information
Brandon-T committed Nov 29, 2024
1 parent da6750a commit ad34702
Show file tree
Hide file tree
Showing 12 changed files with 350 additions and 36 deletions.
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/unicode/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 @@ -124,6 +125,7 @@ brave_core_public_headers += browser_api_features_public_headers
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 += browser_api_unicode_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 isRenderedLeftToRight = url.isRenderedLeftToRight

// 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.isUnidirectional

var isLTR = isRenderedLeftToRight && !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
49 changes: 49 additions & 0 deletions ios/brave-ios/Sources/BraveShared/Extensions/URLExtensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,55 @@ extension URL {

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

/// Determines the predominant directionality of the URL's text as defined:
/// - Left-To-Right - if a string has a strong LTR directionality, it would be laid out LTR.
/// - Right-To-Left - if a string has a strong RTL directionality, it would be laid out RTL.
/// - Mixed - if a string contains BOTH a strong LTR and RTL directionality
/// - Neutral - if a string contains NEITHER a strong LTR or RTL directionality (empty string)
/// Therefore, a string is `Uni-directional`, if it is LTR or RTL, but not both.
public var isUnidirectional: Bool {
// First format the URL which will decode the puny-coding
var renderedString = URLFormatter.formatURL(
absoluteString,
formatTypes: [.omitDefaults, .omitTrivialSubdomains, .omitTrailingSlashOnBareHostname],
unescapeOptions: .normal
)

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

return renderedString.bidiDirection != .mixed
}

/// Determines whether the URL would typically be rendered Left-To-Right instead of Right-To-Left
public var isRenderedLeftToRight: Bool {
// First format the URL which will decode the puny-coding
var renderedString = URLFormatter.formatURL(
absoluteString,
formatTypes: [.omitDefaults, .omitTrivialSubdomains, .omitTrailingSlashOnBareHostname],
unescapeOptions: .normal
)

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

return renderedString.bidiBaseDirection == .leftToRight
}
}

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

0 comments on commit ad34702

Please sign in to comment.