[FIX] SheetView: dirtify sheet viewport at UPDATE_CELL

the issue was found through an autofill bug. To reproduce:

- Add a data validation checkbox to A1:A2
- Add a format (like `%`) to A1:A2
- autofill A1 into A2

-> crash

What happens?

For starters, the plugin `SheetViewPlugin` is always listening to
selection events and will update the viewports 'scroll' value
accordingly.

During the autofill handling of the command, a bunch of `UPDATE_CELL`
commands are dispatched, which invalidates the whole state of
`HeaderPositionsUIPlugin`.
After that dispatch, the plugin  also updates the selection, which
triggers `SheetViewPlugin` to update its viewports but this manipulation
relies on the state of `HeaderPositionsUIPlugin`, which was just invalidated!

It was already known that during a command dispatch, the general state
of plugins might be unstable and `SheetViewPlugin` accounted for it by
*tagging* sheets that might have an unstable state and it would skip the
viewport `scroll` update after selection events.
However, we limited the *tagging* for specific payloads of `UPDATE_CELL`
and the condition set on the style was incorrect. Indeed, a style object
without the `fontSize` does not mean that we do not update the fontsize
but rather that we reset it to its default value which has a direct
impact on the header size and position (hence the invalidation of state
in `HeaderPositionsUIPlugin`.

All-in-all, The presence of the `style` key in the `UPDATE_CELL` command
payload is enough to tag a sheet as invalid.

A side note:
While changing the content or format of a cell can have cross-sheet
impact due to the evaluation process (a cell will take the format of its
reference without a format on its own), it is not the case for the
style. As such, we could make separate cases for commands that update
the content/format, which might theoretically impact every sheet, and
commands that only impact the style, which only impacts the targetted
sheet. Such improvement can target the master dev branch.

closes #8189

Task: 5953775
X-original-commit: a962b50
Signed-off-by: Pierre Rousseau (pro) <pro@odoo.com>
Signed-off-by: Rémi Rahir (rar) <rar@odoo.com>

Author: rrahir

packages/o-spreadsheet-engine/src/plugins/ui_stateful/header_positions.ts

@@ -21,6 +21,8 @@ export class HeaderPositionsUIPlugin extends UIPlugin {
           this.headerPositions[sheetId] = this.computeHeaderPositionsOfSheet(sheetId);
         }
         break;
+      // Either the content, format or style can impact the header sizes of a sheet
+      // As such, every command can have a potential effect on the viewport
       case "UPDATE_CELL":
         this.headerPositions = {};
         this.isDirty = true;

packages/o-spreadsheet-engine/src/plugins/ui_stateful/sheetview.ts

@@ -258,12 +258,12 @@ export class SheetViewPlugin extends UIPlugin {
       case "FOLD_ALL_HEADER_GROUPS":
         this.sheetsWithDirtyViewports.add(cmd.sheetId);
         break;
+      // Either the content, format or style can impact the header sizes of a sheet
+      // As such, every command can have a potential effect on the viewport
       case "UPDATE_CELL":
-        // update cell content or format can change hidden rows because of data filters
-        if ("content" in cmd || "format" in cmd || cmd.style?.fontSize !== undefined) {
-          for (const sheetId of this.getters.getSheetIds()) {
-            this.sheetsWithDirtyViewports.add(sheetId);
-          }
+      case "SET_FORMATTING":
+        for (const sheetId of this.getters.getSheetIds()) {
+          this.sheetsWithDirtyViewports.add(sheetId);
         }
         break;
       case "DELETE_SHEET":

tests/autofill/autofill_plugin.test.ts

@@ -17,6 +17,7 @@ import {
   selectCell,
   setBorders,
   setCellContent,
+  setFormat,
   setSelection,
   updateCell,
 } from "../test_helpers/commands_helpers";
@@ -900,4 +901,12 @@ describe("Autofill", () => {
     setCellContent(model, "A1", "[label](url)");
     expect(autofillTooltip("A1", "A2")).toBe("label");
   });
+
+  test("Autofill that triggers no change does not crash", () => {
+    addDataValidation(model, "A1:A2", "1", { type: "isBoolean", values: [] });
+    setCellContent(model, "A1", "FALSE");
+    setCellContent(model, "A2", "FALSE");
+    setFormat(model, "A1:A2", "0.00%");
+    autofill(model, "A1", "A2"); // A1 and A2 are exactly the same
+  });
 });

tests/sheet/sheetview_plugin.test.ts

@@ -888,7 +888,18 @@ describe("Viewport of Simple sheet", () => {
       right: 10,
       top: 0,
     });
-    setFormatting(model, "A1:A20", { fontSize: 8 });
+    setFormatting(model, "A1:A20", { fontSize: 24 });
+    expect(model.getters.getActiveMainViewport()).toEqual({
+      bottom: 30,
+      left: 0,
+      right: 10,
+      top: 0,
+    });
+
+    const sheetId = model.getters.getActiveSheetId();
+    for (let i = 0; i < 20; ++i) {
+      model.dispatch("UPDATE_CELL", { sheetId, col: 0, row: i, style: {} });
+    }
     expect(model.getters.getActiveMainViewport()).toEqual({
       bottom: 43,
       left: 0,