Skip to content

[PM-9388] Add new device type for DuckDuckGo browser #14708

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.

Sign up for GitHub

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
+150 −48
Open

[PM-9388] Add new device type for DuckDuckGo browser #14708

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
96ab816
Add new device type for DuckDuckGo browser
trmartin4 May 9, 2025
ba824c8
Added feature support property for sync domains
trmartin4 May 10, 2025
8d53b10
Added new features
trmartin4 May 10, 2025
abfeeac
Added isDuckDuckGo() to CLI
trmartin4 May 10, 2025
106b075
Addressed PR feedback.
trmartin4 May 17, 2025
8f9452d
Renamed new property
trmartin4 May 17, 2025
147ecd8
Fixed rename that missed CLI.
trmartin4 May 17, 2025
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
8 changes: 8 additions & 0 deletions apps/browser/src/platform/services/platform-utils/browser-platform-utils.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,10 @@
return this.getDevice() === DeviceType.SafariExtension;
}

isDuckDuckGo(): boolean {
return false;

Check warning on line 155 in apps/browser/src/platform/services/platform-utils/browser-platform-utils.service.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/platform/services/platform-utils/browser-platform-utils.service.ts#L155 

Added line #L155 was not covered by tests
}

isIE(): boolean {
return false;
}
Expand Down Expand Up @@ -221,6 +225,10 @@
return true;
}

supportsSyncDomains(): boolean {
Copy link
Member

Choose a reason for hiding this comment

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

I think a better name for this is supportsAutofill my dream would be that this would live on something autofill owned as well but this needs to be used pretty low level and I don't know of a great place it right now so we can leave it.

return true;

Check warning on line 229 in apps/browser/src/platform/services/platform-utils/browser-platform-utils.service.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/platform/services/platform-utils/browser-platform-utils.service.ts#L229 

Added line #L229 was not covered by tests
}

abstract showToast(
type: "error" | "success" | "warning" | "info",
title: string,
Expand Down
8 changes: 8 additions & 0 deletions apps/cli/src/platform/services/cli-platform-utils.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@
return false;
}

isDuckDuckGo(): boolean {
return false;

Check warning on line 76 in apps/cli/src/platform/services/cli-platform-utils.service.ts

View check run for this annotation

Codecov / codecov/patch

apps/cli/src/platform/services/cli-platform-utils.service.ts#L76 

Added line #L76 was not covered by tests
}

isMacAppStore() {
return false;
}
Expand Down Expand Up @@ -108,6 +112,10 @@
return false;
}

supportsSyncDomains(): boolean {
return false;

Check warning on line 116 in apps/cli/src/platform/services/cli-platform-utils.service.ts

View check run for this annotation

Codecov / codecov/patch

apps/cli/src/platform/services/cli-platform-utils.service.ts#L116 

Added line #L116 was not covered by tests
}

showToast(
type: "error" | "success" | "warning" | "info",
title: string,
Expand Down
8 changes: 8 additions & 0 deletions apps/desktop/src/platform/services/electron-platform-utils.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@
return false;
}

isDuckDuckGo(): boolean {
return false;

Check warning on line 59 in apps/desktop/src/platform/services/electron-platform-utils.service.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/platform/services/electron-platform-utils.service.ts#L59 

Added line #L59 was not covered by tests
}

isMacAppStore(): boolean {
return ipc.platform.isMacAppStore;
}
Expand Down Expand Up @@ -86,6 +90,10 @@
return true;
}

supportsSyncDomains(): boolean {
return false;

Check warning on line 94 in apps/desktop/src/platform/services/electron-platform-utils.service.ts

View check run for this annotation

Codecov / codecov/patch

apps/desktop/src/platform/services/electron-platform-utils.service.ts#L94 

Added line #L94 was not covered by tests
}

showToast(
type: "error" | "success" | "warning" | "info",
title: string,
Expand Down
2 changes: 1 addition & 1 deletion apps/web/src/app/core/web-file-download.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export class WebFileDownloadService implements FileDownloadService {
download(request: FileDownloadRequest): void {
const builder = new FileDownloadBuilder(request);
const a = window.document.createElement("a");
if (!this.platformUtilsService.isSafari()) {
if (!(this.platformUtilsService.isSafari() || this.platformUtilsService.isDuckDuckGo())) {
Copy link
Member

Choose a reason for hiding this comment

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

If we ever accurately identified DuckDuckGo on windows this logic might break down. I think a lot of our isSafari checks should actually be isWebkit checks and I think this one is too. Since DDG on windows is Edge based this logic would become wrong. It should be fine for now but I think we should get a ticket on the backlog for transitioning isSafari checks to capability based checks.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've created a backlog ticket for converting isSafari() to capability-based checks. For this particular check, I converted this to supportsFileDownload(), as that is what we're trying to do here, I believe; my research tells me that Safari's respect of the download property for file downloads is flaky, and so we have to have the backup of opening in a new tab. That's what I tried to express here.

a.rel = "noreferrer";
a.target = "_blank";
}
Expand Down
25 changes: 25 additions & 0 deletions apps/web/src/app/core/web-platform-utils.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { DeviceType } from "@bitwarden/common/enums";

import { WebPlatformUtilsService } from "./web-platform-utils.service";

describe("Web Platform Utils Service", () => {
Expand Down Expand Up @@ -114,4 +116,27 @@ describe("Web Platform Utils Service", () => {
expect(result).toBe("2022.10.2");
});
});
describe("getDevice", () => {
const originalUserAgent = navigator.userAgent;

const setUserAgent = (userAgent: string) => {
Object.defineProperty(navigator, "userAgent", {
value: userAgent,
configurable: true,
});
};

afterEach(() => {
// Reset to original after each test
setUserAgent(originalUserAgent);
});

test("returns DuckDuckGo browser with example User-Agent", () => {
setUserAgent(
"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/18.3.1 Safari/605.1.15 Ddg/18.3.1",
Copy link
Member

Choose a reason for hiding this comment

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

I think a generalization of this test would be useful so we can define a lot of inputs and the expected browser a little easier.

const testData: { userAgent: string, expectedDevice: DeviceType }[] = [
  {
    userAgent: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/18.3.1 Safari/605.1.15 Ddg/18.3.1",
    expectedDevice: DeviceType.DuckDuckGoBrowser,
  }
];

Also some comments about what the user agent was taken from version, OS, browser type for example. One of the test cases could be Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/135.0.0.0 Safari/537.36 Edg/135.0.0.0 which I just grabbed from Duck Duck Go's windows browser v0.109.7 which I'm pretty sure will report Edge. I'd have it include a comment saying that it is expected to be Edge right now but if something changes we'd be happy to have the expected device type become more accurate later on.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added additional tests for all the different browsers, based on current user-agent strings.

);
const result = webPlatformUtilsService.getDevice();
expect(result).toBe(DeviceType.DuckDuckGoBrowser);
});
});
});
13 changes: 13 additions & 0 deletions apps/web/src/app/core/web-platform-utils.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@
this.browserCache = DeviceType.EdgeBrowser;
} else if (navigator.userAgent.indexOf(" Vivaldi/") !== -1) {
this.browserCache = DeviceType.VivaldiBrowser;
} else if (
navigator.userAgent.indexOf(" Safari/") !== -1 &&
navigator.userAgent.indexOf("Ddg") !== -1
) {
this.browserCache = DeviceType.DuckDuckGoBrowser;
} else if (
navigator.userAgent.indexOf(" Safari/") !== -1 &&
navigator.userAgent.indexOf("Chrome") === -1
Expand Down Expand Up @@ -83,6 +88,10 @@
return this.getDevice() === DeviceType.SafariBrowser;
}

isDuckDuckGo(): boolean {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should add these anymore. The ideal is that no team needs to do specific client checks and even if they do the getDevice() check already exists.

return this.getDevice() === DeviceType.DuckDuckGoBrowser;

Check warning on line 92 in apps/web/src/app/core/web-platform-utils.service.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/core/web-platform-utils.service.ts#L92 

Added line #L92 was not covered by tests
}

isMacAppStore(): boolean {
return false;
}
Expand Down Expand Up @@ -120,6 +129,10 @@
return true;
}

supportsSyncDomains(): boolean {
return false;

Check warning on line 133 in apps/web/src/app/core/web-platform-utils.service.ts

View check run for this annotation

Codecov / codecov/patch

apps/web/src/app/core/web-platform-utils.service.ts#L133 

Added line #L133 was not covered by tests
}

showToast(
type: "error" | "success" | "warning" | "info",
title: string,
Expand Down
18 changes: 0 additions & 18 deletions bitwarden_license/bit-web/src/app/secrets-manager/shared/secrets-list.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,22 +179,4 @@ export class SecretsListComponent implements OnDestroy {
i18nService.t("valueCopied", i18nService.t("uuid")),
);
}

/**
* TODO: Remove in favor of updating `PlatformUtilsService.copyToClipboard`
*/
private static copyToClipboardAsync(
text: Promise<string>,
platformUtilsService: PlatformUtilsService,
) {
if (platformUtilsService.isSafari()) {
return navigator.clipboard.write([
new ClipboardItem({
["text/plain"]: text,
}),
]);
}

return text.then((t) => platformUtilsService.copyToClipboard(t));
}
}
8 changes: 0 additions & 8 deletions libs/angular/src/tools/send/add-edit.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,14 +146,6 @@ export class AddEditComponent implements OnInit, OnDestroy {
return null;
}

get isSafari() {
return this.platformUtilsService.isSafari();
}

get isDateTimeLocalSupported(): boolean {
return !(this.platformUtilsService.isFirefox() || this.platformUtilsService.isSafari());
}

async ngOnInit() {
this.accountService.activeAccount$
.pipe(
Expand Down
2 changes: 2 additions & 0 deletions libs/common/src/enums/device-type.enum.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export enum DeviceType {
WindowsCLI = 23,
MacOsCLI = 24,
LinuxCLI = 25,
DuckDuckGoBrowser = 26,
}

/**
Expand Down Expand Up @@ -53,6 +54,7 @@ export const DeviceTypeMetadata: Record<DeviceType, DeviceTypeMetadata> = {
[DeviceType.IEBrowser]: { category: "webVault", platform: "IE" },
[DeviceType.SafariBrowser]: { category: "webVault", platform: "Safari" },
[DeviceType.VivaldiBrowser]: { category: "webVault", platform: "Vivaldi" },
[DeviceType.DuckDuckGoBrowser]: { category: "webVault", platform: "DuckDuckGo" },
[DeviceType.UnknownBrowser]: { category: "webVault", platform: "Unknown" },
[DeviceType.WindowsDesktop]: { category: "desktop", platform: "Windows" },
[DeviceType.MacOsDesktop]: { category: "desktop", platform: "macOS" },
Expand Down
2 changes: 2 additions & 0 deletions libs/common/src/platform/abstractions/platform-utils.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,15 @@ export abstract class PlatformUtilsService {
abstract isOpera(): boolean;
abstract isVivaldi(): boolean;
abstract isSafari(): boolean;
abstract isDuckDuckGo(): boolean;
abstract isMacAppStore(): boolean;
abstract isViewOpen(): Promise<boolean>;
abstract launchUri(uri: string, options?: any): void;
abstract getApplicationVersion(): Promise<string>;
abstract getApplicationVersionNumber(): Promise<string>;
abstract supportsWebAuthn(win: Window): boolean;
abstract supportsDuo(): boolean;
abstract supportsSyncDomains(): boolean;
/**
* @deprecated use `@bitwarden/components/ToastService.showToast` instead
*
Expand Down
26 changes: 5 additions & 21 deletions libs/common/src/services/api.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ import { PaymentResponse } from "../billing/models/response/payment.response";
import { PlanResponse } from "../billing/models/response/plan.response";
import { SubscriptionResponse } from "../billing/models/response/subscription.response";
import { TaxInfoResponse } from "../billing/models/response/tax-info.response";
import { DeviceType } from "../enums";
import { ClientType, DeviceType } from "../enums";
import { KeyConnectorUserKeyRequest } from "../key-management/key-connector/models/key-connector-user-key.request";
import { SetKeyConnectorKeyRequest } from "../key-management/key-connector/models/set-key-connector-key.request";
import { VaultTimeoutSettingsService } from "../key-management/vault-timeout";
Expand Down Expand Up @@ -147,8 +147,6 @@ import { OptionalCipherResponse } from "../vault/models/response/optional-cipher
export class ApiService implements ApiServiceAbstraction {
private device: DeviceType;
private deviceType: string;
private isWebClient = false;
private isDesktopClient = false;
private refreshTokenPromise: Promise<string> | undefined;

/**
Expand All @@ -170,22 +168,6 @@ export class ApiService implements ApiServiceAbstraction {
) {
this.device = platformUtilsService.getDevice();
this.deviceType = this.device.toString();
this.isWebClient =
this.device === DeviceType.IEBrowser ||
this.device === DeviceType.ChromeBrowser ||
this.device === DeviceType.EdgeBrowser ||
this.device === DeviceType.FirefoxBrowser ||
this.device === DeviceType.OperaBrowser ||
this.device === DeviceType.SafariBrowser ||
this.device === DeviceType.UnknownBrowser ||
this.device === DeviceType.VivaldiBrowser;
this.isDesktopClient =
this.device === DeviceType.WindowsDesktop ||
this.device === DeviceType.MacOsDesktop ||
this.device === DeviceType.LinuxDesktop ||
this.device === DeviceType.WindowsCLI ||
this.device === DeviceType.MacOsCLI ||
this.device === DeviceType.LinuxCLI;
}

// Auth APIs
Expand Down Expand Up @@ -875,7 +857,9 @@ export class ApiService implements ApiServiceAbstraction {
// Sync APIs

async getSync(): Promise<SyncResponse> {
const path = this.isDesktopClient || this.isWebClient ? "/sync?excludeDomains=true" : "/sync";
const path = !this.platformUtilsService.supportsSyncDomains()
? "/sync?excludeDomains=true"
: "/sync";
const r = await this.send("GET", path, null, true, true);
return new SyncResponse(r);
}
Expand Down Expand Up @@ -1919,7 +1903,7 @@ export class ApiService implements ApiServiceAbstraction {

private async getCredentials(): Promise<RequestCredentials> {
const env = await firstValueFrom(this.environmentService.environment$);
if (!this.isWebClient || env.hasBaseUrl()) {
if (this.platformUtilsService.getClientType() !== ClientType.Web || env.hasBaseUrl()) {
return "include";
}
return undefined;
Expand Down
Loading