1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-10 13:23:34 +00:00

[PM-27798] Prevent inline menu from opening on the page outside of the viewport (#17664)

* cleanup

* prevent inline menu from opening on the page outside of the viewport

* update inline menu viewport check to include checks on all sides of the viewport

* use VisualViewport when available

* update tests
This commit is contained in:
Jonathan Prusik
2025-12-02 11:31:35 -05:00
committed by GitHub
parent ebd5793568
commit f17890a26b
5 changed files with 306 additions and 27 deletions

View File

@@ -69,8 +69,8 @@ export type FieldRect = {
};
export type InlineMenuPosition = {
button?: InlineMenuElementPosition;
list?: InlineMenuElementPosition;
button?: InlineMenuElementPosition | null;
list?: InlineMenuElementPosition | null;
};
export type NewLoginCipherData = {

View File

@@ -1424,11 +1424,11 @@ export class OverlayBackground implements OverlayBackgroundInterface {
}
/**
* calculates the postion and width for multi-input totp field inline menu
* @param totpFieldArray - the totp fields used to evaluate the position of the menu
* calculates the position and width for multi-input TOTP field inline menu
* @param totpFieldArray - the TOTP fields used to evaluate the position of the menu
*/
private calculateTotpMultiInputMenuBounds(totpFieldArray: AutofillField[]) {
// Filter the fields based on the provided totpfields
// Filter the fields based on the provided TOTP fields
const filteredObjects = this.allFieldData.filter((obj) =>
totpFieldArray.some((o) => o.opid === obj.opid),
);
@@ -1451,8 +1451,8 @@ export class OverlayBackground implements OverlayBackgroundInterface {
}
/**
* calculates the postion for multi-input totp field inline button
* @param totpFieldArray - the totp fields used to evaluate the position of the menu
* calculates the position for multi-input TOTP field inline button
* @param totpFieldArray - the TOTP fields used to evaluate the position of the menu
*/
private calculateTotpMultiInputButtonBounds(totpFieldArray: AutofillField[]) {
const filteredObjects = this.allFieldData.filter((obj) =>

View File

@@ -1,7 +1,7 @@
import { mock } from "jest-mock-extended";
import { EVENTS } from "@bitwarden/common/autofill/constants";
import { ThemeType } from "@bitwarden/common/platform/enums";
import { ThemeTypes } from "@bitwarden/common/platform/enums";
import { AutofillOverlayPort } from "../../../enums/autofill-overlay.enum";
import { createPortSpyMock } from "../../../spec/autofill-mocks";
@@ -66,17 +66,38 @@ describe("AutofillInlineMenuIframeService", () => {
);
});
// TODO CG - This test is brittle and failing due to how we are calling the private method. This needs to be reworked
it.skip("creates an aria alert element if the ariaAlert param is passed", () => {
const ariaAlert = "aria alert";
it("creates an aria alert element if the ariaAlert param is passed to AutofillInlineMenuIframeService", () => {
jest.spyOn(autofillInlineMenuIframeService as any, "createAriaAlertElement");
autofillInlineMenuIframeService.initMenuIframe();
expect(autofillInlineMenuIframeService["createAriaAlertElement"]).toHaveBeenCalledWith(
ariaAlert,
expect(autofillInlineMenuIframeService["createAriaAlertElement"]).toHaveBeenCalled();
expect(autofillInlineMenuIframeService["ariaAlertElement"]).toBeDefined();
expect(autofillInlineMenuIframeService["ariaAlertElement"].getAttribute("role")).toBe(
"alert",
);
expect(autofillInlineMenuIframeService["ariaAlertElement"]).toMatchSnapshot();
expect(autofillInlineMenuIframeService["ariaAlertElement"].getAttribute("aria-live")).toBe(
"polite",
);
expect(autofillInlineMenuIframeService["ariaAlertElement"].getAttribute("aria-atomic")).toBe(
"true",
);
});
it("does not create an aria alert element if the ariaAlert param is not passed to AutofillInlineMenuIframeService", () => {
const shadowWithoutAlert = document.createElement("div").attachShadow({ mode: "open" });
const serviceWithoutAlert = new AutofillInlineMenuIframeService(
shadowWithoutAlert,
AutofillOverlayPort.Button,
{ height: "0px" },
"title",
);
jest.spyOn(serviceWithoutAlert as any, "createAriaAlertElement");
serviceWithoutAlert.initMenuIframe();
expect(serviceWithoutAlert["createAriaAlertElement"]).not.toHaveBeenCalled();
expect(serviceWithoutAlert["ariaAlertElement"]).toBeUndefined();
});
describe("on load of the iframe source", () => {
@@ -200,7 +221,7 @@ describe("AutofillInlineMenuIframeService", () => {
sendPortMessage(portSpy, { command: "updateAutofillInlineMenuPosition" });
expect(
autofillInlineMenuIframeService["iframe"].contentWindow.postMessage,
autofillInlineMenuIframeService["iframe"].contentWindow?.postMessage,
).not.toHaveBeenCalled();
});
@@ -216,7 +237,7 @@ describe("AutofillInlineMenuIframeService", () => {
expect(autofillInlineMenuIframeService["portKey"]).toBe(portKey);
expect(
autofillInlineMenuIframeService["iframe"].contentWindow.postMessage,
autofillInlineMenuIframeService["iframe"].contentWindow?.postMessage,
).toHaveBeenCalledWith(message, autofillInlineMenuIframeService["extensionOrigin"]);
});
});
@@ -234,14 +255,14 @@ describe("AutofillInlineMenuIframeService", () => {
it("passes the message on to the iframe element", () => {
const message = {
command: "initAutofillInlineMenuList",
theme: ThemeType.Light,
theme: ThemeTypes.Light,
};
sendPortMessage(portSpy, message);
expect(updateElementStylesSpy).not.toHaveBeenCalled();
expect(
autofillInlineMenuIframeService["iframe"].contentWindow.postMessage,
autofillInlineMenuIframeService["iframe"].contentWindow?.postMessage,
).toHaveBeenCalledWith(message, autofillInlineMenuIframeService["extensionOrigin"]);
});
@@ -249,18 +270,18 @@ describe("AutofillInlineMenuIframeService", () => {
window.matchMedia = jest.fn(() => mock<MediaQueryList>({ matches: false }));
const message = {
command: "initAutofillInlineMenuList",
theme: ThemeType.System,
theme: ThemeTypes.System,
};
sendPortMessage(portSpy, message);
expect(window.matchMedia).toHaveBeenCalledWith("(prefers-color-scheme: dark)");
expect(
autofillInlineMenuIframeService["iframe"].contentWindow.postMessage,
autofillInlineMenuIframeService["iframe"].contentWindow?.postMessage,
).toHaveBeenCalledWith(
{
command: "initAutofillInlineMenuList",
theme: ThemeType.Light,
theme: ThemeTypes.Light,
},
autofillInlineMenuIframeService["extensionOrigin"],
);
@@ -270,18 +291,18 @@ describe("AutofillInlineMenuIframeService", () => {
window.matchMedia = jest.fn(() => mock<MediaQueryList>({ matches: true }));
const message = {
command: "initAutofillInlineMenuList",
theme: ThemeType.System,
theme: ThemeTypes.System,
};
sendPortMessage(portSpy, message);
expect(window.matchMedia).toHaveBeenCalledWith("(prefers-color-scheme: dark)");
expect(
autofillInlineMenuIframeService["iframe"].contentWindow.postMessage,
autofillInlineMenuIframeService["iframe"].contentWindow?.postMessage,
).toHaveBeenCalledWith(
{
command: "initAutofillInlineMenuList",
theme: ThemeType.Dark,
theme: ThemeTypes.Dark,
},
autofillInlineMenuIframeService["extensionOrigin"],
);
@@ -290,7 +311,7 @@ describe("AutofillInlineMenuIframeService", () => {
it("updates the border to match the `dark` theme", () => {
const message = {
command: "initAutofillInlineMenuList",
theme: ThemeType.Dark,
theme: ThemeTypes.Dark,
};
sendPortMessage(portSpy, message);
@@ -364,6 +385,219 @@ describe("AutofillInlineMenuIframeService", () => {
autofillInlineMenuIframeService["handleFadeInInlineMenuIframe"],
).toHaveBeenCalled();
});
it("closes the inline menu when iframe is outside the viewport (bottom)", () => {
const viewportHeight = 800;
jest.spyOn(globalThis.document, "hasFocus").mockReturnValue(true);
jest
.spyOn(autofillInlineMenuIframeService["iframe"], "getBoundingClientRect")
.mockReturnValue({
top: 0,
left: 0,
right: 100,
bottom: viewportHeight + 1,
height: 98,
width: 262,
} as DOMRect);
Object.defineProperty(globalThis.window, "innerHeight", {
value: viewportHeight,
writable: true,
configurable: true,
});
Object.defineProperty(globalThis.window, "innerWidth", {
value: 1200,
writable: true,
configurable: true,
});
sendPortMessage(portSpy, {
command: "updateAutofillInlineMenuPosition",
styles: {},
});
expect(sendExtensionMessageSpy).toHaveBeenCalledWith("closeAutofillInlineMenu", {
forceCloseInlineMenu: true,
});
});
it("closes the inline menu when iframe is outside the viewport (right)", () => {
const viewportWidth = 1200;
jest.spyOn(globalThis.document, "hasFocus").mockReturnValue(true);
jest
.spyOn(autofillInlineMenuIframeService["iframe"], "getBoundingClientRect")
.mockReturnValue({
top: 0,
left: 0,
right: viewportWidth + 1,
bottom: 100,
height: 98,
width: 262,
} as DOMRect);
Object.defineProperty(globalThis.window, "innerHeight", {
value: 800,
writable: true,
configurable: true,
});
Object.defineProperty(globalThis.window, "innerWidth", {
value: viewportWidth,
writable: true,
configurable: true,
});
sendPortMessage(portSpy, {
command: "updateAutofillInlineMenuPosition",
styles: {},
});
expect(sendExtensionMessageSpy).toHaveBeenCalledWith("closeAutofillInlineMenu", {
forceCloseInlineMenu: true,
});
});
it("closes the inline menu when iframe is outside the viewport (left)", () => {
jest.spyOn(globalThis.document, "hasFocus").mockReturnValue(true);
jest
.spyOn(autofillInlineMenuIframeService["iframe"], "getBoundingClientRect")
.mockReturnValue({
top: 0,
left: -1,
right: 0,
bottom: 100,
height: 98,
width: 262,
} as DOMRect);
Object.defineProperty(globalThis.window, "innerHeight", {
value: 800,
writable: true,
configurable: true,
});
Object.defineProperty(globalThis.window, "innerWidth", {
value: 1200,
writable: true,
configurable: true,
});
sendPortMessage(portSpy, {
command: "updateAutofillInlineMenuPosition",
styles: {},
});
expect(sendExtensionMessageSpy).toHaveBeenCalledWith("closeAutofillInlineMenu", {
forceCloseInlineMenu: true,
});
});
it("closes the inline menu when iframe is outside the viewport (top)", () => {
jest.spyOn(globalThis.document, "hasFocus").mockReturnValue(true);
jest
.spyOn(autofillInlineMenuIframeService["iframe"], "getBoundingClientRect")
.mockReturnValue({
top: -1,
left: 0,
right: 100,
bottom: 0,
height: 98,
width: 262,
} as DOMRect);
Object.defineProperty(globalThis.window, "innerHeight", {
value: 800,
writable: true,
configurable: true,
});
Object.defineProperty(globalThis.window, "innerWidth", {
value: 1200,
writable: true,
configurable: true,
});
sendPortMessage(portSpy, {
command: "updateAutofillInlineMenuPosition",
styles: {},
});
expect(sendExtensionMessageSpy).toHaveBeenCalledWith("closeAutofillInlineMenu", {
forceCloseInlineMenu: true,
});
});
it("allows iframe (do not close) when it has no dimensions", () => {
jest.spyOn(globalThis.document, "hasFocus").mockReturnValue(true);
jest
.spyOn(autofillInlineMenuIframeService["iframe"], "getBoundingClientRect")
.mockReturnValue({
top: 0,
left: 0,
right: 0,
bottom: 0,
height: 0,
width: 0,
} as DOMRect);
Object.defineProperty(globalThis.window, "innerHeight", {
value: 800,
writable: true,
configurable: true,
});
Object.defineProperty(globalThis.window, "innerWidth", {
value: 1200,
writable: true,
configurable: true,
});
sendPortMessage(portSpy, {
command: "updateAutofillInlineMenuPosition",
styles: {},
});
expect(sendExtensionMessageSpy).not.toHaveBeenCalledWith("closeAutofillInlineMenu", {
forceCloseInlineMenu: true,
});
});
it("uses visualViewport when available", () => {
jest.spyOn(globalThis.document, "hasFocus").mockReturnValue(true);
jest
.spyOn(autofillInlineMenuIframeService["iframe"], "getBoundingClientRect")
.mockReturnValue({
top: 0,
left: 0,
right: 100,
bottom: 700,
height: 98,
width: 262,
} as DOMRect);
Object.defineProperty(globalThis.window, "visualViewport", {
value: {
height: 600,
width: 1200,
} as VisualViewport,
writable: true,
configurable: true,
});
Object.defineProperty(globalThis.window, "innerHeight", {
value: 800,
writable: true,
configurable: true,
});
Object.defineProperty(globalThis.window, "innerWidth", {
value: 1200,
writable: true,
configurable: true,
});
sendPortMessage(portSpy, {
command: "updateAutofillInlineMenuPosition",
styles: {},
});
expect(sendExtensionMessageSpy).toHaveBeenCalledWith("closeAutofillInlineMenu", {
forceCloseInlineMenu: true,
});
});
});
it("updates the visibility of the iframe", () => {
@@ -381,7 +615,7 @@ describe("AutofillInlineMenuIframeService", () => {
});
expect(
autofillInlineMenuIframeService["iframe"].contentWindow.postMessage,
autofillInlineMenuIframeService["iframe"].contentWindow?.postMessage,
).toHaveBeenCalledWith(
{
command: "updateAutofillInlineMenuColorScheme",

View File

@@ -282,6 +282,15 @@ export class AutofillInlineMenuIframeService implements AutofillInlineMenuIframe
const styles = this.fadeInTimeout ? Object.assign(position, { opacity: "0" }) : position;
this.updateElementStyles(this.iframe, styles);
const elementHeightCompletelyInViewport = this.isElementCompletelyWithinViewport(
this.iframe.getBoundingClientRect(),
);
if (!elementHeightCompletelyInViewport) {
this.forceCloseInlineMenu();
return;
}
if (this.fadeInTimeout) {
this.handleFadeInInlineMenuIframe();
}
@@ -289,6 +298,42 @@ export class AutofillInlineMenuIframeService implements AutofillInlineMenuIframe
this.announceAriaAlert(this.ariaAlert, 2000);
}
/**
* Check if element is completely within the browser viewport.
*/
private isElementCompletelyWithinViewport(elementPosition: DOMRect) {
// An element that lacks size should be considered within the viewport
if (!elementPosition.height || !elementPosition.width) {
return true;
}
const [viewportHeight, viewportWidth] = this.getViewportSize();
const rightSideIsWithinViewport = (elementPosition.right || 0) <= viewportWidth;
const leftSideIsWithinViewport = (elementPosition.left || 0) >= 0;
const topSideIsWithinViewport = (elementPosition.top || 0) >= 0;
const bottomSideIsWithinViewport = (elementPosition.bottom || 0) <= viewportHeight;
return (
rightSideIsWithinViewport &&
leftSideIsWithinViewport &&
topSideIsWithinViewport &&
bottomSideIsWithinViewport
);
}
/** Use Visual Viewport API if available (better for mobile/zoom) */
private getViewportSize(): [
VisualViewport["height"] | Window["innerHeight"],
VisualViewport["width"] | Window["innerWidth"],
] {
if ("visualViewport" in globalThis.window && globalThis.window.visualViewport) {
return [globalThis.window.visualViewport.height, globalThis.window.visualViewport.width];
}
return [globalThis.window.innerHeight, globalThis.window.innerWidth];
}
/**
* Gets the page color scheme meta tag and sends a message to the iframe
* to update its color scheme. Will default to "normal" if the meta tag

View File

@@ -1400,7 +1400,7 @@ export class CollectAutofillContentService implements CollectAutofillContentServ
this.intersectionObserver = new IntersectionObserver(this.handleFormElementIntersection, {
root: null,
rootMargin: "0px",
threshold: 1.0,
threshold: 0.9999, // Safari doesn't seem to function properly with a threshold of 1,
});
}