1
0
mirror of https://github.com/bitwarden/browser.git synced 2025-01-24 21:41:33 +01:00

[PM-5189] Fixing issues found within code review behind how we position elements

This commit is contained in:
Cesar Gonzalez 2024-06-24 16:56:12 -05:00
parent fe7bd3c2a3
commit 1e28efecfc
No known key found for this signature in database
GPG Key ID: 3381A5457F8CCECF
3 changed files with 72 additions and 29 deletions

View File

@ -286,12 +286,7 @@ export class OverlayBackground implements OverlayBackgroundInterface {
}; };
if (pageDetails.frameId !== 0 && pageDetails.details.fields.length) { if (pageDetails.frameId !== 0 && pageDetails.details.fields.length) {
void this.buildSubFrameOffsets( void this.buildSubFrameOffsets(pageDetails.tab, pageDetails.frameId, pageDetails.details.url);
pageDetails.tab,
pageDetails.frameId,
pageDetails.details.url,
sender,
);
void BrowserApi.tabSendMessage(pageDetails.tab, { void BrowserApi.tabSendMessage(pageDetails.tab, {
command: "setupRebuildSubFrameOffsetsListeners", command: "setupRebuildSubFrameOffsetsListeners",
}); });
@ -341,13 +336,13 @@ export class OverlayBackground implements OverlayBackgroundInterface {
* @param tab - The tab that the sub frame is associated with * @param tab - The tab that the sub frame is associated with
* @param frameId - The frame ID of the sub frame * @param frameId - The frame ID of the sub frame
* @param url - The URL of the sub frame * @param url - The URL of the sub frame
* @param sender - The sender of the message * @param forceRebuild - Identifies whether the sub frame offsets should be rebuilt
*/ */
private async buildSubFrameOffsets( private async buildSubFrameOffsets(
tab: chrome.tabs.Tab, tab: chrome.tabs.Tab,
frameId: number, frameId: number,
url: string, url: string,
sender: chrome.runtime.MessageSender, forceRebuild: boolean = false,
) { ) {
let subFrameDepth = 0; let subFrameDepth = 0;
const tabId = tab.id; const tabId = tab.id;
@ -357,7 +352,7 @@ export class OverlayBackground implements OverlayBackgroundInterface {
subFrameOffsetsForTab = this.subFrameOffsetsForTab[tabId]; subFrameOffsetsForTab = this.subFrameOffsetsForTab[tabId];
} }
if (subFrameOffsetsForTab.get(frameId)) { if (!forceRebuild && subFrameOffsetsForTab.get(frameId)) {
return; return;
} }
@ -438,8 +433,7 @@ export class OverlayBackground implements OverlayBackgroundInterface {
if (subFrameOffsetsForTab) { if (subFrameOffsetsForTab) {
const tabFrameIds = Array.from(subFrameOffsetsForTab.keys()); const tabFrameIds = Array.from(subFrameOffsetsForTab.keys());
for (const frameId of tabFrameIds) { for (const frameId of tabFrameIds) {
subFrameOffsetsForTab.delete(frameId); await this.buildSubFrameOffsets(sender.tab, frameId, sender.url, true);
await this.buildSubFrameOffsets(sender.tab, frameId, sender.url, sender);
} }
} }
} }
@ -664,6 +658,11 @@ export class OverlayBackground implements OverlayBackgroundInterface {
let subFrameOffsets: SubFrameOffsetData; let subFrameOffsets: SubFrameOffsetData;
if (subFrameOffsetsForTab) { if (subFrameOffsetsForTab) {
subFrameOffsets = subFrameOffsetsForTab.get(this.focusedFieldData.frameId); subFrameOffsets = subFrameOffsetsForTab.get(this.focusedFieldData.frameId);
if (subFrameOffsets === null) {
this.cancelUpdateInlineMenuPositionSubject.next();
this.startUpdateInlineMenuPositionSubject.next(sender);
return;
}
} }
if (overlayElement === AutofillOverlayElement.Button) { if (overlayElement === AutofillOverlayElement.Button) {

View File

@ -154,18 +154,6 @@ describe("TabsBackground", () => {
}); });
}); });
it("removes the cached page details from the overlay background if the tab status is `loading`", () => {
triggerTabOnUpdatedEvent(focusedWindowId, { status: "loading" }, tab);
expect(overlayBackground.removePageDetails).toHaveBeenCalledWith(focusedWindowId);
});
it("removes the cached page details from the overlay background if the tab status is `unloaded`", () => {
triggerTabOnUpdatedEvent(focusedWindowId, { status: "unloaded" }, tab);
expect(overlayBackground.removePageDetails).toHaveBeenCalledWith(focusedWindowId);
});
it("skips updating the current tab data the focusedWindowId is set to a value less than zero", async () => { it("skips updating the current tab data the focusedWindowId is set to a value less than zero", async () => {
tab.windowId = -1; tab.windowId = -1;
triggerTabOnUpdatedEvent(focusedWindowId, { status: "loading" }, tab); triggerTabOnUpdatedEvent(focusedWindowId, { status: "loading" }, tab);

View File

@ -844,14 +844,26 @@ export class AutofillOverlayContentService implements AutofillOverlayContentServ
message: AutofillExtensionMessage, message: AutofillExtensionMessage,
): Promise<SubFrameOffsetData | null> { ): Promise<SubFrameOffsetData | null> {
const { subFrameUrl } = message; const { subFrameUrl } = message;
const subFrameUrlWithoutTrailingSlash = subFrameUrl?.replace(/\/$/, "");
const subFrameUrlVariations = this.getSubFrameUrlVariations(subFrameUrl);
if (!subFrameUrlVariations) {
return null;
}
let iframeElement: HTMLIFrameElement | null = null; let iframeElement: HTMLIFrameElement | null = null;
const iframeElements = globalThis.document.querySelectorAll( const iframeElements = globalThis.document.getElementsByTagName("iframe");
`iframe[src="${subFrameUrl}"], iframe[src="${subFrameUrlWithoutTrailingSlash}"]`,
) as NodeListOf<HTMLIFrameElement>; for (let iframeIndex = 0; iframeIndex < iframeElements.length; iframeIndex++) {
if (iframeElements.length === 1) { const iframe = iframeElements[iframeIndex];
iframeElement = iframeElements[0]; if (!subFrameUrlVariations.has(iframe.src)) {
continue;
}
if (iframeElement) {
return null;
}
iframeElement = iframe;
} }
if (!iframeElement) { if (!iframeElement) {
@ -861,6 +873,50 @@ export class AutofillOverlayContentService implements AutofillOverlayContentServ
return this.calculateSubFrameOffsets(iframeElement, subFrameUrl); return this.calculateSubFrameOffsets(iframeElement, subFrameUrl);
} }
private getSubFrameUrlVariations(subFrameUrl: string) {
try {
const url = new URL(subFrameUrl, globalThis.location.href);
const pathAndHash = url.pathname + url.hash;
const pathAndSearch = url.pathname + url.search;
const pathSearchAndHash = pathAndSearch + url.hash;
const pathNameWithoutTrailingSlash = url.pathname.replace(/\/$/, "");
const pathWithoutTrailingSlashAndHash = pathNameWithoutTrailingSlash + url.hash;
const pathWithoutTrailingSlashAndSearch = pathNameWithoutTrailingSlash + url.search;
const pathWithoutTrailingSlashSearchAndHash = pathWithoutTrailingSlashAndSearch + url.hash;
return new Set([
url.href,
url.href.replace(/\/$/, ""),
url.pathname,
pathAndHash,
pathAndSearch,
pathSearchAndHash,
pathNameWithoutTrailingSlash,
pathWithoutTrailingSlashAndHash,
pathWithoutTrailingSlashAndSearch,
pathWithoutTrailingSlashSearchAndHash,
url.hostname + url.pathname,
url.hostname + pathAndHash,
url.hostname + pathAndSearch,
url.hostname + pathSearchAndHash,
url.hostname + pathNameWithoutTrailingSlash,
url.hostname + pathWithoutTrailingSlashAndHash,
url.hostname + pathWithoutTrailingSlashAndSearch,
url.hostname + pathWithoutTrailingSlashSearchAndHash,
url.origin + url.pathname,
url.origin + pathAndHash,
url.origin + pathAndSearch,
url.origin + pathSearchAndHash,
url.origin + pathNameWithoutTrailingSlash,
url.origin + pathWithoutTrailingSlashAndHash,
url.origin + pathWithoutTrailingSlashAndSearch,
url.origin + pathWithoutTrailingSlashSearchAndHash,
]);
} catch (_error) {
return null;
}
}
/** /**
* Posts a message to the parent frame to calculate the sub frame offset of the current frame. * Posts a message to the parent frame to calculate the sub frame offset of the current frame.
* *