fix: address browser import review feedback

This commit is contained in:
Lawrence Chen 2026-03-16 22:30:09 -07:00
parent 92cb42262c
commit dc6bcb259a
No known key found for this signature in database
5 changed files with 145 additions and 39 deletions

View file

@ -4734,6 +4734,23 @@
}
}
},
"browser.import.additionalData": {
"extractionState": "manual",
"localizations": {
"en": {
"stringUnit": {
"state": "translated",
"value": "Additional data (bookmarks, settings, extensions)"
}
},
"ja": {
"stringUnit": {
"state": "translated",
"value": "追加データ(ブックマーク、設定、拡張機能)"
}
}
}
},
"browser.import.back": {
"extractionState": "manual",
"localizations": {
@ -5142,7 +5159,24 @@
}
}
},
"browser.import.detected.more": {
"browser.import.detected.more.one": {
"extractionState": "manual",
"localizations": {
"en": {
"stringUnit": {
"state": "translated",
"value": "Detected: %@, +1 more."
}
},
"ja": {
"stringUnit": {
"state": "translated",
"value": "検出済み: %@、ほか1件。"
}
}
}
},
"browser.import.detected.more.other": {
"extractionState": "manual",
"localizations": {
"en": {

View file

@ -12,6 +12,18 @@ import CommonCrypto
import Security
#endif
fileprivate func dedupedCanonicalURLs(_ urls: [URL]) -> [URL] {
var seen = Set<String>()
var result: [URL] = []
for url in urls {
let canonical = url.standardizedFileURL.resolvingSymlinksInPath().path
if seen.insert(canonical).inserted {
result.append(url)
}
}
return result
}
enum GhosttyBackgroundTheme {
static func clampedOpacity(_ opacity: Double) -> CGFloat {
CGFloat(max(0.0, min(1.0, opacity)))
@ -5532,10 +5544,10 @@ enum BrowserImportScope: String, CaseIterable, Identifiable {
includeHistory: Bool,
includeAdditionalData: Bool
) -> BrowserImportScope? {
guard includeCookies || includeHistory else { return nil }
if includeAdditionalData {
return .everything
}
guard includeCookies || includeHistory else { return nil }
if includeCookies && includeHistory {
return .cookiesAndHistory
}
@ -5949,13 +5961,23 @@ enum InstalledBrowserDetector {
)
}
let shown = names.prefix(limit).joined(separator: ", ")
let remaining = names.count - limit
if remaining == 1 {
return String(
format: String(
localized: "browser.import.detected.more.one",
defaultValue: "Detected: %@, +1 more."
),
shown
)
}
return String(
format: String(
localized: "browser.import.detected.more",
localized: "browser.import.detected.more.other",
defaultValue: "Detected: %@, +%ld more."
),
shown,
names.count - limit
remaining
)
}
@ -6444,18 +6466,6 @@ enum InstalledBrowserDetector {
]
}
private static func dedupedCanonicalURLs(_ urls: [URL]) -> [URL] {
var seen = Set<String>()
var result: [URL] = []
for url in urls {
let canonical = url.standardizedFileURL.resolvingSymlinksInPath().path
if seen.insert(canonical).inserted {
result.append(url)
}
}
return result
}
private static func dedupedProfiles(_ profiles: [InstalledBrowserProfile]) -> [InstalledBrowserProfile] {
var seen = Set<String>()
var result: [InstalledBrowserProfile] = []
@ -7582,7 +7592,7 @@ enum BrowserDataImporter {
domainMatches(host: host, filters: domainFilters) else {
return
}
let lastVisited = firefoxDate(fromUnixMicroseconds: lastVisitMicros) ?? Date()
let lastVisited = firefoxDate(fromUnixMicroseconds: lastVisitMicros) ?? .distantPast
rows.append(HistoryRow(url: url, title: title, visitCount: visitCount, lastVisited: lastVisited))
}
} catch {
@ -7638,7 +7648,7 @@ enum BrowserDataImporter {
domainMatches(host: host, filters: domainFilters) else {
return
}
let lastVisited = chromiumDate(fromWebKitMicroseconds: lastVisitMicros) ?? Date()
let lastVisited = chromiumDate(fromWebKitMicroseconds: lastVisitMicros) ?? .distantPast
rows.append(HistoryRow(url: url, title: title, visitCount: visitCount, lastVisited: lastVisited))
}
} catch {
@ -7930,18 +7940,6 @@ enum BrowserDataImporter {
}
return Data(bytes: pointer, count: length)
}
private static func dedupedCanonicalURLs(_ urls: [URL]) -> [URL] {
var seen = Set<String>()
var result: [URL] = []
for url in urls {
let canonical = url.standardizedFileURL.resolvingSymlinksInPath().path
if seen.insert(canonical).inserted {
result.append(url)
}
}
return result
}
}
#if DEBUG
@ -8279,6 +8277,7 @@ final class BrowserDataImportCoordinator {
private let cookiesCheckbox = NSButton(checkboxWithTitle: "", target: nil, action: nil)
private let historyCheckbox = NSButton(checkboxWithTitle: "", target: nil, action: nil)
private let additionalDataCheckbox = NSButton(checkboxWithTitle: "", target: nil, action: nil)
private let domainField = NSTextField(frame: .zero)
private let backButton = NSButton(title: "", target: nil, action: nil)
@ -8373,10 +8372,11 @@ final class BrowserDataImportCoordinator {
case .dataTypes:
let includeCookies = cookiesCheckbox.state == .on
let includeHistory = historyCheckbox.state == .on
let includeAdditionalData = additionalDataCheckbox.state == .on
guard let scope = BrowserImportScope.fromSelection(
includeCookies: includeCookies,
includeHistory: includeHistory,
includeAdditionalData: false
includeAdditionalData: includeAdditionalData
) else {
validationLabel.stringValue = String(
localized: "browser.import.validation.scope",
@ -8632,6 +8632,7 @@ final class BrowserDataImportCoordinator {
private func setupDataTypesContainer() {
cookiesCheckbox.state = .on
historyCheckbox.state = .on
additionalDataCheckbox.state = .off
cookiesCheckbox.title = String(
localized: "browser.import.cookies",
defaultValue: "Cookies (site sign-ins)"
@ -8640,6 +8641,13 @@ final class BrowserDataImportCoordinator {
localized: "browser.import.history",
defaultValue: "History (visited pages)"
)
additionalDataCheckbox.title = String(
localized: "browser.import.additionalData",
defaultValue: "Additional data (bookmarks, settings, extensions)"
)
cookiesCheckbox.setAccessibilityIdentifier("BrowserImportCookiesCheckbox")
historyCheckbox.setAccessibilityIdentifier("BrowserImportHistoryCheckbox")
additionalDataCheckbox.setAccessibilityIdentifier("BrowserImportAdditionalDataCheckbox")
separateProfilesRadio.title = String(
localized: "browser.import.destinationMode.separate",
defaultValue: "Keep profiles separate"
@ -8721,6 +8729,7 @@ final class BrowserDataImportCoordinator {
dataTypesContainer.addArrangedSubview(destinationHelpLabel)
dataTypesContainer.addArrangedSubview(cookiesCheckbox)
dataTypesContainer.addArrangedSubview(historyCheckbox)
dataTypesContainer.addArrangedSubview(additionalDataCheckbox)
dataTypesContainer.addArrangedSubview(domainRow)
dataTypesContainer.addArrangedSubview(noteLabel)
}

View file

@ -226,6 +226,8 @@ struct BrowserPanelView: View {
@State private var latestRemoteSuggestionQuery: String = ""
@State private var latestRemoteSuggestions: [String] = []
@State private var emptyStateImportBrowsers: [InstalledBrowserCandidate] = []
@State private var emptyStateImportBrowserRefreshTask: Task<Void, Never>?
@State private var emptyStateImportBrowserRefreshGeneration: UInt64 = 0
@State private var inlineCompletion: OmnibarInlineCompletion?
@State private var omnibarSelectionRange: NSRange = NSRange(location: NSNotFound, length: 0)
@State private var omnibarHasMarkedText: Bool = false
@ -981,12 +983,12 @@ struct BrowserPanelView: View {
if addressBarFocused {
setAddressBarFocused(false, reason: "placeholderContent.tapBlur")
}
}
}
}
.overlay {
if isWebViewBlank() {
emptyBrowserStateOverlay
}
.overlay {
if shouldShowEmptyStateImportOverlay {
emptyBrowserStateOverlay
}
}
}
}
.frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading)
@ -1245,7 +1247,6 @@ struct BrowserPanelView: View {
.fixedSize(horizontal: false, vertical: true)
Button(String(localized: "settings.browser.emptyImport.choose", defaultValue: "Choose What to Import…")) {
refreshEmptyStateImportBrowsers()
BrowserDataImportCoordinator.shared.presentImportDialog(
defaultDestinationProfileID: panel.profileID
)
@ -1272,6 +1273,10 @@ struct BrowserPanelView: View {
.padding(.horizontal, 18)
}
private var shouldShowEmptyStateImportOverlay: Bool {
!panel.shouldRenderWebView && isWebViewBlank()
}
/// Treat a WebView with no URL (or about:blank) as "blank" for UX purposes.
private func isWebViewBlank() -> Bool {
guard let url = panel.webView.url else { return true }
@ -1324,7 +1329,28 @@ struct BrowserPanelView: View {
}
private func refreshEmptyStateImportBrowsers() {
emptyStateImportBrowsers = InstalledBrowserDetector.detectInstalledBrowsers()
emptyStateImportBrowserRefreshTask?.cancel()
emptyStateImportBrowserRefreshGeneration &+= 1
let generation = emptyStateImportBrowserRefreshGeneration
guard shouldShowEmptyStateImportOverlay else {
emptyStateImportBrowsers = []
emptyStateImportBrowserRefreshTask = nil
return
}
emptyStateImportBrowserRefreshTask = Task {
let browsers = await Task.detached(priority: .utility) {
InstalledBrowserDetector.detectInstalledBrowsers()
}.value
guard !Task.isCancelled else { return }
await MainActor.run {
guard emptyStateImportBrowserRefreshGeneration == generation,
shouldShowEmptyStateImportOverlay else { return }
emptyStateImportBrowsers = browsers
emptyStateImportBrowserRefreshTask = nil
}
}
}
private func openDevTools() {

View file

@ -1751,6 +1751,15 @@ final class BrowserImportScopeTests: XCTestCase {
XCTAssertEqual(scope, .cookiesAndHistory)
}
func testFromSelectionEverything() {
let scope = BrowserImportScope.fromSelection(
includeCookies: false,
includeHistory: false,
includeAdditionalData: true
)
XCTAssertEqual(scope, .everything)
}
func testFromSelectionRejectsEmptySelection() {
let scope = BrowserImportScope.fromSelection(
includeCookies: false,

View file

@ -70,6 +70,34 @@ final class BrowserImportProfilesUITests: XCTestCase {
XCTAssertEqual(entries[0]["destinationName"] as? String, "Default")
}
func testAdditionalDataSelectionCapturesEverythingScope() throws {
let app = launchApp()
openImportWizard(app)
app.buttons["Next"].click()
app.buttons["Next"].click()
let cookiesCheckbox = app.checkBoxes["BrowserImportCookiesCheckbox"]
XCTAssertTrue(cookiesCheckbox.waitForExistence(timeout: 5.0))
cookiesCheckbox.click()
let historyCheckbox = app.checkBoxes["BrowserImportHistoryCheckbox"]
XCTAssertTrue(historyCheckbox.waitForExistence(timeout: 5.0))
historyCheckbox.click()
let additionalDataCheckbox = app.checkBoxes["BrowserImportAdditionalDataCheckbox"]
XCTAssertTrue(
additionalDataCheckbox.waitForExistence(timeout: 5.0),
"Expected Step 3 to expose the additional data checkbox"
)
additionalDataCheckbox.click()
app.buttons["Start Import"].click()
let capture = try XCTUnwrap(waitForCapturedSelection(timeout: 5.0))
XCTAssertEqual(capture["scope"] as? String, "everything")
}
private func launchApp() -> XCUIApplication {
let app = XCUIApplication()
app.launchEnvironment["CMUX_UI_TEST_MODE"] = "1"