[FIX] Spreadsheet: Make zoom work on Safari

The new zoom feature relies on the zoom css property, however there is a
long standing bug in webkit (https://bugs.webkit.org/show_bug.cgi?id=77998)
that impacts the computation of `Element.getBoundingClientRect`.

This revision adds a check on safari browser to correct the boundingRect
at specific points (pretty much where we handle mouseEvents).

Task: 5232092

Author: rrahir

packages/o-spreadsheet-engine/src/types/rendering.ts

@@ -3,6 +3,11 @@ import { GridIcon } from "../registries/icons_on_cell_registry";
 import { ImageSVG } from "./image";
 import { Alias, Align, BorderDescr, Color, DataBarFill, Pixel, Style, Zone } from "./misc";
 
+export interface DOMRectPosition {
+  top: Pixel;
+  left: Pixel;
+}
+
 /**
  * Coordinate in pixels
  */

src/components/helpers/dom_helpers.ts

@@ -1,5 +1,5 @@
 import { Ref } from "@odoo/o-spreadsheet-engine/types/misc";
-import { Rect } from "@odoo/o-spreadsheet-engine/types/rendering";
+import { DOMRectPosition, Rect } from "@odoo/o-spreadsheet-engine/types/rendering";
 
 const macRegex = /Mac/i;
 
@@ -14,12 +14,27 @@ export function isChildEvent(parent: HTMLElement | null | undefined, ev: Event):
   return !!ev.target && parent!.contains(ev.target as Node);
 }
 
-export function gridOverlayPosition() {
+export function gridOverlayPosition(zoom = 1): DOMRectPosition {
   const spreadsheetElement = document.querySelector(".o-grid-overlay");
-  if (spreadsheetElement) {
-    return spreadsheetElement.getBoundingClientRect();
+  const result = spreadsheetElement && zoomCorrectedElementPosition(spreadsheetElement, zoom);
+  if (!result) {
+    throw new Error("Can't find spreadsheet position");
   }
-  throw new Error("Can't find spreadsheet position");
+  return result;
+}
+
+export function zoomCorrectedElementPosition(
+  el: Element,
+  zoomLevel: number
+): DOMRectPosition | null {
+  const targetEl = el.classList.contains("o-zoomable") ? el : el.closest(".o-zoomable");
+  if (!targetEl) return null;
+  const rect = targetEl!.getBoundingClientRect();
+  const zoom = isBrowserSafari() ? zoomLevel : 1;
+  return {
+    top: rect.top * zoom,
+    left: rect.left * zoom,
+  };
 }
 
 export function getRefBoundingRect(ref: Ref<HTMLElement>): Rect {
@@ -218,6 +233,10 @@ export function isBrowserFirefox() {
   return /Firefox/i.test(navigator.userAgent);
 }
 
+export function isBrowserSafari() {
+  return /Safari/i.test(navigator.userAgent);
+}
+
 // Mobile detection
 function maxTouchPoints() {
   return navigator.maxTouchPoints || 1;

src/components/helpers/drag_and_drop_grid_hook.ts

@@ -45,7 +45,8 @@ export function useDragAndDropBeyondTheViewport(env: SpreadsheetChildEnv) {
     }
 
     const sheetId = getters.getActiveSheetId();
-    const position = gridOverlayPosition();
+    const zoomLevel = env.model.getters.getViewportZoomLevel();
+    const position = gridOverlayPosition(zoomLevel);
     const zoomedMouseEvent = withZoom(env, currentEv, position);
     const { x: offsetCorrectionX, y: offsetCorrectionY } = getters.getMainViewportCoordinates();
     const { top, left, bottom, right } = getters.getActiveMainViewport();
@@ -145,7 +146,8 @@ export function useDragAndDropBeyondTheViewport(env: SpreadsheetChildEnv) {
     startScrollDirection: DnDDirection = "all"
   ) => {
     cleanUp();
-    const position = gridOverlayPosition();
+    const zoomLevel = env.model.getters.getViewportZoomLevel();
+    const position = gridOverlayPosition(zoomLevel);
     scrollDirection = startScrollDirection;
     startingX = initialPointerCoordinates.clientX - position.left;
     startingY = initialPointerCoordinates.clientY - position.top;

src/components/helpers/zoom.ts

@@ -1,5 +1,6 @@
 import { SpreadsheetChildEnv } from "@odoo/o-spreadsheet-engine/types/spreadsheet_env";
-import { Pixel, Rect } from "../..";
+import { DOMRectPosition, Pixel, Rect } from "../..";
+import { zoomCorrectedElementPosition } from "./dom_helpers";
 
 export type ZoomedMouseEvent<T extends MouseEvent | PointerEvent> = {
   clientX: Pixel;
@@ -13,21 +14,21 @@ export type ZoomedMouseEvent<T extends MouseEvent | PointerEvent> = {
  * Return a POJO containing the original event as well as the client position and the client offset
  * where the event would target if the spreadsheet was not zoomed
  * @param ev unzoomed mouse event
- * @param originalTargetRect The original target bounding rect the resulting ZoomedMouseEvent offset must refer to
+ * @param originalTargetPosition The original target bounding rect the resulting ZoomedMouseEvent offset must refer to
  * @returns a ZoomedMouseEvent
  */
 export function withZoom<T extends MouseEvent>(
   env: SpreadsheetChildEnv,
   ev: T,
-  originalTargetRect?: DOMRect | null
+  originalTargetPosition?: DOMRectPosition | null
 ): ZoomedMouseEvent<T> {
   const zoomLevel = env.model.getters.getViewportZoomLevel();
-  if (originalTargetRect === undefined) {
-    originalTargetRect = getZoomTargetBoundingRect(ev);
+  if (originalTargetPosition === undefined) {
+    originalTargetPosition = getZoomTargetPosition(ev, zoomLevel);
   }
-  if (!originalTargetRect) return withNoZoom(ev);
-  const baseOffsetX = ev.clientX - originalTargetRect.left;
-  const baseOffsetY = ev.clientY - originalTargetRect.top;
+  if (!originalTargetPosition) return withNoZoom(ev);
+  const baseOffsetX = ev.clientX - originalTargetPosition.left;
+  const baseOffsetY = ev.clientY - originalTargetPosition.top;
   const offsetX = baseOffsetX / zoomLevel;
   const offsetY = baseOffsetY / zoomLevel;
   return {
@@ -64,12 +65,10 @@ export function getZoomedRect(zoom: number, rect: Rect): Rect {
 /**
  * Returns the bounding rect of the closest or self element who is targetable by a ZoomedMouseEvent
  */
-function getZoomTargetBoundingRect(ev: MouseEvent): DOMRect | null {
+function getZoomTargetPosition(ev: MouseEvent, zoom: number): DOMRectPosition | null {
   const target = ev.target;
   if (!target || !("classList" in target) || !(target instanceof Element)) {
     return null;
   }
-  const targetEl = target.classList.contains("o-zoomable") ? target : target.closest(".o-zoomable");
-  if (!targetEl) return null;
-  return targetEl.getBoundingClientRect();
+  return zoomCorrectedElementPosition(target, zoom);
 }

src/components/highlight/highlight/highlight.ts

@@ -117,9 +117,11 @@ export class Highlight extends Component<HighlightProps, SpreadsheetChildEnv> {
   onMoveHighlight(ev: PointerEvent) {
     this.highlightState.shiftingMode = "isMoving";
     const z = this.props.range.zone;
-    const zoomedMouseEvent = withZoom(this.env, ev);
 
-    const position = gridOverlayPosition();
+    const zoomLevel = this.env.model.getters.getViewportZoomLevel();
+    const position = gridOverlayPosition(zoomLevel);
+    const zoomedMouseEvent = withZoom(this.env, ev, position);
+
     const activeSheetId = this.env.model.getters.getActiveSheetId();
 
     const initCol = this.env.model.getters.getColIndex(zoomedMouseEvent.clientX - position.left);

src/components/spreadsheet/spreadsheet.ts

@@ -291,7 +291,8 @@ export class Spreadsheet extends Component<SpreadsheetProps, SpreadsheetChildEnv
         ?.getBoundingClientRect().height || 0;
 
     const gridWidth =
-      this.spreadsheetRef.el?.querySelector(".o-grid")?.getBoundingClientRect().width || 0;
+      (this.spreadsheetRef.el?.getBoundingClientRect().width || 0) -
+      (this.spreadsheetRef.el?.querySelector(".o-sidePanel")?.getBoundingClientRect().width || 0);
     const gridHeight =
       (this.spreadsheetRef.el?.getBoundingClientRect().height || 0) -
       (this.spreadsheetRef.el?.querySelector(".o-column-groups")?.getBoundingClientRect().height ||

tests/figures/figure_component.test.ts

@@ -2,6 +2,8 @@ import {
   DEFAULT_CELL_HEIGHT,
   DEFAULT_CELL_WIDTH,
   FIGURE_BORDER_WIDTH,
+  HEADER_HEIGHT,
+  HEADER_WIDTH,
   MENU_WIDTH,
   ZOOM_VALUES,
 } from "@odoo/o-spreadsheet-engine/constants";
@@ -155,7 +157,12 @@ beforeEach(() => {
 describe("figures", () => {
   beforeEach(async () => {
     notifyUser = jest.fn();
-    mockSpreadsheetRect = { top: 100, left: 200, height: 1000, width: 1000 };
+    mockSpreadsheetRect = {
+      top: 100,
+      left: 200,
+      height: 1000 + HEADER_HEIGHT,
+      width: 1000 + HEADER_WIDTH,
+    };
     mockFigureMenuItemRect = { top: 500, left: 500 };
     ({ model, parent, fixture, env } = await mountSpreadsheet(undefined, { notifyUser }));
     sheetId = model.getters.getActiveSheetId();

tests/spreadsheet/__snapshots__/spreadsheet_component.test.ts.snap

@@ -872,8 +872,8 @@ exports[`Simple Spreadsheet Component simple rendering snapshot 1`] = `
         
         <canvas
           height="985"
-          style="width:1033px;height:985px;zoom:1"
-          width="1033"
+          style="width:985px;height:985px;zoom:1"
+          width="985"
         />
         
         
@@ -1844,8 +1844,8 @@ exports[`components take the small screen into account 1`] = `
         
         <canvas
           height="985"
-          style="width:1033px;height:985px;zoom:1"
-          width="1033"
+          style="width:485px;height:985px;zoom:1"
+          width="485"
         />
         
         

tests/spreadsheet/spreadsheet_component.test.ts

@@ -67,6 +67,9 @@ beforeEach(() => {
     "o-spreadsheet": () => {
       return { x: 0, y: 0, width: spreadsheetWidth, height: 1000 };
     },
+    ".o-grid": () => {
+      return { x: 0, y: 0, width: spreadsheetWidth, height: 1000 };
+    },
   });
 });
 
@@ -493,6 +496,7 @@ test("*isSmall* is properly recomputed when changing window size", async () => {
 
 test("components take the small screen into account", async () => {
   const model = new Model();
+  spreadsheetWidth = 500;
   const { fixture } = await mountSpreadsheet({ model }, { isSmall: true });
   expect(fixture.querySelector(".o-spreadsheet")).toMatchSnapshot();
   expect(fixture.querySelector(".o-spreadsheet-mobile")).not.toBeNull();