diff --git a/package-lock.json b/package-lock.json index d0122e4f4a..8d96233512 100644 --- a/package-lock.json +++ b/package-lock.json @@ -32,7 +32,7 @@ "abort-utils": "^1.2.0", "ace-builds": "^1.33.1", "autocompleter": "^9.2.1", - "axios": "^0.27.2", + "axios": "^0.28.1", "batched-function": "^2.0.1", "bootstrap": "^4.6.0", "bootstrap-icons": "^1.11.3", @@ -10247,11 +10247,13 @@ } }, "node_modules/axios": { - "version": "0.27.2", - "integrity": "sha512-t+yRIyySRTp/wua5xEr+z1q60QmLq8ABsS5O9Me1AsE5dfKqgnCFzwiCZZ/cGNd1lq4/7akDWMxdhVlucjmnOQ==", + "version": "0.28.1", + "resolved": "https://registry.npmjs.org/axios/-/axios-0.28.1.tgz", + "integrity": "sha512-iUcGA5a7p0mVb4Gm/sy+FSECNkPFT4y7wt6OM/CDpO/OnNCvSs3PoMG8ibrC9jRoGYU0gUK5pXVC4NPXq6lHRQ==", "dependencies": { - "follow-redirects": "^1.14.9", - "form-data": "^4.0.0" + "follow-redirects": "^1.15.0", + "form-data": "^4.0.0", + "proxy-from-env": "^1.1.0" } }, "node_modules/axios-mock-adapter": { @@ -13643,9 +13645,9 @@ "dev": true }, "node_modules/ejs": { - "version": "3.1.9", - "resolved": "https://registry.npmjs.org/ejs/-/ejs-3.1.9.tgz", - "integrity": "sha512-rC+QVNMJWv+MtPgkt0y+0rVEIdbtxVADApW9JXrUVlzHetgcyczP/E7DJmWJ4fJCZF2cPcBk0laWO9ZHMG3DmQ==", + "version": "3.1.10", + "resolved": "https://registry.npmjs.org/ejs/-/ejs-3.1.10.tgz", + "integrity": "sha512-UeJmFfOrAQS8OJWPZ4qtgHyWExa088/MtK5UEyoJGFH67cDEXkZSviOiKRCZ4Xij0zxI3JECgYs3oKx+AizQBA==", "dev": true, "dependencies": { "jake": "^10.8.5" @@ -24492,8 +24494,7 @@ "node_modules/proxy-from-env": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/proxy-from-env/-/proxy-from-env-1.1.0.tgz", - "integrity": "sha512-D+zkORCbA9f1tdWRK0RaCR3GPv50cMxcrz4X8k5LTSUD1Dkw47mKJEZQNunItRTkWwgtaUSo1RVFRIG9ZXiFYg==", - "dev": true + "integrity": "sha512-D+zkORCbA9f1tdWRK0RaCR3GPv50cMxcrz4X8k5LTSUD1Dkw47mKJEZQNunItRTkWwgtaUSo1RVFRIG9ZXiFYg==" }, "node_modules/prr": { "version": "1.0.1", diff --git a/package.json b/package.json index f06375c0fe..9220c99196 100644 --- a/package.json +++ b/package.json @@ -58,7 +58,7 @@ "abort-utils": "^1.2.0", "ace-builds": "^1.33.1", "autocompleter": "^9.2.1", - "axios": "^0.27.2", + "axios": "^0.28.1", "batched-function": "^2.0.1", "bootstrap": "^4.6.0", "bootstrap-icons": "^1.11.3", diff --git a/src/bricks/readers/ElementReader.test.ts b/src/bricks/readers/ElementReader.test.ts index 95334c3e37..a6f90e5b1c 100644 --- a/src/bricks/readers/ElementReader.test.ts +++ b/src/bricks/readers/ElementReader.test.ts @@ -17,10 +17,19 @@ import { ElementReader } from "@/bricks/readers/ElementReader"; import { validateUUID } from "@/types/helpers"; +import { rectFactory } from "@/testUtils/factories/domFactories"; const reader = new ElementReader(); describe("ElementReader", () => { + beforeEach(() => { + // `jsdom` does not implement full layout engine + // https://github.com/jsdom/jsdom#unimplemented-parts-of-the-web-platform + (Element.prototype.getBoundingClientRect as any) = jest.fn(() => + rectFactory(), + ); + }); + test("it produces valid element reference", async () => { const div = document.createElement("div"); const { ref } = await reader.read(div); @@ -44,4 +53,29 @@ describe("ElementReader", () => { const { isVisible } = await reader.read(div); expect(isVisible).toBe(true); }); + + test("isInViewport: true for element in document", async () => { + const div = document.createElement("div"); + div.innerHTML = "

Some text

"; + document.body.append(div); + + const { isInViewport } = await reader.read(div); + expect(isInViewport).toBe(true); + }); + + test("isInViewport: false for element partially outside of document", async () => { + (Element.prototype.getBoundingClientRect as any) = jest.fn(() => + rectFactory({ + width: window.innerWidth + 1, + right: window.innerWidth + 1, + }), + ); + + const div = document.createElement("div"); + div.innerHTML = "

Some text

"; + document.body.append(div); + + const { isInViewport } = await reader.read(div); + expect(isInViewport).toBe(false); + }); }); diff --git a/src/bricks/readers/ElementReader.ts b/src/bricks/readers/ElementReader.ts index e0b0314fe4..9ce9deadac 100644 --- a/src/bricks/readers/ElementReader.ts +++ b/src/bricks/readers/ElementReader.ts @@ -20,7 +20,7 @@ import { getReferenceForElement } from "@/contentScript/elementReference"; import { ReaderABC } from "@/types/bricks/readerTypes"; import { type SelectorRoot } from "@/types/runtimeTypes"; import { type Schema } from "@/types/schemaTypes"; -import { isVisible } from "@/utils/domUtils"; +import { isInViewport, isVisible } from "@/utils/domUtils"; /** * Read attributes, text, etc. from an HTML element. @@ -49,6 +49,7 @@ export class ElementReader extends ReaderABC { return { ref: getReferenceForElement(element), isVisible: isVisible(element), + isInViewport: isInViewport(element), tagName: element.tagName, attrs: Object.fromEntries( Object.values(element.attributes).map((x) => [x.name, x.value]), @@ -89,8 +90,20 @@ export class ElementReader extends ReaderABC { type: "boolean", description: "True if the element is visible", }, + isInViewport: { + type: "boolean", + description: "True if element is completely in the viewport", + }, }, - required: ["tagName", "attrs", "data", "text", "ref", "isVisible"], + required: [ + "tagName", + "attrs", + "data", + "text", + "ref", + "isVisible", + "isInViewport", + ], additionalProperties: false, }; diff --git a/src/components/errors/NetworkErrorDetail.tsx b/src/components/errors/NetworkErrorDetail.tsx index 064a2bc554..a3c61f10af 100644 --- a/src/components/errors/NetworkErrorDetail.tsx +++ b/src/components/errors/NetworkErrorDetail.tsx @@ -55,7 +55,7 @@ const NetworkErrorDetail: React.FunctionComponent<{ ); const cleanConfig = useMemo(() => { - const { data, ...rest } = error.config; + const { data, ...rest } = error.config ?? {}; return { ...rest, data: tryParse(data), diff --git a/src/contrib/google/sheets/core/handleGoogleRequestRejection.ts b/src/contrib/google/sheets/core/handleGoogleRequestRejection.ts index 53911c793c..690dad7b89 100644 --- a/src/contrib/google/sheets/core/handleGoogleRequestRejection.ts +++ b/src/contrib/google/sheets/core/handleGoogleRequestRejection.ts @@ -16,15 +16,11 @@ */ import { type SanitizedIntegrationConfig } from "@/integrations/integrationTypes"; -import { - getErrorMessage, - getRootCause, - selectError, -} from "@/errors/errorHelpers"; +import { getErrorMessage, selectError } from "@/errors/errorHelpers"; import { isObject } from "@/utils/objectUtils"; -import { isAxiosError } from "@/errors/networkErrorHelpers"; import { deleteCachedAuthData } from "@/background/messenger/strict/api"; import { type Nullishable } from "@/utils/nullishUtils"; +import { selectAxiosError } from "@/data/service/requestErrorUtils"; class PermissionsError extends Error { override name = "PermissionsError"; @@ -41,11 +37,9 @@ export async function handleGoogleRequestRejection( error: unknown, googleAccount: Nullishable, ): Promise { - // Request errors from proxyRequest are wrapped in ContextError which includes metadata about the integration - // configuration. Therefore, get root cause for determining if this is an Axios error - const rootCause = getRootCause(error); - - console.debug("Error making Google request", { error }); + console.debug("Error making Google request", { + error, + }); if (!isObject(error)) { // Shouldn't happen in practice, but be defensive @@ -54,12 +48,16 @@ export async function handleGoogleRequestRejection( }); } - if (!isAxiosError(rootCause) || rootCause.response == null) { + // Request errors from proxyRequest are wrapped in ContextError which includes metadata about the integration + // configuration. Therefore, get axios error if it exists. + const axiosError = selectAxiosError(error); + + if (axiosError?.response == null) { // It should always be an error-like object at this point, but be defensive. return selectError(error); } - const { status } = rootCause.response; + const { status } = axiosError.response; if ([403, 404].includes(status)) { const message = diff --git a/src/data/service/errorService.ts b/src/data/service/errorService.ts index 5b43e09930..7e622fe6e6 100644 --- a/src/data/service/errorService.ts +++ b/src/data/service/errorService.ts @@ -91,7 +91,7 @@ export async function selectExtraContext( const axiosError = selectAxiosError(error); if ( - axiosError && + axiosError?.config && ((await flagOn("enterprise-telemetry")) || (await isAppRequestError(axiosError))) ) { diff --git a/src/utils/domUtils.ts b/src/utils/domUtils.ts index 0649513890..14ecd07564 100644 --- a/src/utils/domUtils.ts +++ b/src/utils/domUtils.ts @@ -132,11 +132,40 @@ export async function waitForBody(): Promise { /** * Return true if the element is visible (i.e. not in `display: none`), even if outside the viewport + * See https://developer.mozilla.org/en-US/docs/Web/API/Element/checkVisibility + * + * To determine if an element is fully contained within the viewport, use `isInViewport`. + * + * @see isInViewport */ export function isVisible(element: HTMLElement): boolean { return element.checkVisibility(); } +/** + * Returns true if the element is completely in the viewport. The method does not consider the element's + * opacity, z-index, or other CSS properties that may affect visibility. + * + * Example scenarios where the method may return counterintuitive results: + * - Elements hidden by z-index + * - If they are hidden by overflow-scroll in element's container + * - Things hidden by relative/absolute positioning + * - Elements within an iframe that is not entirely visible in the top frame + * + * @see isVisible + */ +export function isInViewport(element: HTMLElement): boolean { + // https://stackoverflow.com/a/125106 + const rect = element.getBoundingClientRect(); + return ( + rect.top >= 0 && + rect.left >= 0 && + rect.bottom <= + (window.innerHeight || document.documentElement.clientHeight) && + rect.right <= (window.innerWidth || document.documentElement.clientWidth) + ); +} + /** * Returns a callback that runs only when the document is visible. * - If the document is visible, runs immediately diff --git a/src/utils/urlUtils.ts b/src/utils/urlUtils.ts index 784c2a0443..c714f03357 100644 --- a/src/utils/urlUtils.ts +++ b/src/utils/urlUtils.ts @@ -46,7 +46,7 @@ export function selectAbsoluteUrl({ }: { url?: string; baseURL?: string; -}): string { +} = {}): string { assertNotNullish(url, "selectAbsoluteUrl: The URL was not provided"); if (canParseUrl(url)) { return url;