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 - 1.73.x (#26360) #26789

Merged
merged 1 commit into from
Nov 29, 2024
Merged
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/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
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
Loading