-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[PM-5880] Refactor browser platform utils service to remove window
references
#7885
Changes from all commits
96dc70e
0eb8b71
c078292
30ee36f
8e66536
849e76a
da479e1
75e80d3
6a33ad6
b72c4d5
7861f31
eae9e5f
5499456
28dc5e9
6dfa863
cc0b9f2
e37b01f
8d6c7a6
faf95a9
339d783
0dc687d
3f15fce
bb3eb3d
19383f7
e5ecbf8
5d32c08
2a79001
90b7921
7b767c4
365f015
f6eff73
9b412da
71927f6
275d5c5
c46d47c
134e6b2
211c23d
2fdc709
ec96d29
86b60c6
33a944f
bac2429
bc794df
1384048
9335fa7
f33c6a1
ce00415
51411cd
d3533fa
ffb61b7
44c9fc3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
import { Observable } from "rxjs"; | ||
Check notice on line 1 in apps/browser/src/platform/browser/browser-api.ts CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)✅ Getting better: Primitive Obsession
|
||
|
||
import { DeviceType } from "@bitwarden/common/enums"; | ||
|
||
|
@@ -19,6 +19,15 @@ | |
return chrome.runtime.getManifest().manifest_version; | ||
} | ||
|
||
/** | ||
* Determines if the extension manifest version is the given version. | ||
* | ||
* @param expectedVersion - The expected manifest version to check against. | ||
*/ | ||
static isManifestVersion(expectedVersion: 2 | 3) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. non-blocking nit/question: I'm guessing no, but Is there a scenario where either of these two values in the comparison are not 2 or 3 (say, Another way I'm thinking about it; any reason to replace |
||
return BrowserApi.manifestVersion === expectedVersion; | ||
} | ||
|
||
/** | ||
* Gets the current window or the window with the given id. | ||
* | ||
|
@@ -98,12 +107,17 @@ | |
}); | ||
} | ||
|
||
/** | ||
* Gets the tab with the given id. | ||
* | ||
* @param tabId - The id of the tab to get. | ||
*/ | ||
static async getTab(tabId: number): Promise<chrome.tabs.Tab> | null { | ||
if (!tabId) { | ||
return null; | ||
} | ||
|
||
if (BrowserApi.manifestVersion === 3) { | ||
if (BrowserApi.isManifestVersion(3)) { | ||
return await chrome.tabs.get(tabId); | ||
} | ||
|
||
|
@@ -453,8 +467,11 @@ | |
}); | ||
} | ||
|
||
/** | ||
* Returns the supported BrowserAction API based on the manifest version. | ||
*/ | ||
static getBrowserAction() { | ||
return BrowserApi.manifestVersion === 3 ? chrome.action : chrome.browserAction; | ||
return BrowserApi.isManifestVersion(3) ? chrome.action : chrome.browserAction; | ||
} | ||
|
||
static getSidebarAction( | ||
|
@@ -488,7 +505,7 @@ | |
world: chrome.scripting.ExecutionWorld; | ||
}, | ||
): Promise<unknown> { | ||
if (BrowserApi.manifestVersion === 3) { | ||
if (BrowserApi.isManifestVersion(3)) { | ||
return chrome.scripting.executeScript({ | ||
target: { | ||
tabId: tabId, | ||
|
@@ -546,4 +563,32 @@ | |
chrome.privacy.services.autofillCreditCardEnabled.set({ value }); | ||
chrome.privacy.services.passwordSavingEnabled.set({ value }); | ||
} | ||
|
||
/** | ||
* Opens the offscreen document with the given reasons and justification. | ||
* | ||
* @param reasons - List of reasons for opening the offscreen document. | ||
* @see https://developer.chrome.com/docs/extensions/reference/api/offscreen#type-Reason | ||
* @param justification - Custom written justification for opening the offscreen document. | ||
*/ | ||
static async createOffscreenDocument(reasons: chrome.offscreen.Reason[], justification: string) { | ||
await chrome.offscreen.createDocument({ | ||
url: "offscreen-document/index.html", | ||
reasons, | ||
justification, | ||
}); | ||
} | ||
|
||
/** | ||
* Closes the offscreen document. | ||
* | ||
* @param callback - Optional callback to execute after the offscreen document is closed. | ||
*/ | ||
static closeOffscreenDocument(callback?: () => void) { | ||
chrome.offscreen.closeDocument(() => { | ||
if (callback) { | ||
callback(); | ||
} | ||
}); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
non-blocking naming nit (the nittiest) - don't bother if you don't need to make other changes, tho