diff --git a/.github/scripts/package-lock.json b/.github/scripts/package-lock.json index 730f9ee9c8..2bb6b553c8 100644 --- a/.github/scripts/package-lock.json +++ b/.github/scripts/package-lock.json @@ -10,9 +10,9 @@ } }, "node_modules/@anthropic-ai/claude-agent-sdk": { - "version": "0.2.104", - "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk/-/claude-agent-sdk-0.2.104.tgz", - "integrity": "sha512-lVm+nS79r6WWlDnv5AgRzTtAlbP8O6M6kkWmDZAWE3nt9agmngxls9frJFvH55uzws2+6l0yyup/JYspfijkzw==", + "version": "0.2.114", + "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk/-/claude-agent-sdk-0.2.114.tgz", + "integrity": "sha512-plJ+j17jew9tDMHir/90hXrwoB8cZ9GrIyG19zIJcFyQ8pVhRXjZRJCtF2ElfPoiwkxMmNu1Klqyui4xP4shPg==", "license": "SEE LICENSE IN README.md", "dependencies": { "@anthropic-ai/sdk": "^0.81.0", @@ -22,378 +22,189 @@ "node": ">=18.0.0" }, "optionalDependencies": { - "@img/sharp-darwin-arm64": "^0.34.2", - "@img/sharp-darwin-x64": "^0.34.2", - "@img/sharp-linux-arm": "^0.34.2", - "@img/sharp-linux-arm64": "^0.34.2", - "@img/sharp-linux-x64": "^0.34.2", - "@img/sharp-linuxmusl-arm64": "^0.34.2", - "@img/sharp-linuxmusl-x64": "^0.34.2", - "@img/sharp-win32-arm64": "^0.34.2", - "@img/sharp-win32-x64": "^0.34.2" + "@anthropic-ai/claude-agent-sdk-darwin-arm64": "0.2.114", + "@anthropic-ai/claude-agent-sdk-darwin-x64": "0.2.114", + "@anthropic-ai/claude-agent-sdk-linux-arm64": "0.2.114", + "@anthropic-ai/claude-agent-sdk-linux-arm64-musl": "0.2.114", + "@anthropic-ai/claude-agent-sdk-linux-x64": "0.2.114", + "@anthropic-ai/claude-agent-sdk-linux-x64-musl": "0.2.114", + "@anthropic-ai/claude-agent-sdk-win32-arm64": "0.2.114", + "@anthropic-ai/claude-agent-sdk-win32-x64": "0.2.114" }, "peerDependencies": { "zod": "^4.0.0" } }, - "node_modules/@anthropic-ai/claude-agent-sdk/node_modules/@anthropic-ai/sdk": { - "version": "0.81.0", - "resolved": "https://registry.npmjs.org/@anthropic-ai/sdk/-/sdk-0.81.0.tgz", - "integrity": "sha512-D4K5PvEV6wPiRtVlVsJHIUhHAmOZ6IT/I9rKlTf84gR7GyyAurPJK7z9BOf/AZqC5d1DhYQGJNKRmV+q8dGhgw==", - "license": "MIT", - "dependencies": { - "json-schema-to-ts": "^3.1.1" - }, - "bin": { - "anthropic-ai-sdk": "bin/cli" - }, - "peerDependencies": { - "zod": "^3.25.0 || ^4.0.0" - }, - "peerDependenciesMeta": { - "zod": { - "optional": true - } - } - }, - "node_modules/@anthropic-ai/sdk": { - "version": "0.39.0", - "resolved": "https://registry.npmjs.org/@anthropic-ai/sdk/-/sdk-0.39.0.tgz", - "integrity": "sha512-eMyDIPRZbt1CCLErRCi3exlAvNkBtRe+kW5vvJyef93PmNr/clstYgHhtvmkxN82nlKgzyGPCyGxrm0JQ1ZIdg==", - "license": "MIT", - "dependencies": { - "@types/node": "^18.11.18", - "@types/node-fetch": "^2.6.4", - "abort-controller": "^3.0.0", - "agentkeepalive": "^4.2.1", - "form-data-encoder": "1.7.2", - "formdata-node": "^4.3.2", - "node-fetch": "^2.6.7" - } - }, - "node_modules/@babel/runtime": { - "version": "7.29.2", - "resolved": "https://registry.npmjs.org/@babel/runtime/-/runtime-7.29.2.tgz", - "integrity": "sha512-JiDShH45zKHWyGe4ZNVRrCjBz8Nh9TMmZG1kh4QTK8hCBTWBi8Da+i7s1fJw7/lYpM4ccepSNfqzZ/QvABBi5g==", - "license": "MIT", - "engines": { - "node": ">=6.9.0" - } - }, - "node_modules/@hono/node-server": { - "version": "1.19.11", - "resolved": "https://registry.npmjs.org/@hono/node-server/-/node-server-1.19.11.tgz", - "integrity": "sha512-dr8/3zEaB+p0D2n/IUrlPF1HZm586qgJNXK1a9fhg/PzdtkK7Ksd5l312tJX2yBuALqDYBlG20QEbayqPyxn+g==", - "license": "MIT", - "engines": { - "node": ">=18.14.1" - }, - "peerDependencies": { - "hono": "^4" - } - }, - "node_modules/@img/sharp-darwin-arm64": { - "version": "0.34.5", - "resolved": "https://registry.npmjs.org/@img/sharp-darwin-arm64/-/sharp-darwin-arm64-0.34.5.tgz", - "integrity": "sha512-imtQ3WMJXbMY4fxb/Ndp6HBTNVtWCUI0WdobyheGf5+ad6xX8VIDO8u2xE4qc/fr08CKG/7dDseFtn6M6g/r3w==", + "node_modules/@anthropic-ai/claude-agent-sdk-darwin-arm64": { + "version": "0.2.114", + "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk-darwin-arm64/-/claude-agent-sdk-darwin-arm64-0.2.114.tgz", + "integrity": "sha512-0/6LWrNilWpmiX6Xrj5plsBmCrCdKGERgAlKUZQEJZplnfuweFAJu7WXZB4KBaUpGlPO91zB/yqDh6kp5aZFbA==", "cpu": [ "arm64" ], - "license": "Apache-2.0", + "license": "SEE LICENSE IN LICENSE.md", "optional": true, "os": [ "darwin" - ], - "engines": { - "node": "^18.17.0 || ^20.3.0 || >=21.0.0" - }, - "funding": { - "url": "https://opencollective.com/libvips" - }, - "optionalDependencies": { - "@img/sharp-libvips-darwin-arm64": "1.2.4" - } + ] }, - "node_modules/@img/sharp-darwin-x64": { - "version": "0.34.5", - "resolved": "https://registry.npmjs.org/@img/sharp-darwin-x64/-/sharp-darwin-x64-0.34.5.tgz", - "integrity": "sha512-YNEFAF/4KQ/PeW0N+r+aVVsoIY0/qxxikF2SWdp+NRkmMB7y9LBZAVqQ4yhGCm/H3H270OSykqmQMKLBhBJDEw==", + "node_modules/@anthropic-ai/claude-agent-sdk-darwin-x64": { + "version": "0.2.114", + "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk-darwin-x64/-/claude-agent-sdk-darwin-x64-0.2.114.tgz", + "integrity": "sha512-sOHxq1rEO/KZg2iEZILTPn62lMRRMPqtxKx41uGLi3xjVDrAej6Ury9dDZjYBKkK9n4kBylXV0Oom2CZ14dDYw==", "cpu": [ "x64" ], - "license": "Apache-2.0", + "license": "SEE LICENSE IN LICENSE.md", "optional": true, "os": [ "darwin" - ], - "engines": { - "node": "^18.17.0 || ^20.3.0 || >=21.0.0" - }, - "funding": { - "url": "https://opencollective.com/libvips" - }, - "optionalDependencies": { - "@img/sharp-libvips-darwin-x64": "1.2.4" - } + ] }, - "node_modules/@img/sharp-libvips-darwin-arm64": { - "version": "1.2.4", - "resolved": "https://registry.npmjs.org/@img/sharp-libvips-darwin-arm64/-/sharp-libvips-darwin-arm64-1.2.4.tgz", - "integrity": "sha512-zqjjo7RatFfFoP0MkQ51jfuFZBnVE2pRiaydKJ1G/rHZvnsrHAOcQALIi9sA5co5xenQdTugCvtb1cuf78Vf4g==", + "node_modules/@anthropic-ai/claude-agent-sdk-linux-arm64": { + "version": "0.2.114", + "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk-linux-arm64/-/claude-agent-sdk-linux-arm64-0.2.114.tgz", + "integrity": "sha512-j/SfEoN6+fyEsp8EuPe+xKcGfsZtaBmdUUH+YSRk5H/lYgy38yNsDhdt+AJMQcdMKfHsiwZ3Y9Ajoe9G9wNwHQ==", "cpu": [ "arm64" ], - "license": "LGPL-3.0-or-later", - "optional": true, - "os": [ - "darwin" - ], - "funding": { - "url": "https://opencollective.com/libvips" - } - }, - "node_modules/@img/sharp-libvips-darwin-x64": { - "version": "1.2.4", - "resolved": "https://registry.npmjs.org/@img/sharp-libvips-darwin-x64/-/sharp-libvips-darwin-x64-1.2.4.tgz", - "integrity": "sha512-1IOd5xfVhlGwX+zXv2N93k0yMONvUlANylbJw1eTah8K/Jtpi15KC+WSiaX/nBmbm2HxRM1gZ0nSdjSsrZbGKg==", - "cpu": [ - "x64" + "libc": [ + "glibc" ], - "license": "LGPL-3.0-or-later", - "optional": true, - "os": [ - "darwin" - ], - "funding": { - "url": "https://opencollective.com/libvips" - } - }, - "node_modules/@img/sharp-libvips-linux-arm": { - "version": "1.2.4", - "resolved": "https://registry.npmjs.org/@img/sharp-libvips-linux-arm/-/sharp-libvips-linux-arm-1.2.4.tgz", - "integrity": "sha512-bFI7xcKFELdiNCVov8e44Ia4u2byA+l3XtsAj+Q8tfCwO6BQ8iDojYdvoPMqsKDkuoOo+X6HZA0s0q11ANMQ8A==", - "cpu": [ - "arm" - ], - "license": "LGPL-3.0-or-later", + "license": "SEE LICENSE IN LICENSE.md", "optional": true, "os": [ "linux" - ], - "funding": { - "url": "https://opencollective.com/libvips" - } + ] }, - "node_modules/@img/sharp-libvips-linux-arm64": { - "version": "1.2.4", - "resolved": "https://registry.npmjs.org/@img/sharp-libvips-linux-arm64/-/sharp-libvips-linux-arm64-1.2.4.tgz", - "integrity": "sha512-excjX8DfsIcJ10x1Kzr4RcWe1edC9PquDRRPx3YVCvQv+U5p7Yin2s32ftzikXojb1PIFc/9Mt28/y+iRklkrw==", + "node_modules/@anthropic-ai/claude-agent-sdk-linux-arm64-musl": { + "version": "0.2.114", + "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk-linux-arm64-musl/-/claude-agent-sdk-linux-arm64-musl-0.2.114.tgz", + "integrity": "sha512-Mhd7bumTwWvkgjSJnYvCgyt8DfmLiUoK92mfvAKxHX7i5YSw+h5Kprqh2Cap+2SBbpwZvnwIoEYGCxhGwE5ddg==", "cpu": [ "arm64" ], - "license": "LGPL-3.0-or-later", + "libc": [ + "musl" + ], + "license": "SEE LICENSE IN LICENSE.md", "optional": true, "os": [ "linux" - ], - "funding": { - "url": "https://opencollective.com/libvips" - } + ] }, - "node_modules/@img/sharp-libvips-linux-x64": { - "version": "1.2.4", - "resolved": "https://registry.npmjs.org/@img/sharp-libvips-linux-x64/-/sharp-libvips-linux-x64-1.2.4.tgz", - "integrity": "sha512-tJxiiLsmHc9Ax1bz3oaOYBURTXGIRDODBqhveVHonrHJ9/+k89qbLl0bcJns+e4t4rvaNBxaEZsFtSfAdquPrw==", + "node_modules/@anthropic-ai/claude-agent-sdk-linux-x64": { + "version": "0.2.114", + "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk-linux-x64/-/claude-agent-sdk-linux-x64-0.2.114.tgz", + "integrity": "sha512-wbaExKDleLlm2zHEhb74GKMLVhtO0IUmFhdimQcdL6CdTkmDE8ZJi53tYWE9+jq+XWNRXoM2yEmKPzXoUmsJng==", "cpu": [ "x64" ], - "license": "LGPL-3.0-or-later", - "optional": true, - "os": [ - "linux" + "libc": [ + "glibc" ], - "funding": { - "url": "https://opencollective.com/libvips" - } - }, - "node_modules/@img/sharp-libvips-linuxmusl-arm64": { - "version": "1.2.4", - "resolved": "https://registry.npmjs.org/@img/sharp-libvips-linuxmusl-arm64/-/sharp-libvips-linuxmusl-arm64-1.2.4.tgz", - "integrity": "sha512-FVQHuwx1IIuNow9QAbYUzJ+En8KcVm9Lk5+uGUQJHaZmMECZmOlix9HnH7n1TRkXMS0pGxIJokIVB9SuqZGGXw==", - "cpu": [ - "arm64" - ], - "license": "LGPL-3.0-or-later", + "license": "SEE LICENSE IN LICENSE.md", "optional": true, "os": [ "linux" - ], - "funding": { - "url": "https://opencollective.com/libvips" - } + ] }, - "node_modules/@img/sharp-libvips-linuxmusl-x64": { - "version": "1.2.4", - "resolved": "https://registry.npmjs.org/@img/sharp-libvips-linuxmusl-x64/-/sharp-libvips-linuxmusl-x64-1.2.4.tgz", - "integrity": "sha512-+LpyBk7L44ZIXwz/VYfglaX/okxezESc6UxDSoyo2Ks6Jxc4Y7sGjpgU9s4PMgqgjj1gZCylTieNamqA1MF7Dg==", + "node_modules/@anthropic-ai/claude-agent-sdk-linux-x64-musl": { + "version": "0.2.114", + "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk-linux-x64-musl/-/claude-agent-sdk-linux-x64-musl-0.2.114.tgz", + "integrity": "sha512-c1URsameGHAcghen+mY6jvr2oypiAPHXJIdP4huxR25zPdXWv2x+BCy+vcRVeajsq4VmFzAyQJwaM+BXkmXjAw==", "cpu": [ "x64" ], - "license": "LGPL-3.0-or-later", - "optional": true, - "os": [ - "linux" - ], - "funding": { - "url": "https://opencollective.com/libvips" - } - }, - "node_modules/@img/sharp-linux-arm": { - "version": "0.34.5", - "resolved": "https://registry.npmjs.org/@img/sharp-linux-arm/-/sharp-linux-arm-0.34.5.tgz", - "integrity": "sha512-9dLqsvwtg1uuXBGZKsxem9595+ujv0sJ6Vi8wcTANSFpwV/GONat5eCkzQo/1O6zRIkh0m/8+5BjrRr7jDUSZw==", - "cpu": [ - "arm" + "libc": [ + "musl" ], - "license": "Apache-2.0", + "license": "SEE LICENSE IN LICENSE.md", "optional": true, "os": [ "linux" - ], - "engines": { - "node": "^18.17.0 || ^20.3.0 || >=21.0.0" - }, - "funding": { - "url": "https://opencollective.com/libvips" - }, - "optionalDependencies": { - "@img/sharp-libvips-linux-arm": "1.2.4" - } + ] }, - "node_modules/@img/sharp-linux-arm64": { - "version": "0.34.5", - "resolved": "https://registry.npmjs.org/@img/sharp-linux-arm64/-/sharp-linux-arm64-0.34.5.tgz", - "integrity": "sha512-bKQzaJRY/bkPOXyKx5EVup7qkaojECG6NLYswgktOZjaXecSAeCWiZwwiFf3/Y+O1HrauiE3FVsGxFg8c24rZg==", + "node_modules/@anthropic-ai/claude-agent-sdk-win32-arm64": { + "version": "0.2.114", + "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk-win32-arm64/-/claude-agent-sdk-win32-arm64-0.2.114.tgz", + "integrity": "sha512-qeWdUpQymcKCA92osPmffG4QogrOSvuffPvm6c2OlMDjCPYs8vKG7bSe1Vq5tP9tfBszKPVJWBDh+2ANkNissQ==", "cpu": [ "arm64" ], - "license": "Apache-2.0", + "license": "SEE LICENSE IN LICENSE.md", "optional": true, "os": [ - "linux" - ], - "engines": { - "node": "^18.17.0 || ^20.3.0 || >=21.0.0" - }, - "funding": { - "url": "https://opencollective.com/libvips" - }, - "optionalDependencies": { - "@img/sharp-libvips-linux-arm64": "1.2.4" - } + "win32" + ] }, - "node_modules/@img/sharp-linux-x64": { - "version": "0.34.5", - "resolved": "https://registry.npmjs.org/@img/sharp-linux-x64/-/sharp-linux-x64-0.34.5.tgz", - "integrity": "sha512-MEzd8HPKxVxVenwAa+JRPwEC7QFjoPWuS5NZnBt6B3pu7EG2Ge0id1oLHZpPJdn3OQK+BQDiw9zStiHBTJQQQQ==", + "node_modules/@anthropic-ai/claude-agent-sdk-win32-x64": { + "version": "0.2.114", + "resolved": "https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk-win32-x64/-/claude-agent-sdk-win32-x64-0.2.114.tgz", + "integrity": "sha512-nVr43WwsKvWA6rojw15qBS/f31srukdLxy1KwKzpftlpmkzQ9Lh8uhIafOmoIPzz67f8VJ8JqHE0caA5YrhX9A==", "cpu": [ "x64" ], - "license": "Apache-2.0", + "license": "SEE LICENSE IN LICENSE.md", "optional": true, "os": [ - "linux" - ], - "engines": { - "node": "^18.17.0 || ^20.3.0 || >=21.0.0" - }, - "funding": { - "url": "https://opencollective.com/libvips" - }, - "optionalDependencies": { - "@img/sharp-libvips-linux-x64": "1.2.4" - } + "win32" + ] }, - "node_modules/@img/sharp-linuxmusl-arm64": { - "version": "0.34.5", - "resolved": "https://registry.npmjs.org/@img/sharp-linuxmusl-arm64/-/sharp-linuxmusl-arm64-0.34.5.tgz", - "integrity": "sha512-fprJR6GtRsMt6Kyfq44IsChVZeGN97gTD331weR1ex1c1rypDEABN6Tm2xa1wE6lYb5DdEnk03NZPqA7Id21yg==", - "cpu": [ - "arm64" - ], - "license": "Apache-2.0", - "optional": true, - "os": [ - "linux" - ], - "engines": { - "node": "^18.17.0 || ^20.3.0 || >=21.0.0" + "node_modules/@anthropic-ai/claude-agent-sdk/node_modules/@anthropic-ai/sdk": { + "version": "0.81.0", + "resolved": "https://registry.npmjs.org/@anthropic-ai/sdk/-/sdk-0.81.0.tgz", + "integrity": "sha512-D4K5PvEV6wPiRtVlVsJHIUhHAmOZ6IT/I9rKlTf84gR7GyyAurPJK7z9BOf/AZqC5d1DhYQGJNKRmV+q8dGhgw==", + "license": "MIT", + "dependencies": { + "json-schema-to-ts": "^3.1.1" }, - "funding": { - "url": "https://opencollective.com/libvips" + "bin": { + "anthropic-ai-sdk": "bin/cli" }, - "optionalDependencies": { - "@img/sharp-libvips-linuxmusl-arm64": "1.2.4" + "peerDependencies": { + "zod": "^3.25.0 || ^4.0.0" + }, + "peerDependenciesMeta": { + "zod": { + "optional": true + } } }, - "node_modules/@img/sharp-linuxmusl-x64": { - "version": "0.34.5", - "resolved": "https://registry.npmjs.org/@img/sharp-linuxmusl-x64/-/sharp-linuxmusl-x64-0.34.5.tgz", - "integrity": "sha512-Jg8wNT1MUzIvhBFxViqrEhWDGzqymo3sV7z7ZsaWbZNDLXRJZoRGrjulp60YYtV4wfY8VIKcWidjojlLcWrd8Q==", - "cpu": [ - "x64" - ], - "license": "Apache-2.0", - "optional": true, - "os": [ - "linux" - ], - "engines": { - "node": "^18.17.0 || ^20.3.0 || >=21.0.0" - }, - "funding": { - "url": "https://opencollective.com/libvips" - }, - "optionalDependencies": { - "@img/sharp-libvips-linuxmusl-x64": "1.2.4" + "node_modules/@anthropic-ai/sdk": { + "version": "0.39.0", + "resolved": "https://registry.npmjs.org/@anthropic-ai/sdk/-/sdk-0.39.0.tgz", + "integrity": "sha512-eMyDIPRZbt1CCLErRCi3exlAvNkBtRe+kW5vvJyef93PmNr/clstYgHhtvmkxN82nlKgzyGPCyGxrm0JQ1ZIdg==", + "license": "MIT", + "dependencies": { + "@types/node": "^18.11.18", + "@types/node-fetch": "^2.6.4", + "abort-controller": "^3.0.0", + "agentkeepalive": "^4.2.1", + "form-data-encoder": "1.7.2", + "formdata-node": "^4.3.2", + "node-fetch": "^2.6.7" } }, - "node_modules/@img/sharp-win32-arm64": { - "version": "0.34.5", - "resolved": "https://registry.npmjs.org/@img/sharp-win32-arm64/-/sharp-win32-arm64-0.34.5.tgz", - "integrity": "sha512-WQ3AgWCWYSb2yt+IG8mnC6Jdk9Whs7O0gxphblsLvdhSpSTtmu69ZG1Gkb6NuvxsNACwiPV6cNSZNzt0KPsw7g==", - "cpu": [ - "arm64" - ], - "license": "Apache-2.0 AND LGPL-3.0-or-later", - "optional": true, - "os": [ - "win32" - ], + "node_modules/@babel/runtime": { + "version": "7.29.2", + "resolved": "https://registry.npmjs.org/@babel/runtime/-/runtime-7.29.2.tgz", + "integrity": "sha512-JiDShH45zKHWyGe4ZNVRrCjBz8Nh9TMmZG1kh4QTK8hCBTWBi8Da+i7s1fJw7/lYpM4ccepSNfqzZ/QvABBi5g==", + "license": "MIT", "engines": { - "node": "^18.17.0 || ^20.3.0 || >=21.0.0" - }, - "funding": { - "url": "https://opencollective.com/libvips" + "node": ">=6.9.0" } }, - "node_modules/@img/sharp-win32-x64": { - "version": "0.34.5", - "resolved": "https://registry.npmjs.org/@img/sharp-win32-x64/-/sharp-win32-x64-0.34.5.tgz", - "integrity": "sha512-+29YMsqY2/9eFEiW93eqWnuLcWcufowXewwSNIT6UwZdUUCrM3oFjMWH/Z6/TMmb4hlFenmfAVbpWeup2jryCw==", - "cpu": [ - "x64" - ], - "license": "Apache-2.0 AND LGPL-3.0-or-later", - "optional": true, - "os": [ - "win32" - ], + "node_modules/@hono/node-server": { + "version": "1.19.11", + "resolved": "https://registry.npmjs.org/@hono/node-server/-/node-server-1.19.11.tgz", + "integrity": "sha512-dr8/3zEaB+p0D2n/IUrlPF1HZm586qgJNXK1a9fhg/PzdtkK7Ksd5l312tJX2yBuALqDYBlG20QEbayqPyxn+g==", + "license": "MIT", "engines": { - "node": "^18.17.0 || ^20.3.0 || >=21.0.0" + "node": ">=18.14.1" }, - "funding": { - "url": "https://opencollective.com/libvips" + "peerDependencies": { + "hono": "^4" } }, "node_modules/@modelcontextprotocol/sdk": { diff --git a/README.md b/README.md index b49e7d46c0..77717b8d6b 100644 --- a/README.md +++ b/README.md @@ -165,6 +165,9 @@ Special thanks to these community members who have contributed code to SuperDoc: baristaGeek Anuj52 Abdeltoto +JoaaoVerona +michaelreavant +ArturQuirino Want to see your avatar here? Check the [Contributing Guide](CONTRIBUTING.md) to get started. diff --git a/apps/docs/core/superdoc/configuration.mdx b/apps/docs/core/superdoc/configuration.mdx index 6433165c19..855e0bf2bb 100644 --- a/apps/docs/core/superdoc/configuration.mdx +++ b/apps/docs/core/superdoc/configuration.mdx @@ -112,7 +112,7 @@ new SuperDoc({ - `viewing` - Read-only display - `suggesting` - Track changes enabled - See the [Track Changes extension](/extensions/track-changes) for accept/reject commands, and the [runnable example](https://github.com/superdoc-dev/superdoc/tree/main/examples/features/track-changes) for a complete workflow. + See the [Track Changes module](/modules/track-changes) for accept/reject commands, the Document API, and configuration. The [runnable example](https://github.com/superdoc-dev/superdoc/tree/main/examples/features/track-changes) shows a complete workflow. @@ -124,11 +124,11 @@ new SuperDoc({ - - Viewing-mode visibility controls for tracked changes + + **Deprecated** — Use [`modules.trackChanges`](#track-changes-module) instead. This top-level key remains supported as an alias and will emit a one-time console warning. - Show tracked-change markup and threads when `documentMode` is `viewing` + Show tracked-change markup and threads when `documentMode` is `viewing`. @@ -196,6 +196,56 @@ new SuperDoc({ +### Track changes module + + + Track changes configuration. Supersedes the top-level `trackChanges` and `layoutEngineOptions.trackedChanges` keys, which remain supported as deprecated aliases. + + + + Show tracked-change markup and threads when `documentMode` is `viewing`. + + + Rendering mode for tracked changes. + - `'review'`: show insertions and deletions inline (default for editing/suggesting). + - `'original'`: show the document as it existed before tracked changes (default for viewing when `visible` is `false`). + - `'final'`: show the document with changes applied. + - `'off'`: disable tracked-change rendering. + + + Whether the layout engine treats tracked changes as active. + + + How a tracked replacement (adjacent insertion + deletion created by typing over selected text) surfaces in the UI and API. + - `'paired'` (default, Google Docs model): the two halves share one id and resolve together with a single accept/reject click. + - `'independent'` (Microsoft Word / ECMA-376 §17.13.5 model): each insertion and each deletion has its own id, is addressable on its own, and resolves independently. + + + + +```javascript +new SuperDoc({ + selector: '#editor', + document: 'contract.docx', + documentMode: 'viewing', + modules: { + trackChanges: { visible: true, mode: 'review' }, + }, +}); +``` + +Opt into Microsoft Word / ECMA-376-style independent revisions, where each insertion and each deletion has its own id and resolves on its own: + +```javascript +new SuperDoc({ + selector: '#editor', + document: 'contract.docx', + modules: { + trackChanges: { replacements: 'independent' }, + }, +}); +``` + ### Toolbar module diff --git a/apps/docs/docs.json b/apps/docs/docs.json index 8c9d8df79f..d7ac1c5269 100644 --- a/apps/docs/docs.json +++ b/apps/docs/docs.json @@ -154,6 +154,7 @@ ] }, "modules/comments", + "modules/track-changes", { "group": "Toolbar", "tag": "NEW", @@ -455,6 +456,10 @@ "source": "/extensions/slash-menu", "destination": "/extensions/context-menu" }, + { + "source": "/extensions/track-changes", + "destination": "/modules/track-changes" + }, { "source": "/guides/breaking-changes-v1", "destination": "/guides/migration/breaking-changes-v1" diff --git a/apps/docs/extensions/overview.mdx b/apps/docs/extensions/overview.mdx index 76eb84d7c9..584d03ecb2 100644 --- a/apps/docs/extensions/overview.mdx +++ b/apps/docs/extensions/overview.mdx @@ -53,7 +53,7 @@ Basic document capabilities: ### Advanced features Complex functionality: -- **[Track Changes](/extensions/track-changes)** - Revision tracking +- **[Track Changes](/modules/track-changes)** - Revision tracking - **[Comments](/extensions/comments)** - Discussions - **[Field Annotation](/extensions/field-annotation)** - Form fields - **[Document Section](/extensions/document-section)** - Locked sections diff --git a/apps/docs/extensions/track-changes.mdx b/apps/docs/extensions/track-changes.mdx deleted file mode 100644 index f86de2f950..0000000000 --- a/apps/docs/extensions/track-changes.mdx +++ /dev/null @@ -1,301 +0,0 @@ ---- -hidden: true -title: Track Changes extension -sidebarTitle: Track Changes ---- - -Track Changes records all edits with author attribution and timestamps, matching Microsoft Word's revision tracking. - -## Usage - -Enable through document mode: - -```javascript -superdoc.setDocumentMode('suggesting'); // Enable tracking -superdoc.setDocumentMode('editing'); // Disable tracking -``` - -Or toggle programmatically: - - -```javascript Usage -editor.commands.enableTrackChanges() -editor.commands.disableTrackChanges() -editor.commands.toggleTrackChanges() -``` - -```javascript Full Example -import { SuperDoc } from 'superdoc'; -import 'superdoc/style.css'; - -const superdoc = new SuperDoc({ - selector: '#editor', - document: yourFile, - onReady: (superdoc) => { - const editor = superdoc.activeEditor; - editor.commands.enableTrackChanges() - editor.commands.disableTrackChanges() - editor.commands.toggleTrackChanges() - }, -}); -``` - - -## Commands - -### Accept changes - - -```javascript Usage -// Accept at current selection -editor.commands.acceptTrackedChangeBySelection() - -// Accept a specific change by ID -editor.commands.acceptTrackedChangeById('change-123') - -// Accept a change object (with start/end positions) -editor.commands.acceptTrackedChange({ trackedChange: { start: 10, end: 50 } }) - -// Accept changes in a range -editor.commands.acceptTrackedChangesBetween(10, 50) - -// Accept all changes in the document -editor.commands.acceptAllTrackedChanges() - -// Toolbar-aware accept (uses active thread or selection) -editor.commands.acceptTrackedChangeFromToolbar() -``` - -```javascript Full Example -import { SuperDoc } from 'superdoc'; -import 'superdoc/style.css'; - -const superdoc = new SuperDoc({ - selector: '#editor', - document: yourFile, - onReady: (superdoc) => { - const editor = superdoc.activeEditor; - // Accept at current selection - editor.commands.acceptTrackedChangeBySelection() - - // Accept a specific change by ID - editor.commands.acceptTrackedChangeById('change-123') - - // Accept a change object (with start/end positions) - editor.commands.acceptTrackedChange({ trackedChange: { start: 10, end: 50 } }) - - // Accept changes in a range - editor.commands.acceptTrackedChangesBetween(10, 50) - - // Accept all changes in the document - editor.commands.acceptAllTrackedChanges() - - // Toolbar-aware accept (uses active thread or selection) - editor.commands.acceptTrackedChangeFromToolbar() - }, -}); -``` - - -### Reject changes - - -```javascript Usage -// Reject at current selection -editor.commands.rejectTrackedChangeOnSelection() - -// Reject a specific change by ID -editor.commands.rejectTrackedChangeById('change-123') - -// Reject a change object -editor.commands.rejectTrackedChange({ trackedChange: { start: 10, end: 50 } }) - -// Reject changes in a range -editor.commands.rejectTrackedChangesBetween(10, 50) - -// Reject all changes in the document -editor.commands.rejectAllTrackedChanges() - -// Toolbar-aware reject -editor.commands.rejectTrackedChangeFromToolbar() -``` - -```javascript Full Example -import { SuperDoc } from 'superdoc'; -import 'superdoc/style.css'; - -const superdoc = new SuperDoc({ - selector: '#editor', - document: yourFile, - onReady: (superdoc) => { - const editor = superdoc.activeEditor; - // Reject at current selection - editor.commands.rejectTrackedChangeOnSelection() - - // Reject a specific change by ID - editor.commands.rejectTrackedChangeById('change-123') - - // Reject a change object - editor.commands.rejectTrackedChange({ trackedChange: { start: 10, end: 50 } }) - - // Reject changes in a range - editor.commands.rejectTrackedChangesBetween(10, 50) - - // Reject all changes in the document - editor.commands.rejectAllTrackedChanges() - - // Toolbar-aware reject - editor.commands.rejectTrackedChangeFromToolbar() - }, -}); -``` - - -### Insert tracked change programmatically - -Use `insertTrackedChange` to add tracked edits from external sources (e.g., AI suggestions): - - -```javascript Usage -editor.commands.insertTrackedChange({ - from: 10, - to: 25, - text: 'replacement text', - comment: 'AI suggestion: improved wording' -}) -``` - -```javascript Full Example -import { SuperDoc } from 'superdoc'; -import 'superdoc/style.css'; - -const superdoc = new SuperDoc({ - selector: '#editor', - document: yourFile, - onReady: (superdoc) => { - const editor = superdoc.activeEditor; - editor.commands.insertTrackedChange({ - from: 10, - to: 25, - text: 'replacement text', - comment: 'AI suggestion: improved wording' - }) - }, -}); -``` - - -**Parameters:** - - - Object with `from`, `to`, `text`, `user`, `comment`, `addToHistory`, `emitCommentEvent` - - -### View modes - - -```javascript Usage -// Show document as it was before changes -editor.commands.toggleTrackChangesShowOriginal() -editor.commands.enableTrackChangesShowOriginal() -editor.commands.disableTrackChangesShowOriginal() - -// Show document as if all changes were accepted -editor.commands.toggleTrackChangesShowFinal() -editor.commands.enableTrackChangesShowFinal() -``` - -```javascript Full Example -import { SuperDoc } from 'superdoc'; -import 'superdoc/style.css'; - -const superdoc = new SuperDoc({ - selector: '#editor', - document: yourFile, - onReady: (superdoc) => { - const editor = superdoc.activeEditor; - // Show document as it was before changes - editor.commands.toggleTrackChangesShowOriginal() - editor.commands.enableTrackChangesShowOriginal() - editor.commands.disableTrackChangesShowOriginal() - - // Show document as if all changes were accepted - editor.commands.toggleTrackChangesShowFinal() - editor.commands.enableTrackChangesShowFinal() - }, -}); -``` - - -## Helpers - -```javascript -import { trackChangesHelpers } from 'superdoc'; - -// Get all tracked changes in the document -const changes = trackChangesHelpers.getTrackChanges(editor.state); -// Returns: [{ mark, from, to }, ...] - -// Get a specific change by ID -const change = trackChangesHelpers.getTrackChanges(editor.state, 'change-123'); -``` - -## Change types - -| Type | Mark | Visual | -|------|------|--------| -| Insertion | `trackInsert` | Green underline | -| Deletion | `trackDelete` | Red strikethrough | -| Format change | `trackFormat` | Records before/after formatting | - -Each change includes author name, email, timestamp, and a unique ID. - -## Export behavior - -Changes export to DOCX as Word revisions: - - -```javascript Usage -// Export with changes preserved -await superdoc.export(); - -// Accept all first, then export clean -editor.commands.acceptAllTrackedChanges(); -await superdoc.export(); -``` - -```javascript Full Example -import { SuperDoc } from 'superdoc'; -import 'superdoc/style.css'; - -const superdoc = new SuperDoc({ - selector: '#editor', - document: yourFile, - onReady: (superdoc) => { - const editor = superdoc.activeEditor; - // Export with changes preserved - await superdoc.export(); - - // Accept all first, then export clean - editor.commands.acceptAllTrackedChanges(); - await superdoc.export(); - }, -}); -``` - - -## Full example - - - Runnable example with mode switching, accept/reject, and comments sidebar - - -## Source code - -import { SourceCodeLink } from '/snippets/components/source-code-link.jsx' - - diff --git a/apps/docs/getting-started/frameworks/angular.mdx b/apps/docs/getting-started/frameworks/angular.mdx index 9facafb7ee..3395a0e6e7 100644 --- a/apps/docs/getting-started/frameworks/angular.mdx +++ b/apps/docs/getting-started/frameworks/angular.mdx @@ -6,6 +6,8 @@ keywords: "angular docx editor, angular word component, superdoc angular, angula SuperDoc works with Angular through direct DOM manipulation. No wrapper needed. +Requires Angular 17.2+. `viewChild()` and `input()` are stable from Angular 19; developer preview before that. On older versions, use `@ViewChild`, `@Input`, and `ngOnDestroy` in place of `inject(DestroyRef)`. + ## Install ```bash @@ -15,7 +17,7 @@ npm install superdoc ## Basic setup ```typescript -import { Component, ElementRef, ViewChild, OnInit, OnDestroy } from '@angular/core'; +import { Component, ElementRef, viewChild, AfterViewInit, inject, DestroyRef } from '@angular/core'; import { SuperDoc } from 'superdoc'; import 'superdoc/style.css'; @@ -23,21 +25,21 @@ import 'superdoc/style.css'; selector: 'app-editor', template: `
`, }) -export class EditorComponent implements OnInit, OnDestroy { - @ViewChild('editor', { static: true }) editorRef!: ElementRef; +export class EditorComponent implements AfterViewInit { + private readonly editorRef = viewChild.required>('editor'); private superdoc: SuperDoc | null = null; - ngOnInit() { + constructor() { + inject(DestroyRef).onDestroy(() => this.superdoc?.destroy()); + } + + ngAfterViewInit() { this.superdoc = new SuperDoc({ - selector: this.editorRef.nativeElement, + selector: this.editorRef().nativeElement, document: 'contract.docx', documentMode: 'editing', }); } - - ngOnDestroy() { - this.superdoc?.destroy(); - } } ``` @@ -46,7 +48,7 @@ export class EditorComponent implements OnInit, OnDestroy { A reusable editor component with mode switching and export: ```typescript -import { Component, ElementRef, ViewChild, Input, OnInit, OnDestroy } from '@angular/core'; +import { Component, ElementRef, viewChild, input, AfterViewInit, inject, DestroyRef } from '@angular/core'; import { SuperDoc } from 'superdoc'; import 'superdoc/style.css'; @@ -62,27 +64,27 @@ import 'superdoc/style.css';
`, }) -export class DocEditorComponent implements OnInit, OnDestroy { - @ViewChild('editor', { static: true }) editorRef!: ElementRef; - @Input() document!: File | string; - @Input() user?: { name: string; email: string }; +export class DocEditorComponent implements AfterViewInit { + private readonly editorRef = viewChild.required>('editor'); + readonly document = input.required(); + readonly user = input<{ name: string; email: string }>(); private superdoc: SuperDoc | null = null; - ngOnInit() { + constructor() { + inject(DestroyRef).onDestroy(() => this.superdoc?.destroy()); + } + + ngAfterViewInit() { this.superdoc = new SuperDoc({ - selector: this.editorRef.nativeElement, - document: this.document, + selector: this.editorRef().nativeElement, + document: this.document(), documentMode: 'editing', - user: this.user, - onReady: () => console.log('Editor ready'), + user: this.user(), + onReady: () => console.log('Editor ready!'), }); } - ngOnDestroy() { - this.superdoc?.destroy(); - } - setMode(mode: 'editing' | 'viewing' | 'suggesting') { this.superdoc?.setDocumentMode(mode); } @@ -96,6 +98,10 @@ export class DocEditorComponent implements OnInit, OnDestroy { ## Handle file uploads ```typescript +import { Component, ElementRef, viewChild, inject, DestroyRef } from '@angular/core'; +import { SuperDoc } from 'superdoc'; +import 'superdoc/style.css'; + @Component({ selector: 'app-upload-editor', template: ` @@ -103,25 +109,25 @@ export class DocEditorComponent implements OnInit, OnDestroy {
`, }) -export class UploadEditorComponent implements OnDestroy { - @ViewChild('editor', { static: true }) editorRef!: ElementRef; +export class UploadEditorComponent { + private readonly editorRef = viewChild.required>('editor'); private superdoc: SuperDoc | null = null; + constructor() { + inject(DestroyRef).onDestroy(() => this.superdoc?.destroy()); + } + onFileChange(event: Event) { const file = (event.target as HTMLInputElement).files?.[0]; if (!file) return; this.superdoc?.destroy(); this.superdoc = new SuperDoc({ - selector: this.editorRef.nativeElement, + selector: this.editorRef().nativeElement, document: file, documentMode: 'editing', }); } - - ngOnDestroy() { - this.superdoc?.destroy(); - } } ``` diff --git a/apps/docs/llms.txt b/apps/docs/llms.txt index 0f0668d445..1e76a62b5b 100644 --- a/apps/docs/llms.txt +++ b/apps/docs/llms.txt @@ -9,6 +9,7 @@ SuperDoc renders, edits, and automates .docx files — in the browser, headless - Reads and writes OOXML natively — documents stay real .docx files at every step - 180+ MCP tools covering reading, editing, formatting, comments, tracked changes, and more - Tracked changes and comments as first-class operations +- Math equations render natively as MathML — no external library needed - Same document model across browser editor, headless Node, and APIs - Stateless API for convert, annotate, sign, and verify - Real-time collaboration with Yjs CRDT diff --git a/apps/docs/modules/comments.mdx b/apps/docs/modules/comments.mdx index cf9b1f456d..8c6e0783a6 100644 --- a/apps/docs/modules/comments.mdx +++ b/apps/docs/modules/comments.mdx @@ -140,20 +140,27 @@ modules: { ## Viewing mode visibility -Comments are hidden by default when `documentMode` is `viewing`. Use the -top-level `comments.visible` and `trackChanges.visible` flags to control what -renders in read-only mode. +Comments are hidden by default when `documentMode` is `viewing`. Use +`comments.visible` and `modules.trackChanges.visible` to control what renders +in read-only mode. ```javascript new SuperDoc({ selector: "#viewer", document: "contract.docx", documentMode: "viewing", - comments: { visible: true }, // Standard comment threads - trackChanges: { visible: false }, // Tracked-change markup + threads + comments: { visible: true }, // Standard comment threads + modules: { + trackChanges: { visible: false }, // Tracked-change markup + threads + }, }); ``` + + The top-level `trackChanges` key still works as a deprecated alias for + `modules.trackChanges` and will emit a one-time console warning. + + ## Setting up the comments UI During initialization: diff --git a/apps/docs/modules/overview.mdx b/apps/docs/modules/overview.mdx index d40a7de9a8..e2326b85e8 100644 --- a/apps/docs/modules/overview.mdx +++ b/apps/docs/modules/overview.mdx @@ -29,6 +29,9 @@ const superdoc = new SuperDoc({ Threaded discussions and annotations + + Word-style revision tracking with accept/reject + Customizable formatting controls @@ -53,9 +56,6 @@ Each module is configured via `modules.` in the [SuperDoc configuration](/ These features are configured at the top level rather than through `modules`, but are commonly used alongside modules. - - Accept/reject workflow with `documentMode: 'suggesting'` - Provider-based spell check on the layout-engine editor surface diff --git a/apps/docs/modules/toolbar/built-in.mdx b/apps/docs/modules/toolbar/built-in.mdx index b99b12b8ed..2485f9d7a5 100644 --- a/apps/docs/modules/toolbar/built-in.mdx +++ b/apps/docs/modules/toolbar/built-in.mdx @@ -328,21 +328,24 @@ The toolbar dropdown lists only what you register in `fonts`. Imported documents - Display name shown in the dropdown + Display name shown in the dropdown, and the value applied to the selected text. Must match the first family name in `key` — otherwise the dropdown won't light up the current font when the cursor is inside a run that uses it. - Font-family CSS value applied to text + Stable identity for the option. Also used as the row's preview `font-family` so each row renders in its own typeface. Typically a full CSS stack (e.g. `'Cambria, serif'`). Font weight + + Optional. Overrides per-row rendering. Use `props.style.fontFamily` to preview the row in a different font than `key`. + ```javascript fonts: [ - { label: 'Arial', key: 'Arial' }, - { label: 'Times', key: 'Times New Roman' }, + { label: 'Arial', key: 'Arial, sans-serif' }, + { label: 'Times New Roman', key: 'Times New Roman, serif' }, { label: 'Brand Font', key: 'BrandFont, sans-serif', fontWeight: 400 } ] ``` diff --git a/apps/docs/modules/track-changes.mdx b/apps/docs/modules/track-changes.mdx new file mode 100644 index 0000000000..d61a2b8c43 --- /dev/null +++ b/apps/docs/modules/track-changes.mdx @@ -0,0 +1,443 @@ +--- +title: Track Changes +keywords: "word track changes, document revisions, accept reject edits, docx tracked changes, review workflow, suggesting mode" +--- + +Word-style revision tracking. Every edit carries an author, a timestamp, and an id. Accept or reject one at a time, by selection, or in bulk. Changes round-trip through DOCX as native Word revisions — import, edit, export, nothing lost. + +## Quick start + +```javascript +const superdoc = new SuperDoc({ + selector: "#editor", + document: "contract.docx", + documentMode: "suggesting", // record new edits as tracked changes + user: { + name: "John Smith", + email: "john@company.com", + }, + modules: { + trackChanges: { + visible: true, + }, + }, + onCommentsUpdate: (payload) => { + if (payload.type === "trackedChange") { + console.log(payload.event, payload.trackedChangeType, payload.changeId); + } + }, +}); +``` + + + Tracked-edit recording follows `documentMode`. Set `documentMode: 'suggesting'` (or call `superdoc.setDocumentMode('suggesting')` later) to start recording new edits as revisions. + + +## Configuration + + + Show tracked-change markup when `documentMode` is `viewing`. + + + + Rendering mode. + + + - `'review'` — show insertions and deletions inline (default for editing and suggesting). + - `'original'` — show the document as it was before tracked changes (default for viewing when `visible` is `false`). + - `'final'` — show the document with all changes applied. + - `'off'` — suppress tracked-change rendering entirely. + + + + + Whether the layout engine treats tracked changes as active. Turn off to render the document without any revision UI. + + + + How a tracked replacement (typing over selected text) surfaces in the API and UI. See [Revision model](#revision-model). + + + - `'paired'` (default, Google Docs model) — the insertion and deletion share one id and resolve together. + - `'independent'` (Microsoft Word / ECMA-376 §17.13.5) — each side has its own id and resolves on its own. + + + +## Viewing mode visibility + +Tracked-change markup is hidden by default when `documentMode` is `'viewing'`. Flip `modules.trackChanges.visible` to show it in read-only mode. + +```javascript +new SuperDoc({ + selector: "#viewer", + document: "contract.docx", + documentMode: "viewing", + modules: { + trackChanges: { visible: true }, + }, +}); +``` + + + The top-level `trackChanges` key still works as a deprecated alias for `modules.trackChanges` and prints a one-time console warning. + + +## Revision model + +SuperDoc supports two models for how a tracked replacement (an insertion paired with a deletion, created when a user types over selected text) shows up in the API and UI. Pick the one that matches the editor your users expect. + +| Model | `modules.trackChanges.replacements` | Behavior | +| --- | --- | --- | +| **Paired** (default — Google Docs) | `'paired'` | Both halves share one id. One accept/reject resolves both. One sidebar row per replacement. | +| **Independent** (Microsoft Word / ECMA-376 §17.13.5) | `'independent'` | Each insertion and each deletion has its own id. Accept/reject resolves one side at a time. A replacement produces two sidebar rows. | + +Both modes round-trip cleanly through DOCX — the OOXML always emits one `` / `` per mark. The difference is how the API surfaces the revisions at runtime. + +### Paired (default) + +```javascript +const superdoc = new SuperDoc({ + selector: "#editor", + document: yourFile, + // default: modules.trackChanges.replacements === 'paired' +}); +``` + +### Independent (Word-style) + +```javascript +const superdoc = new SuperDoc({ + selector: "#editor", + document: yourFile, + modules: { + trackChanges: { replacements: "independent" }, + }, +}); +``` + +With `replacements: 'independent'`, `editor.doc.trackChanges.list()` returns one entry per revision and `decide({ id })` resolves exactly that one side. The other half of the replacement stays in the document, still addressable by its own id — useful when you're building a custom sidebar and want each revision as a separate row. + +## Document API + +Use the Document API to list, read, and resolve tracked changes. It's stable, typed, framework-agnostic, and works the same in the visual editor and headless mode. + +```javascript +const editor = superdoc.activeEditor; + +// List every tracked change. In 'paired' mode a replacement is one entry; +// in 'independent' mode it's two (one insert, one delete). +const result = editor.doc.trackChanges.list(); + +for (const item of result.items) { + console.log(item.id, item.type, item.author, item.excerpt); +} + +// Fetch a single change by id. +const change = editor.doc.trackChanges.get({ id: result.items[0].id }); + +// Accept or reject by id. +editor.doc.trackChanges.decide({ decision: "accept", target: { id: change.id } }); + +// Accept or reject everything in the document. +editor.doc.trackChanges.decide({ decision: "accept", target: { scope: "all" } }); +``` + +Every entry returned by `list()` and `get()` has this shape: + + + + + SuperDoc's internal id for the revision. Stable across calls. + + + Entity address — `{ kind: 'entity', entityType: 'trackedChange', entityId: id }`. + + + Revision kind. In `'paired'` mode a replacement reports as `'insert'` (both halves grouped). In `'independent'` mode the insertion and deletion are separate entries. + + + Display name of the reviewer. + + + Email of the reviewer. + + + Reviewer avatar URL, when provided. + + + ISO timestamp of the revision. + + + The affected text. + + + Original Word `w:id` values from the source DOCX, keyed by `insert` / `delete` / `format`. Useful for correlating SuperDoc revisions with upstream systems. + + + + +`list()` accepts an optional query with `limit`, `offset`, and `type` (`'insert' | 'delete' | 'format'`) for pagination and filtering. See the full reference: + +- [`trackChanges.list`](/document-api/reference/track-changes/list) +- [`trackChanges.get`](/document-api/reference/track-changes/get) +- [`trackChanges.decide`](/document-api/reference/track-changes/decide) + +## Toggling tracked edits + +Control recording via document mode: + +```javascript +superdoc.setDocumentMode("suggesting"); // record new edits as tracked changes +superdoc.setDocumentMode("editing"); // stop recording +superdoc.setDocumentMode("viewing"); // read-only +``` + +## Change types + +| Type | Mark | Visual | +| --- | --- | --- | +| Insertion | `trackInsert` | Underlined in the reviewer's color | +| Deletion | `trackDelete` | Strikethrough in the reviewer's color | +| Format change | `trackFormat` | Records the before/after formatting on the run | + +Each mark carries `id`, `author`, `authorEmail`, `date`, and — for imports from Word — the original `w:id` as `sourceId` so you can round-trip revision provenance. + +## Events + +Tracked-change events are delivered through the same `onCommentsUpdate` callback as comment events. The top-level `type` field tells them apart; filter on `type === 'trackedChange'` and read the flat payload. + +```javascript +onCommentsUpdate: (payload) => { + if (payload.type !== "trackedChange") return; + + switch (payload.event) { + case "add": + // New tracked change created. + break; + case "update": + // Existing tracked change updated (e.g. reply added, author edited). + break; + case "change-accepted": + await markAccepted(payload.changeId); + break; + case "change-rejected": + await markRejected(payload.changeId); + break; + case "resolved": + // Tracked-change comment thread resolved. + break; + } +}; +``` + +### Payload fields + + + + + Always `'trackedChange'` for tracked-change events. Comment events use other values. + + + Event kind: `'add'`, `'update'`, `'deleted'`, `'resolved'`, `'selected'`, `'change-accepted'`, or `'change-rejected'`. + + + The tracked-change id (matches `TrackChangeInfo.id` returned by `trackChanges.list()`). + + + The mark type. `'both'` is used for a paired replacement (insertion and deletion together). + + + The inserted or affected text. + + + The deleted text, for deletions and replacements. + + + A human-facing summary derived from the change (e.g. `'hyperlinkAdded'`, `'formatChanged'`). + + + Display name. + + + Email. + + + Avatar URL, when provided. + + + ISO timestamp. + + + Source document id. + + + Original author info from the imported DOCX, when available — `{ name }`. + + + + + + Events fire once per user action, not once per mark. A tracked replacement in paired mode emits one event with `trackedChangeType: 'both'`. To enumerate the current set of revisions, use `editor.doc.trackChanges.list()` — not the event stream. + + +## Permissions + +Accept and reject permissions are governed by the same `permissionResolver` used for comments. Return `false` from the resolver to block an action. + +```javascript +modules: { + comments: { + permissionResolver: ({ permission, trackedChange, currentUser, defaultDecision }) => { + if ( + permission === "REJECT_OTHER" && + trackedChange?.attrs?.authorEmail !== currentUser?.email + ) { + return false; + } + return defaultDecision; + }, + }, +} +``` + +Tracked-change permission types: + +| Permission | Description | +| --- | --- | +| `RESOLVE_OWN` | Accept your own tracked changes | +| `RESOLVE_OTHER` | Accept other users' tracked changes | +| `REJECT_OWN` | Reject your own tracked changes | +| `REJECT_OTHER` | Reject other users' tracked changes | + +See [Comments → Permission resolver](/modules/comments#permission-resolver) for the full list of permission types and resolver behavior. + +## Word import/export + +Tracked changes round-trip through DOCX as native Word revisions. Import it. Edit it. Export it. Nothing lost. + +```javascript +// Export with revisions preserved. Word opens the file and shows the +// insertions and deletions under Review. +const blob = await superdoc.export(); + +// Accept all first, then export a clean copy. +superdoc.activeEditor.doc.trackChanges.decide({ + decision: "accept", + target: { scope: "all" }, +}); +const cleanBlob = await superdoc.export(); +``` + +Imported Word revisions preserve their original `w:id` values as `wordRevisionIds` on each `TrackChangeInfo` entry, so you can correlate SuperDoc revisions with the source document or an external review system. + + + Round-trip support today covers inserted run content (``), deleted run content (``), and run-level format changes (``). Paragraph-level property changes, tracked table row and cell edits, and tracked moves are on the roadmap — they import as accepted content today. + + +## Legacy editor commands + + + **Deprecated**. Use the [Document API](#document-api) (`editor.doc.trackChanges.list()`, `editor.doc.trackChanges.decide(...)`) instead. The commands below remain available but will be removed in a future release. + + +These legacy commands live on `superdoc.activeEditor.commands` and predate the Document API. They're still used by the built-in toolbar and a handful of keyboard shortcuts. + +### Enable and toggle + +```javascript +superdoc.activeEditor.commands.enableTrackChanges(); +superdoc.activeEditor.commands.disableTrackChanges(); +superdoc.activeEditor.commands.toggleTrackChanges(); +``` + +### Accept + +```javascript +superdoc.activeEditor.commands.acceptTrackedChangeBySelection(); +superdoc.activeEditor.commands.acceptTrackedChangeById("change-123"); +superdoc.activeEditor.commands.acceptTrackedChangesBetween(10, 50); +superdoc.activeEditor.commands.acceptAllTrackedChanges(); +superdoc.activeEditor.commands.acceptTrackedChangeFromToolbar(); +``` + +### Reject + +```javascript +superdoc.activeEditor.commands.rejectTrackedChangeOnSelection(); +superdoc.activeEditor.commands.rejectTrackedChangeById("change-123"); +superdoc.activeEditor.commands.rejectTrackedChangesBetween(10, 50); +superdoc.activeEditor.commands.rejectAllTrackedChanges(); +superdoc.activeEditor.commands.rejectTrackedChangeFromToolbar(); +``` + +### Insert a tracked change programmatically + +```javascript +superdoc.activeEditor.commands.insertTrackedChange({ + from: 10, + to: 25, + text: "replacement text", + comment: "AI suggestion: improved wording", +}); +``` + + + + + Start of the range. + + + End of the range. When `from === to`, the change is a pure insertion. + + + Replacement text. Omit or leave empty for a pure deletion. + + + Explicit id for the change. Defaults to a generated UUID. + + + Author override (`{ name, email, image? }`). Defaults to the editor's `user` option. + + + Optional comment to attach. + + + Record the transaction in the undo stack. + + + Emit a `trackedChange` event through `onCommentsUpdate`. + + + + +### View modes + +Temporarily render the document without applying revisions — useful for previewing the accepted or original state: + +```javascript +// Show the document as it was before tracked changes. +superdoc.activeEditor.commands.enableTrackChangesShowOriginal(); +superdoc.activeEditor.commands.disableTrackChangesShowOriginal(); +superdoc.activeEditor.commands.toggleTrackChangesShowOriginal(); + +// Show the document with all changes accepted. +superdoc.activeEditor.commands.enableTrackChangesShowFinal(); +superdoc.activeEditor.commands.toggleTrackChangesShowFinal(); +``` + +## Full example + + + Runnable example: mode switching, accept and reject, comments sidebar, DOCX import and export. + diff --git a/package.json b/package.json index 7a7094bba6..499c44ab79 100644 --- a/package.json +++ b/package.json @@ -132,14 +132,14 @@ "canvas": "3.2.3", "happy-dom": "20.4.0", "jsdom": "27.3.0", - "vue": "3.5.25", - "@vue/compiler-core": "3.5.25", - "@vue/compiler-dom": "3.5.25", - "@vue/compiler-sfc": "3.5.25", - "@vue/runtime-core": "3.5.25", - "@vue/runtime-dom": "3.5.25", - "@vue/server-renderer": "3.5.25", - "@vue/shared": "3.5.25", + "vue": "3.5.32", + "@vue/compiler-core": "3.5.32", + "@vue/compiler-dom": "3.5.32", + "@vue/compiler-sfc": "3.5.32", + "@vue/runtime-core": "3.5.32", + "@vue/runtime-dom": "3.5.32", + "@vue/server-renderer": "3.5.32", + "@vue/shared": "3.5.32", "vite": "npm:rolldown-vite@7.3.1", "superdoc": "workspace:*", "@superdoc-dev/react": "workspace:*", diff --git a/packages/layout-engine/contracts/src/column-layout.ts b/packages/layout-engine/contracts/src/column-layout.ts index e63e4e95d2..7dc794c592 100644 --- a/packages/layout-engine/contracts/src/column-layout.ts +++ b/packages/layout-engine/contracts/src/column-layout.ts @@ -19,6 +19,7 @@ export function cloneColumnLayout(columns?: ColumnLayout): ColumnLayout { gap: columns.gap, ...(Array.isArray(columns.widths) ? { widths: [...columns.widths] } : {}), ...(columns.equalWidth !== undefined ? { equalWidth: columns.equalWidth } : {}), + ...(columns.withSeparator !== undefined ? { withSeparator: columns.withSeparator } : {}), } : { count: 1, gap: 0 }; } @@ -62,6 +63,7 @@ export function normalizeColumnLayout( count: 1, gap: 0, width: Math.max(0, contentWidth), + ...(input?.withSeparator !== undefined ? { withSeparator: input.withSeparator } : {}), }; } @@ -70,6 +72,7 @@ export function normalizeColumnLayout( gap, ...(widths.length > 0 ? { widths } : {}), ...(input?.equalWidth !== undefined ? { equalWidth: input.equalWidth } : {}), + ...(input?.withSeparator !== undefined ? { withSeparator: input.withSeparator } : {}), width, }; } diff --git a/packages/layout-engine/contracts/src/engines/tabs.test.ts b/packages/layout-engine/contracts/src/engines/tabs.test.ts index 427d1abc36..e9c8d0b223 100644 --- a/packages/layout-engine/contracts/src/engines/tabs.test.ts +++ b/packages/layout-engine/contracts/src/engines/tabs.test.ts @@ -161,11 +161,49 @@ describe('engines-tabs computeTabStops', () => { expect(stops.find((stop) => stop.pos === 340)).toBeDefined(); // Explicit stop at 709 should be preserved expect(stops.find((stop) => stop.pos === 709)).toBeDefined(); - // First default should be at 709 + 720 = 1429 + // First default should align with Word's 0.5" grid offset from leftIndent (709 + 720 = 1429). expect(stops.find((stop) => stop.pos === 1429)).toBeDefined(); - // No default at 720 (before leftIndent, and no explicit stop there) + // No duplicate default at 720 because explicit stop at 709 occupies that slot expect(stops.filter((stop) => stop.pos === 720).length).toBe(0); }); + + it('still generates default start tabs before explicit right tabs (TOC regression)', () => { + const stops = computeTabStops({ + explicitStops: [{ val: 'end', pos: 10593, leader: 'dot' }], // TOC1 style tab + defaultTabInterval: 720, + paragraphIndent: { left: 454, hanging: 454 }, // first line begins near 0" + }); + + const firstDefault = stops.find((stop) => stop.val === 'start' && stop.leader === 'none'); + expect(firstDefault).toBeDefined(); + expect(firstDefault?.pos).toBe(720); // Word default 0.5" tab stop + expect(firstDefault!.pos).toBeLessThan(10593); + expect(stops.find((stop) => stop.val === 'end' && stop.pos === 10593)).toBeDefined(); + }); + + it('preserves legacy defaults-after-rightmost behavior when a start stop is present', () => { + // Paragraphs with a start-aligned explicit stop (e.g. signature lines, invoice + // headers) must keep the pre-fix behavior: defaults begin after the rightmost + // explicit stop, not from zero. Regression guard for the hasStartAlignedExplicit + // branch added alongside the TOC fix. + const explicitStops = [ + { val: 'start' as const, pos: 500, leader: 'none' as const }, + { val: 'end' as const, pos: 5000, leader: 'dot' as const }, + ]; + const stops = computeTabStops({ + explicitStops, + defaultTabInterval: 720, + paragraphIndent: { left: 0 }, + }); + + const explicitPositions = new Set(explicitStops.map((s) => s.pos)); + // No *default* (non-explicit) stop should appear between 0 and the rightmost + // explicit stop (5000). Explicit stops themselves are allowed. + const generatedBelowEnd = stops.filter((stop) => stop.pos < 5000 && !explicitPositions.has(stop.pos)); + expect(generatedBelowEnd).toHaveLength(0); + // Defaults should resume at 5720 (5000 + 720 interval). + expect(stops.find((stop) => stop.val === 'start' && stop.pos === 5720)).toBeDefined(); + }); }); describe('engines-tabs layoutWithTabs', () => { diff --git a/packages/layout-engine/contracts/src/engines/tabs.ts b/packages/layout-engine/contracts/src/engines/tabs.ts index 9417ff5cb3..0a2ac0d884 100644 --- a/packages/layout-engine/contracts/src/engines/tabs.ts +++ b/packages/layout-engine/contracts/src/engines/tabs.ts @@ -129,18 +129,18 @@ export function computeTabStops(context: TabContext): TabStop[] { // Find the rightmost explicit stop (use original stops for this calculation) const maxExplicit = filteredExplicitStops.reduce((max, stop) => Math.max(max, stop.pos), 0); - const hasExplicit = filteredExplicitStops.length > 0; - // Collect all stops: start with filtered explicit stops const stops = [...filteredExplicitStops]; + const hasStartAlignedExplicit = filteredExplicitStops.some((stop) => stop.val === 'start'); // Generate default stops at regular intervals. - // When explicit stops exist, start after the rightmost explicit or leftIndent. - // When no explicit stops, generate from 0 to ensure we hit multiples that land at/near leftIndent. - // Then filter defaults by leftIndent (body text alignment). - const defaultStart = hasExplicit ? Math.max(maxExplicit, leftIndent) : 0; + // - When no explicit start tabs exist (e.g., TOC paragraphs with only right-aligned tabs), + // seed defaults from the origin so numbering/content still lands on the default grid. + // - Otherwise, preserve legacy behavior: defaults start after the rightmost explicit or left indent. + const seedDefaultsFromZero = !hasStartAlignedExplicit; + const defaultStart = seedDefaultsFromZero ? 0 : Math.max(maxExplicit, leftIndent); let pos = defaultStart; - const targetLimit = Math.max(defaultStart, leftIndent) + 14400; // 14400 twips = 10 inches + const targetLimit = Math.max(defaultStart, leftIndent, maxExplicit) + 14400; // 14400 twips = 10 inches while (pos < targetLimit) { pos += defaultTabInterval; diff --git a/packages/layout-engine/contracts/src/index.ts b/packages/layout-engine/contracts/src/index.ts index 69ca4dc0cc..4cc339e09c 100644 --- a/packages/layout-engine/contracts/src/index.ts +++ b/packages/layout-engine/contracts/src/index.ts @@ -987,10 +987,7 @@ export type SectionBreakBlock = { even?: string; odd?: string; }; - columns?: { - count: number; - gap: number; - widths?: number[]; + columns?: ColumnLayout & { equalWidth?: boolean; }; /** @@ -1478,10 +1475,28 @@ export type FlowBlock = export type ColumnLayout = { count: number; gap: number; + withSeparator?: boolean; widths?: number[]; equalWidth?: boolean; }; +/** + * A vertical region of a page that shares a single column configuration. + * + * Continuous section breaks can introduce multiple column configurations on the + * same page (see ECMA-376 §17.6.22 and §17.18.77). A page may therefore carry + * multiple regions stacked vertically. Consumers (e.g. DomPainter) use + * `yStart`/`yEnd` to bound any per-region overlays such as column separators. + */ +export type ColumnRegion = { + /** Inclusive top of the region, in pixels from the page top. */ + yStart: number; + /** Exclusive bottom of the region, in pixels from the page top. */ + yEnd: number; + /** Column configuration active within this region. */ + columns: ColumnLayout; +}; + /** A measured line within a block, output by the measurer. */ export type Line = { fromRun: number; @@ -1706,6 +1721,29 @@ export type Page = { * Sections are 0-indexed, matching the sectionIndex in SectionMetadata. */ sectionIndex?: number; + /** + * Column layout configuration for this page. + * + * Reflects the column configuration at page start. For pages with continuous + * section breaks that change column layout mid-page, use `columnRegions` for + * accurate per-region information. + * + * Used by the renderer to draw column separator lines when `withSeparator` + * is set to true. + */ + columns?: ColumnLayout; + /** + * Vertical column regions on this page, ordered top to bottom. + * + * Populated when continuous section breaks change column layout mid-page. Each + * region pairs a `{yStart, yEnd}` span with the column config active inside it + * (see ECMA-376 §17.6.22). Renderers should prefer this field over + * `columns` when drawing per-region overlays (e.g. column separators). + * + * If omitted, the page has a single column region and consumers can fall back + * to `columns`. + */ + columnRegions?: ColumnRegion[]; }; /** A paragraph fragment positioned on a page. */ diff --git a/packages/layout-engine/layout-bridge/src/incrementalLayout.ts b/packages/layout-engine/layout-bridge/src/incrementalLayout.ts index 4f82c339fc..79a4423c87 100644 --- a/packages/layout-engine/layout-bridge/src/incrementalLayout.ts +++ b/packages/layout-engine/layout-bridge/src/incrementalLayout.ts @@ -19,6 +19,7 @@ import { resolvePageNumberTokens, type NumberingContext, SEMANTIC_PAGE_HEIGHT_PX, + SINGLE_COLUMN_DEFAULT, } from '@superdoc/layout-engine'; import { remeasureParagraph } from './remeasure'; import { computeDirtyRegions } from './diff'; @@ -183,7 +184,7 @@ const resolvePageColumns = (layout: Layout, options: LayoutOptions, blocks?: Flo ); const contentWidth = pageSize.w - (marginLeft + marginRight); const sectionIndex = page.sectionIndex ?? 0; - const columnsConfig = sectionColumns.get(sectionIndex) ?? options.columns ?? { count: 1, gap: 0 }; + const columnsConfig = sectionColumns.get(sectionIndex) ?? options.columns ?? SINGLE_COLUMN_DEFAULT; const normalized = normalizeColumnsForFootnotes(columnsConfig, contentWidth); result.set(pageIndex, { ...normalized, left: marginLeft, contentWidth }); } @@ -1328,6 +1329,34 @@ export async function incrementalLayout( const columns = pageColumns.get(pageIndex); const columnCount = Math.max(1, Math.floor(columns?.count ?? 1)); + // SD-1680: cap placement to the footnote demand on this page (capped by maxReserve). + // Demand = sum of measured heights of all footnote refs anchored here, plus the + // separator/padding/gap overhead they would incur when stacked. Capping placement + // at `min(demand, maxReserve)` (rather than `baseReserve`) decouples the plan's + // placement from the body's prior-pass reserve: the plan reports how much band + // the footnotes actually need, the body grows its reserve to match on the next + // pass, and placement never exceeds maxReserve so footnotes cannot render past + // the page's bottom margin. + let demand = 0; + for (let columnIndex = 0; columnIndex < columnCount; columnIndex += 1) { + const ids = idsByColumn.get(pageIndex)?.get(columnIndex) ?? []; + let columnDemand = 0; + ids.forEach((id, idx) => { + const ranges = rangesByFootnoteId.get(id) ?? []; + let rangesHeight = 0; + ranges.forEach((range) => { + const spacingAfter = 'spacingAfter' in range ? (range.spacingAfter ?? 0) : 0; + rangesHeight += range.height + spacingAfter; + }); + columnDemand += rangesHeight + (idx > 0 ? safeGap : 0); + }); + if (columnDemand > 0) { + columnDemand += safeSeparatorSpacingBefore + safeDividerHeight + safeTopPadding; + } + if (columnDemand > demand) demand = columnDemand; + } + const placementCeiling = demand > 0 ? Math.min(Math.ceil(demand), maxReserve) : maxReserve; + const pendingForPage = new Map>(); pendingByColumn.forEach((entries, columnIndex) => { const targetIndex = columnIndex < columnCount ? columnIndex : Math.max(0, columnCount - 1); @@ -1365,7 +1394,7 @@ export async function incrementalLayout( : 0; const overhead = isFirstSlice ? separatorBefore + separatorHeight + safeTopPadding : 0; const gapBefore = !isFirstSlice ? safeGap : 0; - const availableHeight = Math.max(0, maxReserve - usedHeight - overhead - gapBefore); + const availableHeight = Math.max(0, placementCeiling - usedHeight - overhead - gapBefore); const { slice, remainingRanges } = fitFootnoteContent( id, ranges, @@ -1374,7 +1403,7 @@ export async function incrementalLayout( columnIndex, isContinuation, measuresById, - isFirstSlice && maxReserve > 0, + isFirstSlice && placementCeiling > 0, ); if (slice.ranges.length === 0) { @@ -1503,7 +1532,7 @@ export async function incrementalLayout( ); const pageContentWidth = pageSize.w - (marginLeft + marginRight); const fallbackColumns = normalizeColumnsForFootnotes( - options.columns ?? { count: 1, gap: 0 }, + options.columns ?? SINGLE_COLUMN_DEFAULT, pageContentWidth, ); const columns = pageColumns.get(pageIndex) ?? { @@ -1756,7 +1785,7 @@ export async function incrementalLayout( // so each page gets the correct reserve (avoids "too much" on one page and "not enough" on another). if (reserves.some((h) => h > 0)) { let reservesStabilized = false; - const seenReserveKeys = new Set([reserves.join(',')]); + const seenReserveVectors: number[][] = [reserves.slice()]; for (let pass = 0; pass < MAX_FOOTNOTE_LAYOUT_PASSES; pass += 1) { layout = relayout(reserves); ({ columns: pageColumns, idsByColumn } = resolveFootnoteAssignments(layout)); @@ -1772,13 +1801,33 @@ export async function incrementalLayout( reservesStabilized = true; break; } - // Detect oscillation: if we've produced a reserve vector we already tried, - // the loop will never converge. Break early to avoid wasted relayout passes. + // SD-1680: when reserves oscillate (typically between a state where all footnotes + // fit and a state where body packs tighter with some footnotes pushed off the + // page), prefer the element-wise max across all seen states. This matches Word's + // bias toward keeping footnotes on their ref's page rather than tight body + // packing, and avoids overflow from the body reserving less than the plan places. const nextKey = nextReserves.join(','); - if (seenReserveKeys.has(nextKey)) { + const seen = seenReserveVectors.some((v) => v.join(',') === nextKey); + if (seen) { + const allVectors = [...seenReserveVectors, nextReserves]; + const mergedLength = Math.max(...allVectors.map((v) => v.length)); + const merged = new Array(mergedLength).fill(0); + for (const vec of allVectors) { + for (let i = 0; i < mergedLength; i += 1) { + if ((vec[i] ?? 0) > merged[i]) merged[i] = vec[i]; + } + } + reserves = merged; + // Relayout with merged reserves so post-loop sees a layout consistent with the + // reserves we're about to apply — otherwise pages may collapse to the layout + // built with the smaller oscillating reserve. + layout = relayout(reserves); + ({ columns: pageColumns, idsByColumn } = resolveFootnoteAssignments(layout)); + ({ measuresById } = await measureFootnoteBlocks(collectFootnoteIdsByColumn(idsByColumn))); + plan = computeFootnoteLayoutPlan(layout, idsByColumn, measuresById, reserves, pageColumns); break; } - seenReserveKeys.add(nextKey); + seenReserveVectors.push(nextReserves.slice()); // Only update reserves when we will do another layout pass; otherwise layout // would be built with the previous reserves while reserves would be nextReserves, // and the plan/injection phase could place footnotes in the wrong band. @@ -1803,15 +1852,31 @@ export async function incrementalLayout( reserves, finalPageColumns, ); - const finalReserves = finalPlan.reserves; let reservesAppliedToLayout = reserves; - const reservesDiffer = - finalReserves.length !== reserves.length || - finalReserves.some((h, i) => (reserves[i] ?? 0) !== h) || - reserves.some((h, i) => (finalReserves[i] ?? 0) !== h); - if (reservesDiffer) { - layout = relayout(finalReserves); - reservesAppliedToLayout = finalReserves; + // SD-1680: the post-loop can still mismatch the body reserve and plan placement when + // relayouting with finalPlan.reserves shifts footnote refs between pages (the newly + // relaxed page now holds refs the old reserves didn't account for). Iterate a few + // times, each step taking the element-wise max of current reserves and the new plan's + // reserves, so the final layout's reservation on every page is at least as large as + // the demand from the final ref assignment. This guarantees placements stay inside + // the band and cannot render past the page's bottom margin. + const MAX_POST_PASSES = 3; + for (let postPass = 0; postPass < MAX_POST_PASSES; postPass += 1) { + const target = reservesAppliedToLayout.slice(); + const planReserves = finalPlan.reserves; + const len = Math.max(target.length, planReserves.length); + let needsRelayout = false; + for (let i = 0; i < len; i += 1) { + const applied = target[i] ?? 0; + const needed = planReserves[i] ?? 0; + if (needed > applied) { + target[i] = needed; + needsRelayout = true; + } + } + if (!needsRelayout) break; + layout = relayout(target); + reservesAppliedToLayout = target; ({ columns: finalPageColumns, idsByColumn: finalIdsByColumn } = resolveFootnoteAssignments(layout)); ({ blocks: finalBlocks, measuresById: finalMeasuresById } = await measureFootnoteBlocks( collectFootnoteIdsByColumn(finalIdsByColumn), diff --git a/packages/layout-engine/layout-bridge/src/remeasure.ts b/packages/layout-engine/layout-bridge/src/remeasure.ts index 5e7ae01e43..0462bbecd5 100644 --- a/packages/layout-engine/layout-bridge/src/remeasure.ts +++ b/packages/layout-engine/layout-bridge/src/remeasure.ts @@ -5,6 +5,7 @@ import type { LineSegment, Run, TextRun, + TabRun, TabStop, ParagraphIndent, LeaderDecoration, @@ -781,6 +782,33 @@ const applyTabLayoutToLines = ( indentLeft: number, rawFirstLineOffset: number, ): void => { + const totalTabRuns = runs.reduce((count, run) => (run.kind === 'tab' ? count + 1 : count), 0); + const alignmentTabStopsPx = tabStops + .map((stop, index) => ({ stop, index })) + .filter(({ stop }) => stop.val === 'end' || stop.val === 'center' || stop.val === 'decimal'); + // Word-compat heuristic (not ECMA-376 17.3.3.32): the last N tab characters in a + // paragraph bind to the last N explicit end/center/decimal stops. Needed for TOC + // entries where a right-aligned dot-leader stop coexists with default grid stops. + // Mirrored in measuring/dom/src/index.ts. + const getAlignmentStopForOrdinal = (ordinal: number): { stop: TabStopPx; index: number } | null => { + if (alignmentTabStopsPx.length === 0 || totalTabRuns === 0 || !Number.isFinite(ordinal)) return null; + if (ordinal < 0 || ordinal >= totalTabRuns) return null; + const remainingTabs = totalTabRuns - ordinal - 1; + const targetIndex = alignmentTabStopsPx.length - 1 - remainingTabs; + if (targetIndex < 0 || targetIndex >= alignmentTabStopsPx.length) return null; + return alignmentTabStopsPx[targetIndex]; + }; + let sequentialTabIndex = 0; + const consumeTabOrdinal = (explicitIndex?: number): number => { + if (typeof explicitIndex === 'number' && Number.isFinite(explicitIndex)) { + sequentialTabIndex = Math.max(sequentialTabIndex, explicitIndex + 1); + return explicitIndex; + } + const ordinal = sequentialTabIndex; + sequentialTabIndex += 1; + return ordinal; + }; + lines.forEach((line, lineIndex) => { let cursorX = 0; let lineWidth = 0; @@ -797,11 +825,23 @@ const applyTabLayoutToLines = ( /** * Processes a tab character, calculating position and handling alignment. */ - const applyTab = (startRunIndex: number, startChar: number, run?: Run): void => { + const applyTab = (startRunIndex: number, startChar: number, run?: Run, tabOrdinal?: number): void => { const originX = cursorX; const absCurrentX = cursorX + effectiveIndent; - const { target, nextIndex, stop } = getNextTabStopPx(absCurrentX, tabStops, tabStopCursor); - tabStopCursor = nextIndex; + let stop: TabStopPx | undefined; + let target: number; + const forcedAlignment = + typeof tabOrdinal === 'number' && Number.isFinite(tabOrdinal) ? getAlignmentStopForOrdinal(tabOrdinal) : null; + if (forcedAlignment && forcedAlignment.stop.pos > absCurrentX + TAB_EPSILON) { + stop = forcedAlignment.stop; + target = forcedAlignment.stop.pos; + tabStopCursor = forcedAlignment.index + 1; + } else { + const next = getNextTabStopPx(absCurrentX, tabStops, tabStopCursor); + stop = next.stop; + target = next.target; + tabStopCursor = next.nextIndex; + } const clampedTarget = Number.isFinite(maxAbsWidth) ? Math.min(target, maxAbsWidth) : target; const relativeTarget = clampedTarget - effectiveIndent; lineWidth = Math.max(lineWidth, relativeTarget); @@ -853,7 +893,9 @@ const applyTabLayoutToLines = ( const run = runs[runIndex]; if (!run) continue; if (run.kind === 'tab') { - applyTab(runIndex + 1, 0, run); + const tabRun = run as TabRun; + const ordinal = consumeTabOrdinal(tabRun.tabIndex); + applyTab(runIndex + 1, 0, run, ordinal); continue; } @@ -889,7 +931,8 @@ const applyTabLayoutToLines = ( lineWidth = Math.max(lineWidth, cursorX); segments.push(segment); } - applyTab(runIndex, i + 1); + const ordinal = consumeTabOrdinal(); + applyTab(runIndex, i + 1, undefined, ordinal); segmentStart = i + 1; } diff --git a/packages/layout-engine/layout-bridge/test/footnoteBandOverflow.test.ts b/packages/layout-engine/layout-bridge/test/footnoteBandOverflow.test.ts new file mode 100644 index 0000000000..d70db31293 --- /dev/null +++ b/packages/layout-engine/layout-bridge/test/footnoteBandOverflow.test.ts @@ -0,0 +1,224 @@ +/** + * SD-1680: Footnotes render past the bottom page margin when multiple footnotes + * need more total height than the reserved band allows. Verifies that the plan + * output is internally consistent with the body layout's bottom margin — i.e., + * no footnote fragment's bottom-Y exceeds the top of the physical bottom margin. + */ + +import { describe, it, expect, vi } from 'vitest'; +import type { FlowBlock, Measure, Fragment } from '@superdoc/contracts'; +import { incrementalLayout } from '../src/incrementalLayout'; + +const makeParagraph = (id: string, text: string, pmStart: number): FlowBlock => ({ + kind: 'paragraph', + id, + runs: [{ text, fontFamily: 'Arial', fontSize: 12, pmStart, pmEnd: pmStart + text.length }], +}); + +const makeSingleLineMeasure = (lineHeight: number, textLength: number): Measure => ({ + kind: 'paragraph', + lines: [ + { + fromRun: 0, + fromChar: 0, + toRun: 0, + toChar: textLength, + width: 200, + ascent: lineHeight * 0.8, + descent: lineHeight * 0.2, + lineHeight, + }, + ], + totalHeight: lineHeight, +}); + +const makeMultiLineMeasure = (lineHeight: number, lineCount: number): Measure => { + const lines = Array.from({ length: lineCount }, (_, i) => ({ + fromRun: 0, + fromChar: i, + toRun: 0, + toChar: i + 1, + width: 200, + ascent: lineHeight * 0.8, + descent: lineHeight * 0.2, + lineHeight, + })); + return { kind: 'paragraph', lines, totalHeight: lineCount * lineHeight }; +}; + +const PAGE_PHYSICAL_BOTTOM_MARGIN = 72; +const BODY_LINE_HEIGHT = 20; +const FOOTNOTE_LINE_HEIGHT = 12; + +describe('SD-1680: Footnote band must not overflow the reserved area', () => { + /** + * Scenario: multiple medium-size footnotes compete for a single page's band. + * Page body fills to have all 4 refs on page 1. Each footnote is long enough that + * the total needed reserve exceeds what a single pass' body layout anticipated. + * Before the fix, footnotes stacked past the reserved band and rendered past the + * page's physical bottom margin. After the fix, excess footnotes must be + * pushed to a later page (or split) and the band must stay within its reserve. + */ + it('keeps every footnote fragment within the band on multi-footnote pages', async () => { + // Realistic-ish scenario: ~20 body paragraphs across 2-3 pages, with 2 footnote + // refs per page. Each footnote is medium-sized (4 lines). Ample body slack so + // the layout converges within MAX_FOOTNOTE_LAYOUT_PASSES. + const BODY_PARAS = 20; + const FOOTNOTE_LINES = 4; + const FOOTNOTE_COUNT = 4; + + let pos = 0; + const bodyBlocks: FlowBlock[] = []; + const refs: Array<{ id: string; pos: number }> = []; + const blocksById = new Map(); + + for (let i = 0; i < BODY_PARAS; i += 1) { + const text = `Body paragraph ${i + 1}.`; + const para = makeParagraph(`body-${i}`, text, pos); + bodyBlocks.push(para); + + // Spread refs so 2 land on each page + if (refs.length < FOOTNOTE_COUNT && i % 3 === 1) { + const refId = String(refs.length + 1); + refs.push({ id: refId, pos: pos + 2 }); + const fnBlock = makeParagraph( + `footnote-${refId}-0-paragraph`, + `Footnote ${refId} content spanning multiple lines.`, + 0, + ); + blocksById.set(refId, [fnBlock]); + } + + pos += text.length + 1; + } + + const measureBlock = vi.fn(async (block: FlowBlock) => { + if (block.id.startsWith('footnote-')) { + return makeMultiLineMeasure(FOOTNOTE_LINE_HEIGHT, FOOTNOTE_LINES); + } + return makeSingleLineMeasure(BODY_LINE_HEIGHT, 20); + }); + + // Page holds ~10 body paragraphs (200px). With footnote reserve, body shrinks + // enough that refs can move between pages, but everything must stay bounded. + const contentHeight = 10 * BODY_LINE_HEIGHT; // 200px + const pageHeight = contentHeight + 72 + PAGE_PHYSICAL_BOTTOM_MARGIN; + + const result = await incrementalLayout( + [], + null, + bodyBlocks, + { + pageSize: { w: 612, h: pageHeight }, + margins: { top: 72, right: 72, bottom: PAGE_PHYSICAL_BOTTOM_MARGIN, left: 72 }, + footnotes: { + refs, + blocksById, + topPadding: 4, + dividerHeight: 2, + }, + }, + measureBlock, + ); + + const { layout } = result; + const pageH = pageHeight; + + // INVARIANT: on every page, every footnote fragment's bottom edge must be ≤ the + // top of the physical bottom margin (pageH - PAGE_PHYSICAL_BOTTOM_MARGIN). + // Anything below that point has rendered past the reserved band into the + // footer area — the core SD-1680 bug. + const overflows: string[] = []; + for (const page of layout.pages) { + const pageBottomLimit = (page.size?.h ?? pageH) - PAGE_PHYSICAL_BOTTOM_MARGIN; + const footnoteFragments = page.fragments.filter( + (f: Fragment) => + typeof f.blockId === 'string' && f.blockId.startsWith('footnote-') && !f.blockId.includes('separator'), + ); + for (const f of footnoteFragments) { + // For paragraph fragments, height is computed from fromLine/toLine * lineHeight. + // The footnote measure's totalHeight is FOOTNOTE_LINES * FOOTNOTE_LINE_HEIGHT. + let fragHeight = 0; + if (f.kind === 'para' && typeof f.toLine === 'number' && typeof f.fromLine === 'number') { + fragHeight = (f.toLine - f.fromLine) * FOOTNOTE_LINE_HEIGHT; + } else if (typeof (f as { height?: number }).height === 'number') { + fragHeight = (f as { height: number }).height; + } + const fragBottom = (f.y ?? 0) + fragHeight; + if (fragBottom > pageBottomLimit + 1) { + overflows.push( + `page ${page.number}: fragment ${f.blockId} bottom=${fragBottom.toFixed(1)} exceeds limit ${pageBottomLimit.toFixed(1)} by ${(fragBottom - pageBottomLimit).toFixed(1)}px`, + ); + } + } + } + + expect(overflows).toEqual([]); + }); + + /** + * A single huge footnote (taller than the page's max reserve) must be split + * across pages with a continuation on the next page — never rendered as one + * monolithic block that extends off the page. + */ + it('splits an oversized footnote across pages rather than overflowing', async () => { + const BIG_FOOTNOTE_LINES = 30; // way bigger than a single page's band + const refPos = 5; + + const bodyBlocks: FlowBlock[] = [makeParagraph('body-0', 'Body before footnote ref.', 0)]; + const footnoteBlock = makeParagraph( + 'footnote-1-0-paragraph', + 'A very long footnote that should not fit on a single page band.', + 0, + ); + + const measureBlock = vi.fn(async (block: FlowBlock) => { + if (block.id.startsWith('footnote-')) { + return makeMultiLineMeasure(FOOTNOTE_LINE_HEIGHT, BIG_FOOTNOTE_LINES); + } + return makeSingleLineMeasure(BODY_LINE_HEIGHT, 26); + }); + + const contentHeight = 200; + const pageHeight = contentHeight + 72 + PAGE_PHYSICAL_BOTTOM_MARGIN; + + const result = await incrementalLayout( + [], + null, + bodyBlocks, + { + pageSize: { w: 612, h: pageHeight }, + margins: { top: 72, right: 72, bottom: PAGE_PHYSICAL_BOTTOM_MARGIN, left: 72 }, + footnotes: { + refs: [{ id: '1', pos: refPos }], + blocksById: new Map([['1', [footnoteBlock]]]), + topPadding: 4, + dividerHeight: 2, + }, + }, + measureBlock, + ); + + const { layout } = result; + const pageBottomLimit = pageHeight - PAGE_PHYSICAL_BOTTOM_MARGIN; + + // No single fragment may extend past the page's bottom limit. + for (const page of layout.pages) { + const pageBottomThisPage = (page.size?.h ?? pageHeight) - PAGE_PHYSICAL_BOTTOM_MARGIN; + const footnoteFragments = page.fragments.filter( + (f: Fragment) => + typeof f.blockId === 'string' && f.blockId.startsWith('footnote-') && !f.blockId.includes('separator'), + ); + for (const f of footnoteFragments) { + let fragHeight = 0; + if (f.kind === 'para' && typeof f.toLine === 'number' && typeof f.fromLine === 'number') { + fragHeight = (f.toLine - f.fromLine) * FOOTNOTE_LINE_HEIGHT; + } else if (typeof (f as { height?: number }).height === 'number') { + fragHeight = (f as { height: number }).height; + } + const fragBottom = (f.y ?? 0) + fragHeight; + expect(fragBottom).toBeLessThanOrEqual(pageBottomThisPage + 1); + } + } + }); +}); diff --git a/packages/layout-engine/layout-bridge/test/remeasure.test.ts b/packages/layout-engine/layout-bridge/test/remeasure.test.ts index f7e757e55a..66d5a2ee1a 100644 --- a/packages/layout-engine/layout-bridge/test/remeasure.test.ts +++ b/packages/layout-engine/layout-bridge/test/remeasure.test.ts @@ -445,6 +445,27 @@ describe('remeasureParagraph', () => { expect(measure.lines[0].segments?.length).toBeGreaterThan(0); }); + it('aligns trailing TOC-style tab to explicit right stop with leader', () => { + const rightStopPx = 300; + const block = createBlock( + [textRun('1.'), tabRun({ tabIndex: 0 }), textRun('Generalities'), tabRun({ tabIndex: 1 }), textRun('5')], + { + tabs: [{ pos: pxToTwips(rightStopPx), val: 'end', leader: 'dot' }], + indent: { left: 30, hanging: 30 }, + tabIntervalTwips: DEFAULT_TAB_INTERVAL_TWIPS, + }, + ); + + const measure = remeasureParagraph(block, 800); + expect(measure.lines).toHaveLength(1); + const leaders = measure.lines[0].leaders; + expect(leaders).toBeDefined(); + expect(leaders?.length).toBe(1); + const leader = leaders![0]; + expect(leader.style).toBe('dot'); + expect(leader.to).toBeCloseTo(rightStopPx - CHAR_WIDTH, 0); + }); + it('handles tab at various positions within text', () => { // Tab after some text should advance to next stop after current position const tabStop: TabStop = { pos: 720, val: 'start' }; // 48px @@ -481,7 +502,6 @@ describe('remeasureParagraph', () => { const tabStop: TabStop = { pos: 1440, val: 'start' }; const block = createBlock([textRun('A'), tabRun(), textRun('B')], { tabs: [tabStop] }); const measure = remeasureParagraph(block, 200); - expect(measure.lines).toHaveLength(1); // Tab should advance to 96px (1 inch) expect(measure.lines[0].width).toBeGreaterThan(96); diff --git a/packages/layout-engine/layout-engine/src/index.test.ts b/packages/layout-engine/layout-engine/src/index.test.ts index ab883680e2..84ff86b583 100644 --- a/packages/layout-engine/layout-engine/src/index.test.ts +++ b/packages/layout-engine/layout-engine/src/index.test.ts @@ -228,6 +228,125 @@ describe('layoutDocument', () => { expect(layout.columns).toMatchObject({ count: 2, gap: 20 }); }); + it('sets "page.columns" with separator when column separator is enabled', () => { + const options: LayoutOptions = { + pageSize: { w: 600, h: 800 }, + margins: { top: 40, right: 40, bottom: 40, left: 40 }, + columns: { count: 2, gap: 20, withSeparator: true }, + }; + const layout = layoutDocument([block], [makeMeasure([350, 350, 350])], options); + + expect(layout.pages).toHaveLength(1); + expect(layout.pages[0].columns).toEqual({ count: 2, gap: 20, withSeparator: true }); + expect(layout.columns).toMatchObject({ count: 2, gap: 20, withSeparator: true }); + }); + + it('preserves explicit column widths on page-level column metadata', () => { + const options: LayoutOptions = { + pageSize: { w: 600, h: 800 }, + margins: { top: 40, right: 40, bottom: 40, left: 40 }, + columns: { count: 2, gap: 20, widths: [100, 400], equalWidth: false, withSeparator: true }, + }; + const layout = layoutDocument([block], [makeMeasure([350, 350, 350])], options); + + expect(layout.pages).toHaveLength(1); + expect(layout.pages[0].columns).toEqual({ + count: 2, + gap: 20, + widths: [100, 400], + equalWidth: false, + withSeparator: true, + }); + expect(layout.columns).toEqual({ + count: 2, + gap: 20, + widths: [100, 400], + equalWidth: false, + withSeparator: true, + }); + }); + + it('does not set "page.columns" on single column layout', () => { + const options: LayoutOptions = { + pageSize: { w: 600, h: 800 }, + margins: { top: 40, right: 40, bottom: 40, left: 40 }, + }; + const layout = layoutDocument([block], [makeMeasure([350])], options); + + expect(layout.pages).toHaveLength(1); + expect(layout.pages[0].columns).toBeUndefined(); + expect(layout.columns).toBeUndefined(); + }); + + it('sets "page.columns" without separator when column separator is not enabled', () => { + const options: LayoutOptions = { + pageSize: { w: 600, h: 800 }, + margins: { top: 40, right: 40, bottom: 40, left: 40 }, + columns: { count: 2, gap: 20, withSeparator: false }, + }; + const layout = layoutDocument([block], [makeMeasure([350, 350, 350])], options); + + expect(layout.pages).toHaveLength(1); + expect(layout.pages[0].columns).toEqual({ count: 2, gap: 20, withSeparator: false }); + expect(layout.columns).toEqual({ count: 2, gap: 20, withSeparator: false }); + }); + + it('emits page.columnRegions for continuous section breaks that change column config mid-page', () => { + // Two sections on the same page: first 2-col with separator, then a + // continuous break that switches to 3-col still with separator. The + // layout engine should record a ConstraintBoundary and surface it on + // page.columnRegions so the renderer can bound each separator to the + // correct Y range. + const blocks: FlowBlock[] = [ + { kind: 'paragraph', id: 'intro', runs: [] }, + { + kind: 'sectionBreak', + id: 'sb-continuous', + type: 'continuous', + columns: { count: 3, gap: 20, withSeparator: true }, + }, + { kind: 'paragraph', id: 'body', runs: [] }, + ]; + const measures: Measure[] = [makeMeasure([30]), { kind: 'sectionBreak' }, makeMeasure([30, 30, 30])]; + + const options: LayoutOptions = { + pageSize: { w: 600, h: 800 }, + margins: { top: 40, right: 40, bottom: 40, left: 40 }, + columns: { count: 2, gap: 20, withSeparator: true }, + }; + + const layout = layoutDocument(blocks, measures, options); + + expect(layout.pages).toHaveLength(1); + const regions = layout.pages[0].columnRegions; + expect(regions).toBeDefined(); + expect(regions!.length).toBeGreaterThanOrEqual(2); + // First region covers the initial 2-col layout from topMargin to the boundary. + expect(regions![0].yStart).toBe(40); + expect(regions![0].columns).toEqual({ count: 2, gap: 20, withSeparator: true }); + // Second region picks up the continuous break's 3-col config and ends at + // the bottom of the content area. + const last = regions![regions!.length - 1]; + expect(last.columns).toMatchObject({ count: 3, gap: 20, withSeparator: true }); + expect(last.yEnd).toBe(800 - 40); + // Regions must tile (no gaps, no overlap). + for (let i = 1; i < regions!.length; i++) { + expect(regions![i].yStart).toBe(regions![i - 1].yEnd); + } + }); + + it('omits page.columnRegions when no mid-page column change occurs', () => { + const options: LayoutOptions = { + pageSize: { w: 600, h: 800 }, + margins: { top: 40, right: 40, bottom: 40, left: 40 }, + columns: { count: 2, gap: 20, withSeparator: true }, + }; + const layout = layoutDocument([block], [makeMeasure([350, 350, 350])], options); + + expect(layout.pages).toHaveLength(1); + expect(layout.pages[0].columnRegions).toBeUndefined(); + }); + it('applies spacing before and after paragraphs', () => { const spacingBlock: FlowBlock = { kind: 'paragraph', @@ -1919,6 +2038,78 @@ describe('layoutDocument', () => { expect(p3Fragment).toBeDefined(); expect(p3Fragment?.width).toBe(210); // Half width = two columns }); + + it('starts new region below tallest column when columns have unequal heights', () => { + // Regression test for SD-1869: when a multi-column section has unequal column + // heights, the next region must start below the TALLEST column, not the last + // column's cursor. Without the maxCursorY fix, the new region would start at + // the shorter column's bottom, overlapping the taller one. + // + // Uses a 3-col → 2-col transition because the layout engine forces a new page + // when reducing to fewer columns than the current column index (guard at + // columnIndexBefore >= newColumns.count). With 3→2, content in col1 + // (columnIndex=1) stays on the same page (1 < 2). + const toThreeColumns: FlowBlock = { + kind: 'sectionBreak', + id: 'sb-to-3col', + type: 'continuous', + columns: { count: 3, gap: 24 }, + margins: {}, + }; + const toTwoColumns: FlowBlock = { + kind: 'sectionBreak', + id: 'sb-to-2col', + type: 'continuous', + columns: { count: 2, gap: 48 }, + margins: {}, + }; + + const blocks: FlowBlock[] = [ + { kind: 'paragraph', id: 'p1', runs: [] }, // single column preamble + toThreeColumns, + { kind: 'paragraph', id: 'p-cols', runs: [] }, // 3 lines → col0 gets 2, col1 gets 1 + toTwoColumns, + { kind: 'paragraph', id: 'p-after', runs: [] }, // must start below tallest column + ]; + + // p-cols: 3 lines of 250px each (750px total) + // Available column height = 720 (page bottom) - 112 (region top) = 608px + // Column 0 fits lines 0+1 (500px), line 2 overflows to column 1 + // Column 0 bottom = 112 + 500 = 612 + // Column 1 bottom = 112 + 250 = 362 + const measures: Measure[] = [ + makeMeasure([40]), // p1 + { kind: 'sectionBreak' }, + makeMeasure([250, 250, 250]), // p-cols: 3 lines, 2 in col0 + 1 in col1 + { kind: 'sectionBreak' }, + makeMeasure([40]), // p-after + ]; + + const options: LayoutOptions = { + pageSize: { w: 612, h: 792 }, + margins: { top: 72, right: 72, bottom: 72, left: 72 }, + }; + + const layout = layoutDocument(blocks, measures, options); + + // Everything should fit on one page + expect(layout.pages.length).toBe(1); + + // p1 at y=72, height=40 → region for 3-col section starts at y=112 + const regionTop = 72 + 40; // 112 + + // Column 0: 2 lines × 250px = 500px → bottom at 112 + 500 = 612 + // Column 1: 1 line × 250px = 250px → bottom at 112 + 250 = 362 + const tallestColumnBottom = regionTop + 500; // 612 + + const page = layout.pages[0]; + const pAfter = page.fragments.find((f) => f.blockId === 'p-after'); + expect(pAfter).toBeDefined(); + + // KEY ASSERTION: p-after must start at or below the tallest column's bottom (612) + // Without the fix, it would start at 362 (column 1's bottom), overlapping column 0 + expect(pAfter!.y).toBeGreaterThanOrEqual(tallestColumnBottom); + }); }); describe('columnBreak with multi-column pages', () => { @@ -5421,3 +5612,312 @@ describe('requirePageBoundary edge cases', () => { }); }); }); + +describe('alternateHeaders (odd/even header differentiation)', () => { + // Two tall paragraphs (400px each) that force a 2-page layout. + const tallBlock = (id: string): FlowBlock => ({ + kind: 'paragraph', + id, + runs: [], + }); + const tallMeasure = makeMeasure([400]); + + it('selects even/odd header heights when alternateHeaders is true', () => { + const options: LayoutOptions = { + pageSize: { w: 600, h: 800 }, + margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 }, + alternateHeaders: true, + headerContentHeights: { + odd: 80, // Odd pages: header pushes body start down + even: 40, // Even pages: smaller header + }, + }; + + const layout = layoutDocument([tallBlock('p1'), tallBlock('p2')], [tallMeasure, tallMeasure], options); + + expect(layout.pages).toHaveLength(2); + + // Page 1 is odd (documentPageNumber=1) → uses 'odd' header height (80px) + // Body should start at max(margin.top, margin.header + headerContentHeight) = max(50, 30+80) = 110 + const p1Fragment = layout.pages[0].fragments.find((f) => f.blockId === 'p1'); + expect(p1Fragment).toBeDefined(); + expect(p1Fragment!.y).toBeCloseTo(110, 0); + + // Page 2 is even (documentPageNumber=2) → uses 'even' header height (40px) + // Body should start at max(margin.top, margin.header + headerContentHeight) = max(50, 30+40) = 70 + const p2Fragment = layout.pages[1].fragments.find((f) => f.blockId === 'p2'); + expect(p2Fragment).toBeDefined(); + expect(p2Fragment!.y).toBeCloseTo(70, 0); + }); + + it('uses default header height for all pages when alternateHeaders is false', () => { + const options: LayoutOptions = { + pageSize: { w: 600, h: 800 }, + margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 }, + alternateHeaders: false, + headerContentHeights: { + default: 60, + odd: 80, + even: 40, + }, + }; + + const layout = layoutDocument([tallBlock('p1'), tallBlock('p2')], [tallMeasure, tallMeasure], options); + + expect(layout.pages).toHaveLength(2); + + // Both pages use 'default' header height (60px) + // Body start = max(50, 30+60) = 90 + const p1Fragment = layout.pages[0].fragments.find((f) => f.blockId === 'p1'); + const p2Fragment = layout.pages[1].fragments.find((f) => f.blockId === 'p2'); + expect(p1Fragment!.y).toBeCloseTo(90, 0); + expect(p2Fragment!.y).toBeCloseTo(90, 0); + }); + + it('defaults to false when alternateHeaders is omitted', () => { + const options: LayoutOptions = { + pageSize: { w: 600, h: 800 }, + margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 }, + // alternateHeaders not set + headerContentHeights: { + default: 60, + odd: 80, + even: 40, + }, + }; + + const layout = layoutDocument([tallBlock('p1'), tallBlock('p2')], [tallMeasure, tallMeasure], options); + + expect(layout.pages).toHaveLength(2); + + // Both pages should use 'default' (60px), not odd/even + const p1Fragment = layout.pages[0].fragments.find((f) => f.blockId === 'p1'); + const p2Fragment = layout.pages[1].fragments.find((f) => f.blockId === 'p2'); + expect(p1Fragment!.y).toBeCloseTo(90, 0); + expect(p2Fragment!.y).toBeCloseTo(90, 0); + }); + + it('first page uses first variant when titlePg is enabled with alternateHeaders', () => { + const sectionBreak: SectionBreakBlock = { + kind: 'sectionBreak', + id: 'sb', + attrs: { isFirstSection: true, source: 'sectPr', sectionIndex: 0 }, + pageSize: { w: 600, h: 800 }, + margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 }, + }; + + const options: LayoutOptions = { + pageSize: { w: 600, h: 800 }, + margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 }, + alternateHeaders: true, + sectionMetadata: [{ sectionIndex: 0, titlePg: true }], + headerContentHeights: { + first: 100, // First page: tallest header + odd: 80, + even: 40, + }, + }; + + const layout = layoutDocument( + [sectionBreak, tallBlock('p1'), tallBlock('p2'), tallBlock('p3')], + [{ kind: 'sectionBreak' }, tallMeasure, tallMeasure, tallMeasure], + options, + ); + + expect(layout.pages.length).toBeGreaterThanOrEqual(3); + + // Page 1 (first page of section, titlePg=true) → 'first' variant → 100px + // Body start = max(50, 30+100) = 130 + const p1Fragment = layout.pages[0].fragments.find((f) => f.blockId === 'p1'); + expect(p1Fragment).toBeDefined(); + expect(p1Fragment!.y).toBeCloseTo(130, 0); + + // Page 2 (documentPageNumber=2, even) → 'even' variant → 40px + // Body start = max(50, 30+40) = 70 + const p2Fragment = layout.pages[1].fragments.find((f) => f.blockId === 'p2'); + expect(p2Fragment).toBeDefined(); + expect(p2Fragment!.y).toBeCloseTo(70, 0); + + // Page 3 (documentPageNumber=3, odd) → 'odd' variant → 80px + // Body start = max(50, 30+80) = 110 + const p3Fragment = layout.pages[2].fragments.find((f) => f.blockId === 'p3'); + expect(p3Fragment).toBeDefined(); + expect(p3Fragment!.y).toBeCloseTo(110, 0); + }); + + it('multi-section: uses document page number for even/odd, not section-relative', () => { + // Section 1 has 3 pages (pages 1-3), section 2 starts on page 4. + // Page 4 is even by document number, but sectionPageNumber=1 (odd). + // The fix ensures document page number is used for even/odd. + const sb1: SectionBreakBlock = { + kind: 'sectionBreak', + id: 'sb1', + attrs: { isFirstSection: true, source: 'sectPr', sectionIndex: 0 }, + pageSize: { w: 600, h: 800 }, + margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 }, + }; + const sb2: SectionBreakBlock = { + kind: 'sectionBreak', + id: 'sb2', + type: 'nextPage', + attrs: { source: 'sectPr', sectionIndex: 1 }, + pageSize: { w: 600, h: 800 }, + margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 }, + }; + + const options: LayoutOptions = { + pageSize: { w: 600, h: 800 }, + margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 }, + alternateHeaders: true, + sectionMetadata: [{ sectionIndex: 0 }, { sectionIndex: 1 }], + headerContentHeights: { + odd: 80, + even: 40, + }, + }; + + const layout = layoutDocument( + [sb1, tallBlock('p1'), tallBlock('p2'), tallBlock('p3'), sb2, tallBlock('p4')], + [{ kind: 'sectionBreak' }, tallMeasure, tallMeasure, tallMeasure, { kind: 'sectionBreak' }, tallMeasure], + options, + ); + + expect(layout.pages.length).toBeGreaterThanOrEqual(4); + + // Page 4 (documentPageNumber=4, even) → should use 'even' header (40px) + // NOT 'odd' which would happen if sectionPageNumber (1) were used + // Body start = max(50, 30+40) = 70 + const p4Fragment = layout.pages[3]?.fragments.find((f) => f.blockId === 'p4'); + expect(p4Fragment).toBeDefined(); + expect(p4Fragment!.y).toBeCloseTo(70, 0); + }); + + it('selects even/odd footer heights when alternateHeaders is true', () => { + // The footer-height path uses the per-rId map + sectionMetadata.footerRefs. + // Exposing the variant selection through `footerContentHeights` alone is not + // sufficient — without refs, the code falls back to 'default' for the footer + // variant regardless. We need the ref map to observe variant switching on + // `page.margins.bottom`. + const options: LayoutOptions = { + pageSize: { w: 600, h: 800 }, + margins: { top: 50, right: 50, bottom: 50, left: 50, footer: 30 }, + alternateHeaders: true, + sectionMetadata: [{ sectionIndex: 0, footerRefs: { odd: 'rIdFooterOdd', even: 'rIdFooterEven' } }], + footerContentHeightsByRId: new Map([ + ['rIdFooterOdd', 80], // Odd pages: larger footer + ['rIdFooterEven', 40], // Even pages: smaller footer + ]), + }; + + const layout = layoutDocument([tallBlock('p1'), tallBlock('p2')], [tallMeasure, tallMeasure], options); + + expect(layout.pages).toHaveLength(2); + + // Page 1 is odd → 'odd' footer (80px) → bottom = max(50, 30+80) = 110 + // Page 2 is even → 'even' footer (40px) → bottom = max(50, 30+40) = 70 + // Body-top Y is footer-independent, so assert on the effective bottom margin + // the paginator stamped on each page. + expect(layout.pages[0].margins?.bottom).toBeCloseTo(110, 0); + expect(layout.pages[1].margins?.bottom).toBeCloseTo(70, 0); + }); + + it('falls back to default header when only default is defined with alternateHeaders', () => { + // Production path: a document with `w:evenAndOddHeaders` on but only a + // `default` header authored. sectionMetadata supplies the `default` ref and + // the per-rId height map supplies its measurement. Step-3 fallback at + // index.ts:1345-1349 kicks in and `effectiveVariantType` drops to 'default'. + const options: LayoutOptions = { + pageSize: { w: 600, h: 800 }, + margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 }, + alternateHeaders: true, + sectionMetadata: [{ sectionIndex: 0, headerRefs: { default: 'rIdHeaderDefault' } }], + headerContentHeightsByRId: new Map([['rIdHeaderDefault', 60]]), + }; + + const layout = layoutDocument([tallBlock('p1'), tallBlock('p2')], [tallMeasure, tallMeasure], options); + + expect(layout.pages).toHaveLength(2); + + // Both pages fall back to the default header (60px), so body start is the + // same on odd and even: max(50, 30+60) = 90. + const p1Fragment = layout.pages[0].fragments.find((f) => f.blockId === 'p1'); + const p2Fragment = layout.pages[1].fragments.find((f) => f.blockId === 'p2'); + expect(p1Fragment!.y).toBeCloseTo(90, 0); + expect(p2Fragment!.y).toBeCloseTo(90, 0); + // Effective top margin is also 90 on both pages. + expect(layout.pages[0].margins?.top).toBeCloseTo(90, 0); + expect(layout.pages[1].margins?.top).toBeCloseTo(90, 0); + }); + + it('multi-section + titlePg + alternateHeaders: first page of section 2 lands on an even doc-page', () => { + // Most realistic mixed case. Section 1 has 3 pages (docPN 1-3). Section 2 + // has titlePg=true and starts on docPN=4. + // - Page 4 is sectionPageNumber=1 for section 2 + titlePg=true → 'first' + // - Page 5 is docPN=5 (odd) → 'odd' (regardless of section-relative number) + // - Page 6 is docPN=6 (even) → 'even' + // If the code used sectionPageNumber for even/odd, pages 5 and 6 would be + // swapped (section-relative 2 and 3 respectively). This guards both titlePg + // and the docPN rule across a section boundary. + const sb1: SectionBreakBlock = { + kind: 'sectionBreak', + id: 'sb1', + attrs: { isFirstSection: true, source: 'sectPr', sectionIndex: 0 }, + pageSize: { w: 600, h: 800 }, + margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 }, + }; + const sb2: SectionBreakBlock = { + kind: 'sectionBreak', + id: 'sb2', + type: 'nextPage', + attrs: { source: 'sectPr', sectionIndex: 1 }, + pageSize: { w: 600, h: 800 }, + margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 }, + }; + + const options: LayoutOptions = { + pageSize: { w: 600, h: 800 }, + margins: { top: 50, right: 50, bottom: 50, left: 50, header: 30 }, + alternateHeaders: true, + sectionMetadata: [{ sectionIndex: 0 }, { sectionIndex: 1, titlePg: true }], + headerContentHeights: { + first: 100, // section 2 title-page header + odd: 80, + even: 40, + }, + }; + + const layout = layoutDocument( + [sb1, tallBlock('p1'), tallBlock('p2'), tallBlock('p3'), sb2, tallBlock('p4'), tallBlock('p5'), tallBlock('p6')], + [ + { kind: 'sectionBreak' }, + tallMeasure, + tallMeasure, + tallMeasure, + { kind: 'sectionBreak' }, + tallMeasure, + tallMeasure, + tallMeasure, + ], + options, + ); + + expect(layout.pages.length).toBeGreaterThanOrEqual(6); + + // Page 4: section 2 first page + titlePg → 'first' (100px) → y = max(50, 30+100) = 130 + const p4Fragment = layout.pages[3]?.fragments.find((f) => f.blockId === 'p4'); + expect(p4Fragment).toBeDefined(); + expect(p4Fragment!.y).toBeCloseTo(130, 0); + + // Page 5: docPN=5, odd → 'odd' (80px) → y = max(50, 30+80) = 110 + // If sectionPageNumber were used: sectionPN=2 → 'even' (40) → y = 70 (wrong) + const p5Fragment = layout.pages[4]?.fragments.find((f) => f.blockId === 'p5'); + expect(p5Fragment).toBeDefined(); + expect(p5Fragment!.y).toBeCloseTo(110, 0); + + // Page 6: docPN=6, even → 'even' (40px) → y = max(50, 30+40) = 70 + // If sectionPageNumber were used: sectionPN=3 → 'odd' (80) → y = 110 (wrong) + const p6Fragment = layout.pages[5]?.fragments.find((f) => f.blockId === 'p6'); + expect(p6Fragment).toBeDefined(); + expect(p6Fragment!.y).toBeCloseTo(70, 0); + }); +}); diff --git a/packages/layout-engine/layout-engine/src/index.ts b/packages/layout-engine/layout-engine/src/index.ts index f2a384ee37..1b0574b964 100644 --- a/packages/layout-engine/layout-engine/src/index.ts +++ b/packages/layout-engine/layout-engine/src/index.ts @@ -1,5 +1,6 @@ import type { ColumnLayout, + ColumnRegion, FlowBlock, Fragment, HeaderFooterLayout, @@ -35,6 +36,7 @@ import { scheduleSectionBreak as scheduleSectionBreakExport, type SectionState, applyPendingToActive, + SINGLE_COLUMN_DEFAULT, } from './section-breaks.js'; import { layoutParagraphBlock } from './layout-paragraph.js'; import { layoutImageBlock } from './layout-image.js'; @@ -526,6 +528,17 @@ export type LayoutOptions = { * behavior for paragraph-free overlays. */ allowSectionBreakOnlyPageFallback?: boolean; + /** + * Whether the document has odd/even header/footer differentiation enabled. + * Corresponds to the w:evenAndOddHeaders element in OOXML settings.xml. + * When true, odd pages use the 'odd' variant and even pages use the 'even' variant. + * When false or omitted, all pages use the 'default' variant. + * + * Must stay in sync with `getHeaderFooterTypeForSection` in + * `layout-bridge/src/headerFooterUtils.ts` — both sides read this value + * and must agree on variant selection. + */ + alternateHeaders?: boolean; }; export type HeaderFooterConstraints = { @@ -667,23 +680,29 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options /** * Determines the header/footer variant type for a given page based on section settings. * - * @param sectionPageNumber - The page number within the current section (1-indexed) + * Takes a params object because the two page-number fields have very similar + * names and types — a positional call site is easy to get wrong. + * + * @param sectionPageNumber - The page number within the current section (1-indexed), used for titlePg + * @param documentPageNumber - The absolute document page number (1-indexed), used for even/odd * @param titlePgEnabled - Whether the section has "different first page" enabled - * @param alternateHeaders - Whether the section has odd/even differentiation enabled + * @param alternateHeaders - Whether the document has odd/even differentiation enabled * @returns The variant type: 'first', 'even', 'odd', or 'default' */ - const getVariantTypeForPage = ( - sectionPageNumber: number, - titlePgEnabled: boolean, - alternateHeaders: boolean, - ): 'default' | 'first' | 'even' | 'odd' => { + const getVariantTypeForPage = (args: { + sectionPageNumber: number; + documentPageNumber: number; + titlePgEnabled: boolean; + alternateHeaders: boolean; + }): 'default' | 'first' | 'even' | 'odd' => { // First page of section with titlePg enabled uses 'first' variant - if (sectionPageNumber === 1 && titlePgEnabled) { + if (args.sectionPageNumber === 1 && args.titlePgEnabled) { return 'first'; } - // Alternate headers (even/odd differentiation) - if (alternateHeaders) { - return sectionPageNumber % 2 === 0 ? 'even' : 'odd'; + // Alternate headers: even/odd based on document page number, matching + // the rendering side (getHeaderFooterTypeForSection in headerFooterUtils.ts) + if (args.alternateHeaders) { + return args.documentPageNumber % 2 === 0 ? 'even' : 'odd'; } return 'default'; }; @@ -1001,14 +1020,18 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options if (block.orientation) next.pendingOrientation = block.orientation; const sectionType = block.type ?? 'continuous'; // Check if columns are changing: either explicitly to a different config, - // or implicitly resetting to single column (undefined = single column in OOXML) + // or implicitly resetting to single column (undefined = single column in OOXML). + // withSeparator must be compared because a sep-only toggle still needs a new + // column region so the renderer can draw (or stop drawing) the separator from + // the toggle point onward. const isColumnsChanging = (block.columns && (block.columns.count !== next.activeColumns.count || block.columns.gap !== next.activeColumns.gap || + Boolean(block.columns.withSeparator) !== Boolean(next.activeColumns.withSeparator) || block.columns.equalWidth !== next.activeColumns.equalWidth || !widthsEqual(block.columns.widths, next.activeColumns.widths))) || - (!block.columns && next.activeColumns.count > 1); + (!block.columns && (next.activeColumns.count > 1 || Boolean(next.activeColumns.withSeparator))); // Schedule section index change for next page (enables section-aware page numbering) const sectionIndexRaw = block.attrs?.sectionIndex; const metadataIndex = typeof sectionIndexRaw === 'number' ? sectionIndexRaw : Number(sectionIndexRaw ?? NaN); @@ -1074,6 +1097,11 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options if (activeOrientation) { page.orientation = activeOrientation; } + + if (activeColumns.count > 1) { + page.columns = cloneColumnLayout(activeColumns); + } + // Set vertical alignment from active section state if (activeVAlign && activeVAlign !== 'top') { page.vAlign = activeVAlign; @@ -1284,11 +1312,15 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options // Get section metadata for titlePg setting const sectionMetadata = sectionMetadataList[activeSectionIndex]; const titlePgEnabled = sectionMetadata?.titlePg ?? false; - // TODO: Support alternateHeaders (odd/even) when needed - const alternateHeaders = false; + const alternateHeaders = options.alternateHeaders ?? false; // Determine which header/footer variant applies to this page - const variantType = getVariantTypeForPage(sectionPageNumber, titlePgEnabled, alternateHeaders); + const variantType = getVariantTypeForPage({ + sectionPageNumber, + documentPageNumber: newPageNumber, + titlePgEnabled, + alternateHeaders, + }); // Resolve header/footer refs for margin calculation using OOXML inheritance model. // This must match the rendering logic in PresentationEditor to ensure margins @@ -1440,9 +1472,16 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options // Start a new mid-page region with different column configuration const startMidPageRegion = (state: PageState, newColumns: ColumnLayout): void => { - // Record the boundary at current Y position + // Use the maximum Y reached across all columns so the new region starts + // below ALL column content, not just the current column's cursor position. + // This prevents overlap when a multi-column section's columns have unequal heights. + const regionStartY = Math.max(state.cursorY, state.maxCursorY); + state.cursorY = regionStartY; + state.maxCursorY = regionStartY; + + // Record the boundary at the resolved Y position const boundary: ConstraintBoundary = { - y: state.cursorY, + y: regionStartY, columns: newColumns, }; state.constraintBoundaries.push(boundary); @@ -1454,7 +1493,7 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options layoutLog(`[Layout] *** COLUMNS CHANGED MID-PAGE ***`); layoutLog(` OLD activeColumns: ${JSON.stringify(activeColumns)}`); layoutLog(` NEW activeColumns: ${JSON.stringify(newColumns)}`); - layoutLog(` Current page: ${state.page.number}, cursorY: ${state.cursorY}`); + layoutLog(` Current page: ${state.page.number}, cursorY: ${state.cursorY}, maxCursorY: ${state.maxCursorY}`); // Update activeColumns so subsequent pages use this column configuration activeColumns = cloneColumnLayout(newColumns); @@ -1468,9 +1507,6 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options { left: activeLeftMargin, right: activeRightMargin }, activePageSize.w, ); - - // Note: We do NOT reset cursorY - content continues from current position - // This creates the mid-page region effect }; // Collect anchored drawings mapped to their anchor paragraphs @@ -2118,6 +2154,7 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options } } state.cursorY = tableBottomY; + state.maxCursorY = Math.max(state.maxCursorY, state.cursorY); } continue; } @@ -2527,6 +2564,39 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options } } + // Serialize constraint boundaries into page.columnRegions so DomPainter can + // draw per-region overlays (e.g. column separator lines) bounded by the + // correct Y span. Continuous section breaks with a changed column config + // push boundaries into PageState.constraintBoundaries during layout; without + // this step the renderer only sees the page-start column config and would + // draw a single full-page separator across regions it no longer applies to. + for (const state of states) { + const boundaries = state.constraintBoundaries; + if (boundaries.length === 0) continue; + + const regions: ColumnRegion[] = []; + // First region spans from the top of the content area to the first boundary. + // Its columns come from page.columns (set at page creation before any + // mid-page region change) or fall back to a single-column default so the + // contract stays self-describing even when the page starts single-column. + const firstRegionColumns: ColumnLayout = state.page.columns ?? { count: 1, gap: 0 }; + regions.push({ + yStart: state.topMargin, + yEnd: boundaries[0].y, + columns: firstRegionColumns, + }); + for (let i = 0; i < boundaries.length; i++) { + const start = boundaries[i]; + const end = boundaries[i + 1]; + regions.push({ + yStart: start.y, + yEnd: end ? end.y : state.contentBottom, + columns: start.columns, + }); + } + state.page.columnRegions = regions; + } + return { pageSize, pages, @@ -2534,7 +2604,7 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options // after processing sections. Page/region-specific column changes are encoded // implicitly via fragment positions. Consumers should not assume this is // a static document-wide value. - columns: activeColumns.count > 1 ? { count: activeColumns.count, gap: activeColumns.gap } : undefined, + columns: activeColumns.count > 1 ? cloneColumnLayout(activeColumns) : undefined, }; } @@ -2961,3 +3031,5 @@ export type { NumberingContext, ResolvePageTokensResult } from './resolvePageTok // Table utilities consumed by layout-bridge and cross-package sync tests export { getCellLines, getEmbeddedRowLines } from './layout-table.js'; export { describeCellRenderBlocks, computeCellSliceContentHeight } from './table-cell-slice.js'; + +export { SINGLE_COLUMN_DEFAULT } from './section-breaks.js'; diff --git a/packages/layout-engine/layout-engine/src/layout-drawing.test.ts b/packages/layout-engine/layout-engine/src/layout-drawing.test.ts index 27a5f7f01e..d70415fad2 100644 --- a/packages/layout-engine/layout-engine/src/layout-drawing.test.ts +++ b/packages/layout-engine/layout-engine/src/layout-drawing.test.ts @@ -62,6 +62,7 @@ describe('layoutDrawingBlock', () => { constraintBoundaries: []; activeConstraintIndex: number; trailingSpacing: number; + maxCursorY: number; }; const createMockPageState = (overrides: Record = {}): MockPageState => @@ -76,6 +77,7 @@ describe('layoutDrawingBlock', () => { constraintBoundaries: [], activeConstraintIndex: -1, trailingSpacing: 0, + maxCursorY: 100, ...overrides, }) as MockPageState; diff --git a/packages/layout-engine/layout-engine/src/layout-drawing.ts b/packages/layout-engine/layout-engine/src/layout-drawing.ts index 12596f4da7..1ec149f646 100644 --- a/packages/layout-engine/layout-engine/src/layout-drawing.ts +++ b/packages/layout-engine/layout-engine/src/layout-drawing.ts @@ -140,4 +140,5 @@ export function layoutDrawingBlock({ state.page.fragments.push(fragment); state.cursorY += requiredHeight; + state.maxCursorY = Math.max(state.maxCursorY, state.cursorY); } diff --git a/packages/layout-engine/layout-engine/src/layout-image.ts b/packages/layout-engine/layout-engine/src/layout-image.ts index a1cfdc651e..ee14cae6b1 100644 --- a/packages/layout-engine/layout-engine/src/layout-image.ts +++ b/packages/layout-engine/layout-engine/src/layout-image.ts @@ -86,4 +86,5 @@ export function layoutImageBlock({ state.page.fragments.push(fragment); state.cursorY += requiredHeight; + state.maxCursorY = Math.max(state.maxCursorY, state.cursorY); } diff --git a/packages/layout-engine/layout-engine/src/layout-paragraph.test.ts b/packages/layout-engine/layout-engine/src/layout-paragraph.test.ts index ade89230e7..39220fe3cb 100644 --- a/packages/layout-engine/layout-engine/src/layout-paragraph.test.ts +++ b/packages/layout-engine/layout-engine/src/layout-paragraph.test.ts @@ -60,6 +60,7 @@ const makePageState = (): PageState => ({ trailingSpacing: 0, lastParagraphStyleId: undefined, lastParagraphContextualSpacing: false, + maxCursorY: 50, }); /** @@ -1271,6 +1272,7 @@ describe('layoutParagraphBlock - keepLines', () => { currentState = { ...state, cursorY: 50, // Reset to top of new page + maxCursorY: 50, page: { number: state.page.number + 1, fragments: [] }, trailingSpacing: 0, }; @@ -1420,6 +1422,7 @@ describe('layoutParagraphBlock - keepLines', () => { const advanceColumn = mock((state: PageState) => ({ ...state, cursorY: 50, + maxCursorY: 50, trailingSpacing: 0, page: { number: 2, fragments: [] }, })); diff --git a/packages/layout-engine/layout-engine/src/layout-paragraph.ts b/packages/layout-engine/layout-engine/src/layout-paragraph.ts index 598f1503c7..5bbce31c03 100644 --- a/packages/layout-engine/layout-engine/src/layout-paragraph.ts +++ b/packages/layout-engine/layout-engine/src/layout-paragraph.ts @@ -789,6 +789,7 @@ export function layoutParagraphBlock(ctx: ParagraphLayoutContext, anchors?: Para if (neededSpacingBefore > 0) { state.cursorY += neededSpacingBefore; + state.maxCursorY = Math.max(state.maxCursorY, state.cursorY); if (spacingDebugEnabled) { spacingDebugLog('spacingBefore applied', { blockId: block.id, @@ -907,6 +908,7 @@ export function layoutParagraphBlock(ctx: ParagraphLayoutContext, anchors?: Para state.page.fragments.push(fragment); state.cursorY += borderExpansion.top + fragmentHeight + borderExpansion.bottom; + state.maxCursorY = Math.max(state.maxCursorY, state.cursorY); lastState = state; fromLine = slice.toLine; } @@ -929,6 +931,7 @@ export function layoutParagraphBlock(ctx: ParagraphLayoutContext, anchors?: Para appliedSpacingAfter = 0; } else { targetState.cursorY += spacingAfter; + targetState.maxCursorY = Math.max(targetState.maxCursorY, targetState.cursorY); } targetState.trailingSpacing = appliedSpacingAfter; if (spacingDebugEnabled) { diff --git a/packages/layout-engine/layout-engine/src/layout-table.ts b/packages/layout-engine/layout-engine/src/layout-table.ts index 7f37f86d40..101de88516 100644 --- a/packages/layout-engine/layout-engine/src/layout-table.ts +++ b/packages/layout-engine/layout-engine/src/layout-table.ts @@ -1239,6 +1239,7 @@ function layoutMonolithicTable(context: TableLayoutContext): void { applyTableFragmentPmRange(fragment, context.block, context.measure); state.page.fragments.push(fragment); state.cursorY += height; + state.maxCursorY = Math.max(state.maxCursorY, state.cursorY); } /** @@ -1389,6 +1390,7 @@ export function layoutTableBlock({ applyTableFragmentPmRange(fragment, block, measure); state.page.fragments.push(fragment); state.cursorY += height; + state.maxCursorY = Math.max(state.maxCursorY, state.cursorY); return; } @@ -1554,6 +1556,7 @@ export function layoutTableBlock({ applyTableFragmentPmRange(fragment, block, measure); state.page.fragments.push(fragment); state.cursorY += fragmentHeight; + state.maxCursorY = Math.max(state.maxCursorY, state.cursorY); } const rowComplete = !hasRemainingLinesAfterContinuation; @@ -1668,6 +1671,7 @@ export function layoutTableBlock({ applyTableFragmentPmRange(fragment, block, measure); state.page.fragments.push(fragment); state.cursorY += fragmentHeight; + state.maxCursorY = Math.max(state.maxCursorY, state.cursorY); pendingPartialRow = forcedPartialRow; samePagePartialContinuation = true; isTableContinuation = true; @@ -1717,6 +1721,7 @@ export function layoutTableBlock({ applyTableFragmentPmRange(fragment, block, measure); state.page.fragments.push(fragment); state.cursorY += fragmentHeight; + state.maxCursorY = Math.max(state.maxCursorY, state.cursorY); // Handle partial row tracking if (partialRow && !partialRow.isLastPart) { diff --git a/packages/layout-engine/layout-engine/src/paginator.ts b/packages/layout-engine/layout-engine/src/paginator.ts index a87d43b73c..f09597f3ad 100644 --- a/packages/layout-engine/layout-engine/src/paginator.ts +++ b/packages/layout-engine/layout-engine/src/paginator.ts @@ -20,6 +20,10 @@ export type PageState = { lastParagraphContextualSpacing: boolean; /** Border hash of the last paragraph for between-border group detection. */ lastParagraphBorderHash?: string; + /** Tracks the maximum cursorY reached across all columns on this page. + * Used when starting a mid-page region so the new section begins below + * all column content, not just the current column's cursor. */ + maxCursorY: number; }; export type PaginatorOptions = { @@ -107,6 +111,7 @@ export function createPaginator(opts: PaginatorOptions) { trailingSpacing: 0, lastParagraphStyleId: undefined, lastParagraphContextualSpacing: false, + maxCursorY: topMargin, }; states.push(state); pages.push(state.page); @@ -123,6 +128,8 @@ export function createPaginator(opts: PaginatorOptions) { const advanceColumn = (state: PageState): PageState => { const activeCols = getActiveColumnsForState(state); if (state.columnIndex < activeCols.count - 1) { + // Snapshot max Y before resetting cursor for the next column + state.maxCursorY = Math.max(state.maxCursorY, state.cursorY); state.columnIndex += 1; if (state.activeConstraintIndex >= 0 && state.constraintBoundaries[state.activeConstraintIndex]) { state.cursorY = state.constraintBoundaries[state.activeConstraintIndex].y; diff --git a/packages/layout-engine/layout-engine/src/section-breaks.d.ts b/packages/layout-engine/layout-engine/src/section-breaks.d.ts index a4fec8ae80..310d6e4690 100644 --- a/packages/layout-engine/layout-engine/src/section-breaks.d.ts +++ b/packages/layout-engine/layout-engine/src/section-breaks.d.ts @@ -1,4 +1,5 @@ -import type { SectionBreakBlock } from '@superdoc/contracts'; +import type { ColumnLayout, SectionBreakBlock } from '@superdoc/contracts'; + export type SectionState = { activeTopMargin: number; activeBottomMargin: number; @@ -20,14 +21,8 @@ export type SectionState = { w: number; h: number; } | null; - activeColumns: { - count: number; - gap: number; - }; - pendingColumns: { - count: number; - gap: number; - } | null; + activeColumns: ColumnLayout; + pendingColumns: ColumnLayout | null; activeOrientation: 'portrait' | 'landscape' | null; pendingOrientation: 'portrait' | 'landscape' | null; hasAnyPages: boolean; @@ -37,6 +32,7 @@ export type BreakDecision = { forceMidPageRegion: boolean; requiredParity?: 'even' | 'odd'; }; + /** * Schedule section break effects by updating pending/active state and returning a break decision. * This function is pure with respect to inputs/outputs and does not mutate external variables. @@ -56,6 +52,7 @@ export declare function scheduleSectionBreak( decision: BreakDecision; state: SectionState; }; + /** * Apply pending margins/pageSize/columns/orientation to active values at a page boundary and clear pending. */ diff --git a/packages/layout-engine/layout-engine/src/section-breaks.test.ts b/packages/layout-engine/layout-engine/src/section-breaks.test.ts index 209c601ecf..49f3143b1f 100644 --- a/packages/layout-engine/layout-engine/src/section-breaks.test.ts +++ b/packages/layout-engine/layout-engine/src/section-breaks.test.ts @@ -140,6 +140,44 @@ describe('scheduleSectionBreak', () => { expect(result.decision.forceMidPageRegion).toBe(false); expect(result.state.pendingColumns).toEqual({ count: 2, gap: 48 }); }); + + it('detects column change when only withSeparator toggles on', () => { + const state = createSectionState({ activeColumns: { count: 2, gap: 48, withSeparator: false } }); + const block = createSectionBreak({ + type: 'continuous', + columns: { count: 2, gap: 48, withSeparator: true }, + }); + + const result = scheduleSectionBreak(block, state, BASE_MARGINS); + + expect(result.decision.forceMidPageRegion).toBe(true); + expect(result.state.pendingColumns).toEqual({ count: 2, gap: 48, withSeparator: true }); + }); + + it('detects column change when only withSeparator toggles off', () => { + const state = createSectionState({ activeColumns: { count: 2, gap: 48, withSeparator: true } }); + const block = createSectionBreak({ + type: 'continuous', + columns: { count: 2, gap: 48, withSeparator: false }, + }); + + const result = scheduleSectionBreak(block, state, BASE_MARGINS); + + expect(result.decision.forceMidPageRegion).toBe(true); + expect(result.state.pendingColumns).toEqual({ count: 2, gap: 48, withSeparator: false }); + }); + + it('does not trigger mid-page region change when undefined and defined false match', () => { + const state = createSectionState({ activeColumns: { count: 2, gap: 48, withSeparator: false } }); + const block = createSectionBreak({ + type: 'continuous', + columns: { count: 2, gap: 48 }, + }); + + const result = scheduleSectionBreak(block, state, BASE_MARGINS); + + expect(result.decision.forceMidPageRegion).toBe(false); + }); }); describe('first section handling', () => { diff --git a/packages/layout-engine/layout-engine/src/section-breaks.ts b/packages/layout-engine/layout-engine/src/section-breaks.ts index 6eb911657f..3fce475a66 100644 --- a/packages/layout-engine/layout-engine/src/section-breaks.ts +++ b/packages/layout-engine/layout-engine/src/section-breaks.ts @@ -30,7 +30,7 @@ export type BreakDecision = { }; /** Default single-column configuration per OOXML spec (absence of w:cols element) */ -const SINGLE_COLUMN_DEFAULT: Readonly = { count: 1, gap: 0 }; +export const SINGLE_COLUMN_DEFAULT: Readonly = { count: 1, gap: 0 }; /** * Get the column configuration for a section break. @@ -38,7 +38,7 @@ const SINGLE_COLUMN_DEFAULT: Readonly = { count: 1, gap: 0 }; * Per OOXML spec, absence of element means single column layout. * * @param blockColumns - The columns property from the section break block (may be undefined) - * @returns Column configuration with count and gap + * @returns Column configuration with count, gap, and separator presence */ function getColumnConfig(blockColumns: ColumnLayout | undefined): ColumnLayout { return blockColumns ? cloneColumnLayout(blockColumns) : { ...SINGLE_COLUMN_DEFAULT }; @@ -56,17 +56,22 @@ function getColumnConfig(blockColumns: ColumnLayout | undefined): ColumnLayout { */ function isColumnConfigChanging(blockColumns: ColumnLayout | undefined, activeColumns: ColumnLayout): boolean { if (blockColumns) { - // Explicit column change + // Explicit column change: any of count, gap, separator presence, equalWidth, + // or widths differs. withSeparator must be included because a sep-only toggle + // still needs a new column region so the renderer can draw (or stop drawing) + // the separator from the toggle point onward. return ( blockColumns.count !== activeColumns.count || blockColumns.gap !== activeColumns.gap || + Boolean(blockColumns.withSeparator) !== Boolean(activeColumns.withSeparator) || blockColumns.equalWidth !== activeColumns.equalWidth || !widthsEqual(blockColumns.widths, activeColumns.widths) ); } - // No columns specified = reset to single column (OOXML default) - // This is a change only if currently in multi-column layout - return activeColumns.count > 1; + // No columns specified = reset to single column (OOXML default). + // This is a change if currently in multi-column layout, or if the separator was on + // (the reset implicitly turns it off). + return activeColumns.count > 1 || Boolean(activeColumns.withSeparator); } /** diff --git a/packages/layout-engine/layout-engine/src/section-props.test.ts b/packages/layout-engine/layout-engine/src/section-props.test.ts index 49dbfd4f30..4b65588b3d 100644 --- a/packages/layout-engine/layout-engine/src/section-props.test.ts +++ b/packages/layout-engine/layout-engine/src/section-props.test.ts @@ -61,4 +61,53 @@ describe('computeNextSectionPropsAtBreak', () => { expect(snapshot?.columns).toEqual({ count: 2, gap: 48 }); expect(snapshot?.columns).not.toBe(sourceColumns); }); + + it('propagates withSeparator flag through section property snapshots', () => { + const sourceColumns = { count: 2, gap: 48, withSeparator: true }; + const blocks: FlowBlock[] = [sectionBreak({ id: 'sb-0', columns: sourceColumns })]; + const map = computeNextSectionPropsAtBreak(blocks); + const snapshot = map.get(0); + + expect(snapshot?.columns).toEqual({ count: 2, gap: 48, withSeparator: true }); + expect(snapshot?.columns).not.toBe(sourceColumns); + }); + + it('omits withSeparator from the snapshot when not set on source block', () => { + const sourceColumns = { count: 2, gap: 48 }; + const blocks: FlowBlock[] = [sectionBreak({ id: 'sb-0', columns: sourceColumns })]; + const map = computeNextSectionPropsAtBreak(blocks); + const snapshot = map.get(0); + + expect(snapshot?.columns).toEqual({ count: 2, gap: 48 }); + expect(snapshot?.columns?.withSeparator).toBeUndefined(); + }); + + it('propagates withSeparator from the next section in lookahead', () => { + const blocks: FlowBlock[] = [ + sectionBreak({ id: 'sb-0', columns: { count: 1, gap: 0 } }), + { kind: 'paragraph', id: 'p-1', runs: [] } as FlowBlock, + sectionBreak({ id: 'sb-2', columns: { count: 2, gap: 48, withSeparator: true } }), + ]; + const map = computeNextSectionPropsAtBreak(blocks); + + expect(map.get(0)?.columns).toEqual({ count: 2, gap: 48, withSeparator: true }); + expect(map.get(2)?.columns).toEqual({ count: 2, gap: 48, withSeparator: true }); + }); + + it('preserves explicit column widths and equalWidth in snapshots', () => { + const sourceColumns = { count: 2, gap: 48, widths: [120, 360], equalWidth: false, withSeparator: true }; + const blocks: FlowBlock[] = [sectionBreak({ id: 'sb-0', columns: sourceColumns })]; + const map = computeNextSectionPropsAtBreak(blocks); + const snapshot = map.get(0); + + expect(snapshot?.columns).toEqual({ + count: 2, + gap: 48, + widths: [120, 360], + equalWidth: false, + withSeparator: true, + }); + expect(snapshot?.columns).not.toBe(sourceColumns); + expect(snapshot?.columns?.widths).not.toBe(sourceColumns.widths); + }); }); diff --git a/packages/layout-engine/layout-engine/src/section-props.ts b/packages/layout-engine/layout-engine/src/section-props.ts index 6c8c6c84c3..e43bc07f5c 100644 --- a/packages/layout-engine/layout-engine/src/section-props.ts +++ b/packages/layout-engine/layout-engine/src/section-props.ts @@ -1,4 +1,5 @@ -import type { FlowBlock, SectionVerticalAlign } from '@superdoc/contracts'; +import type { ColumnLayout, FlowBlock, SectionVerticalAlign } from '@superdoc/contracts'; +import { cloneColumnLayout } from './column-utils.js'; /** * Section-level formatting properties that control page layout. @@ -16,11 +17,16 @@ import type { FlowBlock, SectionVerticalAlign } from '@superdoc/contracts'; export type SectionProps = { margins?: { header?: number; footer?: number; top?: number; right?: number; bottom?: number; left?: number }; pageSize?: { w: number; h: number }; - columns?: { count: number; gap: number }; + columns?: ColumnLayout; orientation?: 'portrait' | 'landscape'; vAlign?: SectionVerticalAlign; }; +const snapshotColumns = (columns?: ColumnLayout): ColumnLayout | undefined => { + if (!columns) return undefined; + return cloneColumnLayout(columns); +}; + /** * Extracts section properties from a section break block if any are present. * Returns null if the block has no section-related properties. @@ -59,7 +65,7 @@ const _snapshotSectionProps = (block: FlowBlock): SectionProps | null => { } if (block.columns) { hasProps = true; - props.columns = { count: block.columns.count, gap: block.columns.gap }; + props.columns = snapshotColumns(block.columns); } if (block.orientation) { hasProps = true; @@ -135,7 +141,7 @@ export function computeNextSectionPropsAtBreak(blocks: FlowBlock[]): Map { } }); + it('aligns trailing tabs to explicit right stops with dot leaders (TOC regression)', async () => { + const rightStopTwips = 10593; + const rightStopPx = rightStopTwips * (96 / 1440); // ~706px + const block: FlowBlock = { + kind: 'paragraph', + id: 'toc-paragraph', + runs: [ + { text: '1.', fontFamily: 'Arial', fontSize: 13.333 }, + { kind: 'tab', text: '\t', tabIndex: 0, pmStart: 2, pmEnd: 3 }, + { text: 'Generalities', fontFamily: 'Arial', fontSize: 13.333 }, + { kind: 'tab', text: '\t', tabIndex: 1, pmStart: 15, pmEnd: 16 }, + { text: '5', fontFamily: 'Arial', fontSize: 13.333 }, + ], + attrs: { + indent: { left: 30, right: 0, firstLine: 0, hanging: 30 }, + tabs: [{ val: 'end', leader: 'dot', pos: rightStopTwips }], + }, + }; + + const measure = expectParagraphMeasure(await measureBlock(block, 800)); + expect(measure.lines).toHaveLength(1); + const line = measure.lines[0]; + expect(line.leaders).toBeDefined(); + expect(line.leaders?.[0]?.style).toBe('dot'); + // Leader must end right before the page number — within ~20px of the right stop + // (page number "5" is a few px wide, not 100+ px wide). + expect(line.leaders?.[0]?.to).toBeLessThanOrEqual(rightStopPx); + expect(line.leaders?.[0]?.to).toBeGreaterThan(rightStopPx - 20); + // Leader must start AFTER the title text, not at "1." — proves the first tab + // fell on the default 0.5" grid, not on the end stop. + expect(line.leaders?.[0]?.from).toBeGreaterThan(100); + const trailingTab = block.runs[3]; + if (trailingTab.kind === 'tab') { + expect(trailingTab.width).toBeGreaterThan(50); + } + }); + + it('maps three trailing tabs to two explicit alignment stops (asymmetric case)', async () => { + const centerStopTwips = 5000; + const endStopTwips = 10000; + const block: FlowBlock = { + kind: 'paragraph', + id: 'asymmetric-tabs', + runs: [ + { text: 'A', fontFamily: 'Arial', fontSize: 13.333 }, + { kind: 'tab', text: '\t', tabIndex: 0, pmStart: 1, pmEnd: 2 }, + { text: 'B', fontFamily: 'Arial', fontSize: 13.333 }, + { kind: 'tab', text: '\t', tabIndex: 1, pmStart: 3, pmEnd: 4 }, + { text: 'C', fontFamily: 'Arial', fontSize: 13.333 }, + { kind: 'tab', text: '\t', tabIndex: 2, pmStart: 5, pmEnd: 6 }, + { text: 'D', fontFamily: 'Arial', fontSize: 13.333 }, + ], + attrs: { + indent: { left: 0, right: 0, firstLine: 0, hanging: 0 }, + tabs: [ + { val: 'center', pos: centerStopTwips }, + { val: 'end', pos: endStopTwips }, + ], + }, + }; + + const measure = expectParagraphMeasure(await measureBlock(block, 800)); + expect(measure.lines).toHaveLength(1); + // Three tabs, two alignment stops: last two tabs bind to center + end. + // The first tab must NOT bind to either alignment stop — it should fall on the + // default grid. The last tab ends near the end stop position. + const lineWidth = measure.lines[0].width; + const endStopPx = endStopTwips * (96 / 1440); + expect(lineWidth).toBeCloseTo(endStopPx, 0); + }); + it('handles multiple tabs in a row', async () => { const block: FlowBlock = { kind: 'paragraph', @@ -1881,6 +1952,91 @@ describe('measureBlock', () => { } } }); + + it('uses surrounding text font size for tab line height, not hardcoded 12', async () => { + // Regression: tab runs previously hardcoded maxFontSize=12, producing + // wrong line heights when the surrounding text used a larger font. + const largeFontBlock: FlowBlock = { + kind: 'paragraph', + id: 'tab-font-size-large', + runs: [ + { text: 'Hello', fontFamily: 'Arial', fontSize: 24 }, + { kind: 'tab', text: '\t', pmStart: 5, pmEnd: 6 }, + { text: 'World', fontFamily: 'Arial', fontSize: 24 }, + ], + attrs: {}, + }; + + const smallFontBlock: FlowBlock = { + kind: 'paragraph', + id: 'tab-font-size-small', + runs: [ + { text: 'Hello', fontFamily: 'Arial', fontSize: 10 }, + { kind: 'tab', text: '\t', pmStart: 5, pmEnd: 6 }, + { text: 'World', fontFamily: 'Arial', fontSize: 10 }, + ], + attrs: {}, + }; + + const largeMeasure = expectParagraphMeasure(await measureBlock(largeFontBlock, 1000)); + const smallMeasure = expectParagraphMeasure(await measureBlock(smallFontBlock, 1000)); + + expect(largeMeasure.lines).toHaveLength(1); + expect(smallMeasure.lines).toHaveLength(1); + + // The large-font paragraph must have a taller line than the small-font one. + // With the old hardcoded 12, both could collapse to similar heights. + expect(largeMeasure.lines[0].lineHeight).toBeGreaterThan(smallMeasure.lines[0].lineHeight); + }); + + it('uses fallback font size when tab is the first run (no preceding text)', async () => { + // When a tab starts a paragraph, lastFontSize should fall back to the + // first text run's font size, not a hardcoded default. + const block: FlowBlock = { + kind: 'paragraph', + id: 'tab-first-run', + runs: [ + { kind: 'tab', text: '\t', pmStart: 0, pmEnd: 1 }, + { text: 'After tab', fontFamily: 'Arial', fontSize: 20 }, + ], + attrs: {}, + }; + + const refBlock: FlowBlock = { + kind: 'paragraph', + id: 'no-tab-ref', + runs: [{ text: 'After tab', fontFamily: 'Arial', fontSize: 20 }], + attrs: {}, + }; + + const measure = expectParagraphMeasure(await measureBlock(block, 1000)); + const refMeasure = expectParagraphMeasure(await measureBlock(refBlock, 1000)); + + expect(measure.lines).toHaveLength(1); + // Line height should match or exceed the reference (same font size drives both) + expect(measure.lines[0].lineHeight).toBeGreaterThanOrEqual(refMeasure.lines[0].lineHeight); + }); + + it('tab-only line inherits font size from following text run', async () => { + // A line that contains only a tab should derive its height from the + // paragraph's font size context, not from a hardcoded 12pt. + const block: FlowBlock = { + kind: 'paragraph', + id: 'tab-only-line', + runs: [{ kind: 'tab', text: '\t', pmStart: 0, pmEnd: 1 }], + attrs: { + // paragraph-level font size hint via a nearby run + }, + }; + + const measure = expectParagraphMeasure(await measureBlock(block, 1000)); + + expect(measure.lines).toHaveLength(1); + // With the fallback font size (default 12 when no runs present), + // the line should still have a reasonable height + expect(measure.lines[0].lineHeight).toBeGreaterThan(0); + expect(measure.lines[0].maxFontSize).toBeGreaterThan(0); + }); }); describe('space-only runs', () => { diff --git a/packages/layout-engine/measuring/dom/src/index.ts b/packages/layout-engine/measuring/dom/src/index.ts index ded7439f9e..d756023eea 100644 --- a/packages/layout-engine/measuring/dom/src/index.ts +++ b/packages/layout-engine/measuring/dom/src/index.ts @@ -982,6 +982,9 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P block.attrs?.tabs as TabStop[], block.attrs?.tabIntervalTwips as number | undefined, ); + const alignmentTabStopsPx = tabStops + .map((stop, index) => ({ stop, index })) + .filter(({ stop }) => stop.val === 'end' || stop.val === 'center' || stop.val === 'decimal'); const decimalSeparator = sanitizeDecimalSeparator(block.attrs?.decimalSeparator); // Extract bar tab stops for paragraph-level rendering (OOXML: bars on all lines) @@ -1230,7 +1233,7 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P }; // Expand runs to handle inline newlines as explicit break runs - const runsToProcess: Run[] = []; + let runsToProcess: Run[] = []; for (const run of normalizedRuns as Run[]) { if ((run as TextRun).text && typeof (run as TextRun).text === 'string' && (run as TextRun).text.includes('\n')) { const textRun = run as TextRun; @@ -1259,6 +1262,58 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P runsToProcess.push(run as Run); } } + if (runsToProcess.some((run) => isTextRun(run) && typeof run.text === 'string' && run.text.includes('\t'))) { + const expandedRuns: Run[] = []; + for (const run of runsToProcess) { + if (!isTextRun(run) || typeof run.text !== 'string' || !run.text.includes('\t')) { + expandedRuns.push(run); + continue; + } + const textRun = run as TextRun; + let buffer = ''; + let cursor = textRun.pmStart ?? 0; + const text = textRun.text; + for (let i = 0; i < text.length; i += 1) { + const char = text[i]; + if (char === '\t') { + if (buffer.length > 0) { + expandedRuns.push({ + ...textRun, + text: buffer, + pmStart: cursor - buffer.length, + pmEnd: cursor, + }); + buffer = ''; + } + const tabRun: TabRun = { + kind: 'tab', + text: '\t', + pmStart: cursor, + pmEnd: cursor + 1, + tabStops: block.attrs?.tabs as TabStop[] | undefined, + indent, + leader: (textRun as unknown as TabRun)?.leader ?? null, + sdt: textRun.sdt, + }; + expandedRuns.push(tabRun); + cursor += 1; + continue; + } + buffer += char; + cursor += 1; + } + if (buffer.length > 0) { + expandedRuns.push({ + ...textRun, + text: buffer, + pmStart: cursor - buffer.length, + pmEnd: cursor, + }); + } + } + runsToProcess = expandedRuns; + } + const totalTabRuns = runsToProcess.reduce((count, run) => (isTabRun(run) ? count + 1 : count), 0); /** * Trims trailing regular spaces from a line when it is finalized. @@ -1306,7 +1361,23 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P } }; - // Process each run + // Word-compat heuristic (not ECMA-376 17.3.3.32): the last N tab characters in a + // paragraph bind to the last N explicit end/center/decimal stops. Needed for TOC + // entries where a right-aligned dot-leader stop coexists with default grid stops — + // strict greedy next-stop resolution would land the trailing tab on a default stop + // instead of the leader stop. Mirrored in layout-bridge/src/remeasure.ts. + const getAlignmentStopForOrdinal = (ordinal: number): { stop: TabStopPx; index: number } | null => { + if (alignmentTabStopsPx.length === 0 || totalTabRuns === 0 || !Number.isFinite(ordinal)) { + return null; + } + if (ordinal < 0 || ordinal >= totalTabRuns) return null; + const remainingTabs = totalTabRuns - ordinal - 1; + const targetIndex = alignmentTabStopsPx.length - 1 - remainingTabs; + if (targetIndex < 0 || targetIndex >= alignmentTabStopsPx.length) return null; + return alignmentTabStopsPx[targetIndex]; + }; + + let sequentialTabIndex = 0; for (let runIndex = 0; runIndex < runsToProcess.length; runIndex++) { const run = runsToProcess[runIndex]; @@ -1411,20 +1482,40 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P toRun: runIndex, toChar: 1, width: 0, - maxFontSize: 12, // Default font size for tabs + maxFontSize: lastFontSize, + maxFontInfo: hasSeenTextRun ? undefined : fallbackFontInfo, maxWidth: getEffectiveWidth(lines.length === 0 ? initialAvailableWidth : bodyContentWidth), segments: [], spaceCount: 0, }; } - // Advance to next tab stop using the same logic as inline "\t" handling + // Advance to the appropriate tab stop (explicit alignment stops take precedence for trailing tabs) const originX = currentLine.width; // Use first-line effective indent (accounts for hanging) on first line, body indent otherwise const effectiveIndent = lines.length === 0 ? indentLeft + rawFirstLineOffset : indentLeft; const absCurrentX = currentLine.width + effectiveIndent; - const { target, nextIndex, stop } = getNextTabStopPx(absCurrentX, tabStops, tabStopCursor); - tabStopCursor = nextIndex; + let stop: TabStopPx | undefined; + let target: number; + const resolvedTabIndex = + typeof (run as TabRun).tabIndex === 'number' && Number.isFinite((run as TabRun).tabIndex) + ? (run as TabRun).tabIndex! + : sequentialTabIndex; + // Keep the sequential counter in sync with explicit tabIndex values so mixed + // inputs (explicit + synthetic TabRuns) don't produce out-of-order ordinals. + // Mirrors consumeTabOrdinal() in layout-bridge/src/remeasure.ts. + sequentialTabIndex = Math.max(sequentialTabIndex, resolvedTabIndex + 1); + const forcedAlignment = getAlignmentStopForOrdinal(resolvedTabIndex); + if (forcedAlignment && forcedAlignment.stop.pos > absCurrentX + TAB_EPSILON) { + stop = forcedAlignment.stop; + target = forcedAlignment.stop.pos; + tabStopCursor = forcedAlignment.index + 1; + } else { + const nextStop = getNextTabStopPx(absCurrentX, tabStops, tabStopCursor); + target = nextStop.target; + tabStopCursor = nextStop.nextIndex; + stop = nextStop.stop; + } const maxAbsWidth = currentLine.maxWidth + effectiveIndent; const clampedTarget = Math.min(target, maxAbsWidth); const tabAdvance = Math.max(0, clampedTarget - absCurrentX); @@ -1432,7 +1523,7 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P // Persist measured tab width on the TabRun for downstream consumers/tests (run as TabRun & { width?: number }).width = tabAdvance; - currentLine.maxFontSize = Math.max(currentLine.maxFontSize, 12); + currentLine.maxFontSize = Math.max(currentLine.maxFontSize, lastFontSize); currentLine.toRun = runIndex; currentLine.toChar = 1; // tab is a single character let currentLeader: LeaderDecoration | null = null; diff --git a/packages/layout-engine/painters/dom/src/features/math/CONTRIBUTING.md b/packages/layout-engine/painters/dom/src/features/math/CONTRIBUTING.md index 8c002d976d..3764f6da11 100644 --- a/packages/layout-engine/painters/dom/src/features/math/CONTRIBUTING.md +++ b/packages/layout-engine/painters/dom/src/features/math/CONTRIBUTING.md @@ -36,7 +36,9 @@ type MathObjectConverter = ( doc: Document, // For creating DOM elements convertChildren: (children: OmmlJsonNode[]) => DocumentFragment, // Recursively converts nested OMML content -) => Element | null; +) => Node | null; // Return a single Element for one atom, or a + // DocumentFragment when your converter produces + // multiple sibling elements (see m:r / math-run). ``` `convertChildren` is the important one. Pass it any child elements that contain nested math content (`m:e`, `m:num`, `m:sub`, etc.). It handles everything inside them, including other math objects. diff --git a/packages/layout-engine/painters/dom/src/features/math/converters/box.ts b/packages/layout-engine/painters/dom/src/features/math/converters/box.ts new file mode 100644 index 0000000000..45dbf14bda --- /dev/null +++ b/packages/layout-engine/painters/dom/src/features/math/converters/box.ts @@ -0,0 +1,122 @@ +import type { MathObjectConverter } from '../types.js'; + +const MATHML_NS = 'http://www.w3.org/1998/Math/MathML'; + +/** + * Convert m:box (grouping container) to MathML . + * + * OMML structure: + * m:box → m:boxPr (optional), m:e (content) + * + * MathML output: + * content + * + * Per §22.1.2.13 / §22.1.2.14, m:box can carry boxPr children that affect + * layout and spacing — opEmu (operator emulator), noBreak (disallow line + * breaks), aln (alignment point), diff (differential spacing), argSz. These + * have no clean MathML equivalent and are currently dropped; the box + * degrades to a plain that preserves grouping but not the other + * semantics. Extend here when any of these need first-class support. + * + * @spec ECMA-376 §22.1.2.13, §22.1.2.14 + */ +export const convertBox: MathObjectConverter = (node, doc, convertChildren) => { + const elements = node.elements ?? []; + const base = elements.find((e) => e.name === 'm:e'); + + const mrow = doc.createElementNS(MATHML_NS, 'mrow'); + mrow.appendChild(convertChildren(base?.elements ?? [])); + + return mrow.childNodes.length > 0 ? mrow : null; +}; + +/** + * Convert m:borderBox (bordered box) to MathML . + * + * OMML structure: + * m:borderBox → m:borderBoxPr (optional: m:hideTop, m:hideBot, m:hideLeft, m:hideRight, + * m:strikeBLTR, m:strikeH, m:strikeTLBR, m:strikeV), + * m:e (content) + * + * MathML output: + * content + * + * By default all four borders are shown (notation="box"). Individual borders + * can be hidden via m:hide* flags, and diagonal/horizontal/vertical strikes + * can be added via m:strike* flags. + * + * @spec ECMA-376 §22.1.2.11 + */ +export const convertBorderBox: MathObjectConverter = (node, doc, convertChildren) => { + const elements = node.elements ?? []; + const props = elements.find((e) => e.name === 'm:borderBoxPr'); + const base = elements.find((e) => e.name === 'm:e'); + + /** + * OOXML ST_OnOff (§22.9.2.7): on when the element is present and either + * `m:val` is absent (spec default = 1) or equals "1" / "true". "on" is + * accepted for leniency — Annex L.6.1.3 uses that form even though the + * normative enum is {0, 1, true, false}. + * TODO: extract to a shared util when m:acc / m:phant / matrix m:tblLook land. + */ + const isOn = (el?: { attributes?: Record }) => { + if (!el) return false; + const val = el.attributes?.['m:val']; + if (val === undefined) return true; + return val === '1' || val === 'true' || val === 'on'; + }; + + const hideTop = props?.elements?.find((e) => e.name === 'm:hideTop'); + const hideBot = props?.elements?.find((e) => e.name === 'm:hideBot'); + const hideLeft = props?.elements?.find((e) => e.name === 'm:hideLeft'); + const hideRight = props?.elements?.find((e) => e.name === 'm:hideRight'); + const strikeBLTR = props?.elements?.find((e) => e.name === 'm:strikeBLTR'); + const strikeH = props?.elements?.find((e) => e.name === 'm:strikeH'); + const strikeTLBR = props?.elements?.find((e) => e.name === 'm:strikeTLBR'); + const strikeV = props?.elements?.find((e) => e.name === 'm:strikeV'); + + const notations: string[] = []; + + const allHidden = isOn(hideTop) && isOn(hideBot) && isOn(hideLeft) && isOn(hideRight); + + if (!allHidden) { + if (!isOn(hideTop) && !isOn(hideBot) && !isOn(hideLeft) && !isOn(hideRight)) { + notations.push('box'); + } else { + if (!isOn(hideTop)) notations.push('top'); + if (!isOn(hideBot)) notations.push('bottom'); + if (!isOn(hideLeft)) notations.push('left'); + if (!isOn(hideRight)) notations.push('right'); + } + } + + if (isOn(strikeBLTR)) notations.push('updiagonalstrike'); + if (isOn(strikeH)) notations.push('horizontalstrike'); + if (isOn(strikeTLBR)) notations.push('downdiagonalstrike'); + if (isOn(strikeV)) notations.push('verticalstrike'); + + const content = convertChildren(base?.elements ?? []); + + // Drop empty wrappers — matches convertBox / convertFunction. + if (content.childNodes.length === 0) return null; + + if (notations.length === 0) { + const mrow = doc.createElementNS(MATHML_NS, 'mrow'); + mrow.appendChild(content); + return mrow; + } + + // Wrap the content in an inner before placing it inside . + // MathML Core dropped , so Chrome treats it as unknown and does + // not apply row layout — each child would render as its own `block math` + // line, stacking vertically. An inner is a MathML Core element, so + // the row layout runs on its children and everything stays inline. + const innerMrow = doc.createElementNS(MATHML_NS, 'mrow'); + innerMrow.appendChild(content); + + const menclose = doc.createElementNS(MATHML_NS, 'menclose'); + menclose.setAttribute('notation', notations.join(' ')); + menclose.appendChild(innerMrow); + + return menclose; +}; diff --git a/packages/layout-engine/painters/dom/src/features/math/converters/function.ts b/packages/layout-engine/painters/dom/src/features/math/converters/function.ts index 65d1461f88..6aa879340b 100644 --- a/packages/layout-engine/painters/dom/src/features/math/converters/function.ts +++ b/packages/layout-engine/painters/dom/src/features/math/converters/function.ts @@ -1,12 +1,91 @@ import type { MathObjectConverter } from '../types.js'; +import { convertMathRunAsFunctionName } from './math-run.js'; const MATHML_NS = 'http://www.w3.org/1998/Math/MathML'; const FUNCTION_APPLY_OPERATOR = '\u2061'; +// Boundary elements for the function-name mathvariant walk: every MathML +// element whose children occupy their own semantic slot (base, subscript, +// limit, matrix cell, etc.). When m:fName wraps one of these, the slot +// content carries authored styling per ECMA-376 §22.1.2.111 and must not be +// overwritten. Anything inside these is skipped. +const MATH_VARIANT_BOUNDARY_ELEMENTS = new Set([ + 'munder', + 'mover', + 'munderover', + 'msub', + 'msup', + 'msubsup', + 'mmultiscripts', + 'mfrac', + 'msqrt', + 'mroot', + 'mtable', + 'mtr', + 'mtd', +]); + function forceNormalMathVariant(root: ParentNode): void { - root.querySelectorAll('mi').forEach((identifier) => { - identifier.setAttribute('mathvariant', 'normal'); - }); + // Array.from is required here: HTMLCollection is not iterable under the + // default DOM lib (needs `dom.iterable`), so `for…of root.children` fails + // type-check. + for (const child of Array.from(root.children)) { + if (MATH_VARIANT_BOUNDARY_ELEMENTS.has(child.localName)) continue; + if (child.localName === 'mi' && !child.hasAttribute('mathvariant')) { + child.setAttribute('mathvariant', 'normal'); + } + forceNormalMathVariant(child); + } +} + +/** + * Structural MathML elements whose FIRST child is the "function-name base" + * when nested inside m:fName (e.g. m:limLow → , m:limUpp → , + * m:sSub → , etc.). Word's OMML2MML.XSL keeps the base text whole + * (e.g. "lim" as one ) even though it splits regular runs per-character. + */ +const BASE_BEARING_ELEMENTS = new Set([ + 'munder', + 'mover', + 'munderover', + 'msub', + 'msup', + 'msubsup', + 'mmultiscripts', // m:sPre inside m:fName +]); + +/** + * After per-character splitting in convertMathRun, the base of a nested + * limit/script inside m:fName comes out as multiple single-char siblings + * wrapped in an . Word's XSL keeps that base whole — merge the siblings + * back into a single if they all share the same (or no) mathvariant. + */ +function collapseFunctionNameBases(root: ParentNode): void { + for (const child of Array.from(root.children)) { + if (BASE_BEARING_ELEMENTS.has(child.localName)) { + const base = child.children[0]; + if (base) { + collapseMrowToSingleMi(base); + collapseFunctionNameBases(base); + } + } else { + collapseFunctionNameBases(child); + } + } +} + +function collapseMrowToSingleMi(container: Element): void { + const children = Array.from(container.children); + if (children.length < 2) return; + if (!children.every((c) => c.localName === 'mi')) return; + const variant = children[0]!.getAttribute('mathvariant'); + if (!children.every((c) => c.getAttribute('mathvariant') === variant)) return; + + const merged = container.ownerDocument!.createElementNS(MATHML_NS, 'mi'); + merged.textContent = children.map((c) => c.textContent ?? '').join(''); + if (variant) merged.setAttribute('mathvariant', variant); + container.insertBefore(merged, children[0]!); + for (const c of children) c.remove(); } /** @@ -31,7 +110,19 @@ export const convertFunction: MathObjectConverter = (node, doc, convertChildren) const wrapper = doc.createElementNS(MATHML_NS, 'mrow'); const functionNameRow = doc.createElementNS(MATHML_NS, 'mrow'); - functionNameRow.appendChild(convertChildren(functionName?.elements ?? [])); + // m:r children of m:fName stay whole (Word's OMML2MML.XSL keeps multi-letter + // function names like "sin" or "lim" as a single ). Non-m:r children — + // like a nested m:limLow — go through the normal recursive path. + for (const child of functionName?.elements ?? []) { + if (child.name === 'm:r') { + const atom = convertMathRunAsFunctionName(child, doc); + if (atom) functionNameRow.appendChild(atom); + } else { + const converted = convertChildren([child]); + if (converted.childNodes.length > 0) functionNameRow.appendChild(converted); + } + } + collapseFunctionNameBases(functionNameRow); forceNormalMathVariant(functionNameRow); if (functionNameRow.childNodes.length > 0) { diff --git a/packages/layout-engine/painters/dom/src/features/math/converters/index.ts b/packages/layout-engine/painters/dom/src/features/math/converters/index.ts index fcc7916fcb..0af6795656 100644 --- a/packages/layout-engine/painters/dom/src/features/math/converters/index.ts +++ b/packages/layout-engine/painters/dom/src/features/math/converters/index.ts @@ -24,3 +24,4 @@ export { convertNary } from './nary.js'; export { convertPhantom } from './phantom.js'; export { convertGroupCharacter } from './group-character.js'; export { convertMatrix } from './matrix.js'; +export { convertBox, convertBorderBox } from './box.js'; diff --git a/packages/layout-engine/painters/dom/src/features/math/converters/math-run.ts b/packages/layout-engine/painters/dom/src/features/math/converters/math-run.ts index 79fddd2375..7f979563f9 100644 --- a/packages/layout-engine/painters/dom/src/features/math/converters/math-run.ts +++ b/packages/layout-engine/painters/dom/src/features/math/converters/math-run.ts @@ -46,8 +46,7 @@ const OPERATOR_CHARS = new Set([ '\u220C', // ∈, ∉, ∋, ∌ '\u2211', '\u220F', // ∑, ∏ - '\u221A', - '\u221E', // √, ∞ + '\u221A', // √ (radical sign — prefix operator) '\u2227', '\u2228', '\u2229', @@ -65,16 +64,70 @@ const OPERATOR_CHARS = new Set([ '\u2287', // ⊂, ⊃, ⊆, ⊇ ]); +type MathAtomTag = 'mi' | 'mo' | 'mn'; + +function isDigit(ch: string): boolean { + return ch >= '0' && ch <= '9'; +} + /** - * Classify a text string into MathML element type. - * - All-digit strings → (number) - * - Known operators → (operator) - * - Everything else → (identifier) + * Length in UTF-16 code units of the code point starting at `text[i]`. + * Handles surrogate pairs so astral-plane characters (e.g. mathematical + * italic U+1D465) don't get split into two bogus atoms. */ -function classifyMathText(text: string): 'mn' | 'mo' | 'mi' { - if (/^\d*\.?\d+$/.test(text)) return 'mn'; - if (text.length === 1 && OPERATOR_CHARS.has(text)) return 'mo'; - return 'mi'; +function codePointUnitLength(text: string, i: number): number { + const hi = text.charCodeAt(i); + if (hi >= 0xd800 && hi <= 0xdbff && i + 1 < text.length) { + const lo = text.charCodeAt(i + 1); + if (lo >= 0xdc00 && lo <= 0xdfff) return 2; + } + return 1; +} + +/** + * Split a math run's text into MathML atoms, matching Word's OMML2MML.XSL. + * + * Rules (ECMA-376 §22.1.2.116 example + Annex L.6.1.13): + * - Consecutive digits — optionally containing one decimal point between digits — + * group into a single ``. + * - Each recognized operator character becomes its own ``. + * - Every other character becomes its own ``. + * + * Example: `"n+1"` → `[n, +, 1]`. + */ +export function tokenizeMathText(text: string): Array<{ tag: MathAtomTag; content: string }> { + const atoms: Array<{ tag: MathAtomTag; content: string }> = []; + let i = 0; + while (i < text.length) { + const step = codePointUnitLength(text, i); + const ch = text.slice(i, i + step); + if (step === 1 && isDigit(ch)) { + let end = i + 1; + let sawDot = false; + while (end < text.length) { + const c = text[end]!; + if (isDigit(c)) { + end++; + continue; + } + if (c === '.' && !sawDot && end + 1 < text.length && isDigit(text[end + 1]!)) { + sawDot = true; + end++; + continue; + } + break; + } + atoms.push({ tag: 'mn', content: text.slice(i, end) }); + i = end; + } else if (step === 1 && OPERATOR_CHARS.has(ch)) { + atoms.push({ tag: 'mo', content: ch }); + i++; + } else { + atoms.push({ tag: 'mi', content: ch }); + i += step; + } + } + return atoms; } /** ECMA-376 m:sty → MathML mathvariant (§22.1.2 math run properties). */ @@ -115,47 +168,140 @@ function resolveMathVariant(rPr: OmmlJsonNode | undefined): string | null { return null; } +function extractText(node: OmmlJsonNode): string { + let text = ''; + for (const child of node.elements ?? []) { + if (child.name === 'm:t') { + for (const tc of child.elements ?? []) { + if (tc.type === 'text' && typeof tc.text === 'string') text += tc.text; + } + } + } + return text; +} + /** - * Convert an m:r (math run) element to MathML. + * Convert an m:r (math run) element to MathML atoms. * * m:r contains: * - m:rPr (math run properties: script, style, normal text flag) * - m:t (text content) * - Optionally w:rPr (WordprocessingML run properties for formatting) * - * The text is classified as , , or based on content. + * The run's text is split per-character into `` / `` / `` atoms + * per Word's OMML2MML.XSL. For a single-atom run (common case — a one-letter + * variable, single operator, or an all-digit number) the converter returns a + * single Element. For a multi-atom run (e.g. "→∞", "x+1") it returns a + * DocumentFragment whose children become siblings of the parent mrow. + * + * @spec ECMA-376 §22.1.2.116 (t) — example shows multi-char mixed runs as the + * normal authored shape; §22.1.2.58 (lit) implies operators are classified + * per-character by default. */ export const convertMathRun: MathObjectConverter = (node, doc) => { - const elements = node.elements ?? []; + const text = extractText(node); + if (!text) return null; - // Extract text from m:t children - let text = ''; - for (const child of elements) { - if (child.name === 'm:t') { - const textChildren = child.elements ?? []; - for (const tc of textChildren) { - if (tc.type === 'text' && typeof tc.text === 'string') { - text += tc.text; + const rPr = (node.elements ?? []).find((el) => el.name === 'm:rPr'); + const variant = resolveMathVariant(rPr); + const atoms = tokenizeMathText(text); + + const createAtom = (atom: { tag: MathAtomTag; content: string }): Element => { + const el = doc.createElementNS(MATHML_NS, atom.tag); + el.textContent = atom.content; + // Apply m:rPr-derived variant to every atom in the run. Omitted attribute + // means "use the MathML default" (italic for single-char , normal + // for multi-char //). + if (variant) el.setAttribute('mathvariant', variant); + return el; + }; + + if (atoms.length === 1) return createAtom(atoms[0]!); + + const fragment = doc.createDocumentFragment(); + for (const atom of atoms) fragment.appendChild(createAtom(atom)); + return fragment; +}; + +/** + * Tokenize a math run's text for the m:fName context: consecutive non-digit, + * non-operator characters stay grouped in one `` (so "log" in "log_2" + * remains a single identifier), while digits still group into `` and + * each operator character is its own ``. + * + * Matches Word's OMML2MML.XSL run-internal classification for m:fName + * content: `log_2` → `log_2`. + */ +function tokenizeFunctionNameText(text: string): Array<{ tag: MathAtomTag; content: string }> { + const atoms: Array<{ tag: MathAtomTag; content: string }> = []; + let i = 0; + while (i < text.length) { + const step = codePointUnitLength(text, i); + const ch = text.slice(i, i + step); + if (step === 1 && isDigit(ch)) { + let end = i + 1; + let sawDot = false; + while (end < text.length) { + const c = text[end]!; + if (isDigit(c)) { + end++; + continue; + } + if (c === '.' && !sawDot && end + 1 < text.length && isDigit(text[end + 1]!)) { + sawDot = true; + end++; + continue; } + break; } + atoms.push({ tag: 'mn', content: text.slice(i, end) }); + i = end; + } else if (step === 1 && OPERATOR_CHARS.has(ch)) { + atoms.push({ tag: 'mo', content: ch }); + i++; + } else { + // Group consecutive non-digit, non-operator code points into one . + let end = i + step; + while (end < text.length) { + const s = codePointUnitLength(text, end); + const c = text.slice(end, end + s); + if (s === 1 && (isDigit(c) || OPERATOR_CHARS.has(c))) break; + end += s; + } + atoms.push({ tag: 'mi', content: text.slice(i, end) }); + i = end; } } + return atoms; +} +/** + * Convert an m:r inside m:fName (m:func's function-name slot). Word's + * OMML2MML.XSL keeps each letter-sequence whole while still splitting out + * digits and operators — so `sin` stays `sin`, but `log_2` becomes + * `log_2`. + * + * Returns a single Element for single-atom runs or a DocumentFragment when + * the run emits multiple atoms. Returns null for empty text. + */ +export function convertMathRunAsFunctionName(node: OmmlJsonNode, doc: Document): Node | null { + const text = extractText(node); if (!text) return null; - const rPr = elements.find((el) => el.name === 'm:rPr'); + const rPr = (node.elements ?? []).find((el) => el.name === 'm:rPr'); const variant = resolveMathVariant(rPr); - const tag = classifyMathText(text); + const atoms = tokenizeFunctionNameText(text); - const el = doc.createElementNS(MATHML_NS, tag); - el.textContent = text; + const createAtom = (atom: { tag: MathAtomTag; content: string }): Element => { + const el = doc.createElementNS(MATHML_NS, atom.tag); + el.textContent = atom.content; + if (variant) el.setAttribute('mathvariant', variant); + return el; + }; - // Apply mathvariant when the spec properties resolve to one. The default - // for single-char is italic and for multi-char // is - // normal — we only set an attribute when m:rPr explicitly specifies it. - if (variant) { - el.setAttribute('mathvariant', variant); - } + if (atoms.length === 1) return createAtom(atoms[0]!); - return el; -}; + const fragment = doc.createDocumentFragment(); + for (const atom of atoms) fragment.appendChild(createAtom(atom)); + return fragment; +} diff --git a/packages/layout-engine/painters/dom/src/features/math/omml-to-mathml.test.ts b/packages/layout-engine/painters/dom/src/features/math/omml-to-mathml.test.ts index c5a1345725..32930a01eb 100644 --- a/packages/layout-engine/painters/dom/src/features/math/omml-to-mathml.test.ts +++ b/packages/layout-engine/painters/dom/src/features/math/omml-to-mathml.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect } from 'vitest'; import { JSDOM } from 'jsdom'; import { convertOmmlToMathml, MATHML_NS } from './omml-to-mathml.js'; +import { tokenizeMathText } from './converters/math-run.js'; const dom = new JSDOM(''); const doc = dom.window.document; @@ -343,6 +344,310 @@ describe('convertOmmlToMathml', () => { expect(children.some((c) => c.localName === 'mo')).toBe(true); // + expect(children.some((c) => c.localName === 'mn')).toBe(true); // 1 }); + + // ─── tokenizeMathText direct coverage (SD-2632) ──────────────────────────── + + it('tokenizes leading-decimal content with . as an operator followed by digits', () => { + // ".5" has no leading digit, so the "." is not part of a number. + expect(tokenizeMathText('.5')).toEqual([ + { tag: 'mo', content: '.' }, + { tag: 'mn', content: '5' }, + ]); + }); + + it('tokenizes a trailing decimal point as a separate operator', () => { + // "5." — the digit run ends at "5" because a lookahead digit is required. + expect(tokenizeMathText('5.')).toEqual([ + { tag: 'mn', content: '5' }, + { tag: 'mo', content: '.' }, + ]); + }); + + it('tokenizes "1.2.3" as number, operator, number — only first dot is inline', () => { + expect(tokenizeMathText('1.2.3')).toEqual([ + { tag: 'mn', content: '1.2' }, + { tag: 'mo', content: '.' }, + { tag: 'mn', content: '3' }, + ]); + }); + + it('tokenizes "2x+1" — number-identifier-operator-number', () => { + expect(tokenizeMathText('2x+1')).toEqual([ + { tag: 'mn', content: '2' }, + { tag: 'mi', content: 'x' }, + { tag: 'mo', content: '+' }, + { tag: 'mn', content: '1' }, + ]); + }); + + it('tokenizes consecutive operator characters as separate atoms', () => { + expect(tokenizeMathText('\u2264\u2265')).toEqual([ + { tag: 'mo', content: '\u2264' }, + { tag: 'mo', content: '\u2265' }, + ]); + }); + + it('tokenizes empty text as an empty list', () => { + expect(tokenizeMathText('')).toEqual([]); + }); + + it('tokenizes standalone ∞ as identifier, not operator (SD-2632)', () => { + // U+221E was removed from OPERATOR_CHARS; Word classifies it as . + expect(tokenizeMathText('\u221E')).toEqual([{ tag: 'mi', content: '\u221E' }]); + }); + + it('keeps astral-plane characters whole (does not split surrogate pairs)', () => { + // 𝑥 (U+1D465, mathematical italic small x) is a UTF-16 surrogate pair. + // Splitting by code unit would emit two bogus half-pair s. + const text = '\u{1D465}+1'; + expect(tokenizeMathText(text)).toEqual([ + { tag: 'mi', content: '\u{1D465}' }, + { tag: 'mo', content: '+' }, + { tag: 'mn', content: '1' }, + ]); + }); + + // ─── SD-2632: per-character split of multi-char m:r text ────────────────── + + it('splits a single m:r containing operator + identifier into + (SD-2632)', () => { + // Fixture case 1 of math-limit-tests.docx has m:r "→∞" as one run inside + // m:limLow's m:lim. Word's OMML2MML.XSL splits it to →∞. + const omml = { + name: 'm:oMath', + elements: [{ name: 'm:r', elements: [{ name: 'm:t', elements: [{ type: 'text', text: '\u2192\u221E' }] }] }], + }; + const result = convertOmmlToMathml(omml, doc); + const children = Array.from(result!.children); + expect(children.map((c) => `${c.localName}:${c.textContent}`)).toEqual(['mo:\u2192', 'mi:\u221E']); + }); + + it('splits "x+1=2" per character with digits grouped (SD-2632)', () => { + const omml = { + name: 'm:oMath', + elements: [{ name: 'm:r', elements: [{ name: 'm:t', elements: [{ type: 'text', text: 'x+1=2' }] }] }], + }; + const result = convertOmmlToMathml(omml, doc); + const children = Array.from(result!.children); + expect(children.map((c) => `${c.localName}:${c.textContent}`)).toEqual(['mi:x', 'mo:+', 'mn:1', 'mo:=', 'mn:2']); + }); + + it('groups consecutive digits with an interior decimal point into one (SD-2632)', () => { + const omml = { + name: 'm:oMath', + elements: [{ name: 'm:r', elements: [{ name: 'm:t', elements: [{ type: 'text', text: '123.45+67' }] }] }], + }; + const result = convertOmmlToMathml(omml, doc); + const children = Array.from(result!.children); + expect(children.map((c) => `${c.localName}:${c.textContent}`)).toEqual(['mn:123.45', 'mo:+', 'mn:67']); + }); + + it('splits m:r content inside m:sub of an m:sSub (SD-2632 F3)', () => { + // Word's built-up "b_(n+1)" has "n+1" as a single m:r inside m:sub. + // The subscript should contain separate n+1. + const omml = { + name: 'm:oMath', + elements: [ + { + name: 'm:sSub', + elements: [ + { + name: 'm:e', + elements: [{ name: 'm:r', elements: [{ name: 'm:t', elements: [{ type: 'text', text: 'b' }] }] }], + }, + { + name: 'm:sub', + elements: [{ name: 'm:r', elements: [{ name: 'm:t', elements: [{ type: 'text', text: 'n+1' }] }] }], + }, + ], + }, + ], + }; + const result = convertOmmlToMathml(omml, doc); + const subMrow = result!.querySelector('msub > mrow:nth-child(2)'); + expect(subMrow).not.toBeNull(); + const children = Array.from(subMrow!.children); + expect(children.map((c) => `${c.localName}:${c.textContent}`)).toEqual(['mi:n', 'mo:+', 'mn:1']); + }); + + it('preserves m:rPr mathvariant across every atom of a split run (SD-2632)', () => { + // When m:sty="b" (bold) applies to the whole run, every atom emitted + // from the split inherits it. + const omml = { + name: 'm:oMath', + elements: [ + { + name: 'm:r', + elements: [ + { name: 'm:rPr', elements: [{ name: 'm:sty', attributes: { 'm:val': 'b' } }] }, + { name: 'm:t', elements: [{ type: 'text', text: 'x+1' }] }, + ], + }, + ], + }; + const result = convertOmmlToMathml(omml, doc); + const variants = Array.from(result!.children).map((c) => c.getAttribute('mathvariant')); + expect(variants).toEqual(['bold', 'bold', 'bold']); + }); + + it('keeps "log" whole but splits operators and digits for m:fName with mixed content (SD-2632)', () => { + // Word's OMML2MML.XSL for log_2: letters group + // into one , operators and digits still split. + const omml = { + name: 'm:oMath', + elements: [ + { + name: 'm:func', + elements: [ + { + name: 'm:fName', + elements: [{ name: 'm:r', elements: [{ name: 'm:t', elements: [{ type: 'text', text: 'log_2' }] }] }], + }, + { + name: 'm:e', + elements: [{ name: 'm:r', elements: [{ name: 'm:t', elements: [{ type: 'text', text: 'x' }] }] }], + }, + ], + }, + ], + }; + const result = convertOmmlToMathml(omml, doc); + const fnameRow = result!.querySelector('mrow > mrow:first-child'); + const children = Array.from(fnameRow!.children); + expect(children.map((c) => `${c.localName}:${c.textContent}`)).toEqual(['mi:log', 'mo:_', 'mn:2']); + }); + + it('collapses a multi-char base inside nested m:sSub wrapped by m:fName (SD-2632)', () => { + // Ensures the msub/msup entries of BASE_BEARING_ELEMENTS are actually pinned. + // fi should + // keep "f" as a single inside the subscript wrapper's base slot. + const omml = { + name: 'm:oMath', + elements: [ + { + name: 'm:func', + elements: [ + { + name: 'm:fName', + elements: [ + { + name: 'm:sSub', + elements: [ + { + name: 'm:e', + elements: [ + { + name: 'm:r', + elements: [ + { name: 'm:rPr', elements: [{ name: 'm:sty', attributes: { 'm:val': 'p' } }] }, + { name: 'm:t', elements: [{ type: 'text', text: 'log' }] }, + ], + }, + ], + }, + { + name: 'm:sub', + elements: [{ name: 'm:r', elements: [{ name: 'm:t', elements: [{ type: 'text', text: '2' }] }] }], + }, + ], + }, + ], + }, + { + name: 'm:e', + elements: [{ name: 'm:r', elements: [{ name: 'm:t', elements: [{ type: 'text', text: 'x' }] }] }], + }, + ], + }, + ], + }; + const result = convertOmmlToMathml(omml, doc); + const msub = result!.querySelector('msub'); + expect(msub).not.toBeNull(); + const baseMi = msub!.children[0]!.querySelector('mi'); + expect(baseMi!.textContent).toBe('log'); + expect(baseMi!.getAttribute('mathvariant')).toBe('normal'); + }); + + it('collapses multi-char base inside nested m:sPre (mmultiscripts) wrapped by m:fName (SD-2632)', () => { + const omml = { + name: 'm:oMath', + elements: [ + { + name: 'm:func', + elements: [ + { + name: 'm:fName', + elements: [ + { + name: 'm:sPre', + elements: [ + { + name: 'm:sub', + elements: [{ name: 'm:r', elements: [{ name: 'm:t', elements: [{ type: 'text', text: '2' }] }] }], + }, + { + name: 'm:sup', + elements: [{ name: 'm:r', elements: [{ name: 'm:t', elements: [{ type: 'text', text: 'n' }] }] }], + }, + { + name: 'm:e', + elements: [ + { + name: 'm:r', + elements: [ + { name: 'm:rPr', elements: [{ name: 'm:sty', attributes: { 'm:val': 'p' } }] }, + { name: 'm:t', elements: [{ type: 'text', text: 'log' }] }, + ], + }, + ], + }, + ], + }, + ], + }, + { + name: 'm:e', + elements: [{ name: 'm:r', elements: [{ name: 'm:t', elements: [{ type: 'text', text: 'x' }] }] }], + }, + ], + }, + ], + }; + const result = convertOmmlToMathml(omml, doc); + const mms = result!.querySelector('mmultiscripts'); + expect(mms).not.toBeNull(); + const baseMi = mms!.children[0]!.querySelector('mi'); + expect(baseMi!.textContent).toBe('log'); + expect(baseMi!.getAttribute('mathvariant')).toBe('normal'); + }); + + it('keeps multi-letter function names whole inside m:func > m:fName (SD-2632 exception)', () => { + // Word's OMML2MML.XSL keeps "sin" as one when nested in m:fName, + // even though it would otherwise per-char split a bare m:r. Exception is + // applied by convertFunction. + const omml = { + name: 'm:oMath', + elements: [ + { + name: 'm:func', + elements: [ + { + name: 'm:fName', + elements: [{ name: 'm:r', elements: [{ name: 'm:t', elements: [{ type: 'text', text: 'sin' }] }] }], + }, + { + name: 'm:e', + elements: [{ name: 'm:r', elements: [{ name: 'm:t', elements: [{ type: 'text', text: 'x' }] }] }], + }, + ], + }, + ], + }; + const result = convertOmmlToMathml(omml, doc); + const functionName = result!.querySelector('mrow > mrow:first-child > mi'); + expect(functionName!.textContent).toBe('sin'); + expect(functionName!.getAttribute('mathvariant')).toBe('normal'); + }); }); describe('m:bar converter', () => { @@ -861,6 +1166,43 @@ describe('m:func converter', () => { expect(mis[0]!.textContent).toBe('sin'); expect(mis[1]!.textContent).toBe('cos'); }); + + it('preserves explicit m:sty=i on function-name runs', () => { + // SD-2538 preserve branch: forceNormalMathVariant must NOT overwrite + // an existing mathvariant. When Word marks a function-name run with + // m:sty="i", convertMathRun already set mathvariant="italic" — the + // function-apply pass must leave it alone. + const omml = { + name: 'm:oMath', + elements: [ + { + name: 'm:func', + elements: [ + { + name: 'm:fName', + elements: [ + { + name: 'm:r', + elements: [ + { name: 'm:rPr', elements: [{ name: 'm:sty', attributes: { 'm:val': 'i' } }] }, + { name: 'm:t', elements: [{ type: 'text', text: 'L' }] }, + ], + }, + ], + }, + { + name: 'm:e', + elements: [{ name: 'm:r', elements: [{ name: 'm:t', elements: [{ type: 'text', text: 'x' }] }] }], + }, + ], + }, + ], + }; + const result = convertOmmlToMathml(omml, doc); + const nameMi = result!.querySelectorAll('mi')[0]; + expect(nameMi!.textContent).toBe('L'); + expect(nameMi!.getAttribute('mathvariant')).toBe('italic'); + }); }); describe('m:rad converter', () => { @@ -2296,8 +2638,18 @@ describe('m:limLow converter', () => { const munder = result!.querySelector('munder'); expect(munder).not.toBeNull(); expect(munder!.children.length).toBe(2); - expect(munder!.children[0]!.textContent).toBe('lim'); - expect(munder!.children[1]!.textContent).toBe('n'); + + // Base: "lim" — upright via m:sty=p on the run. + const limMi = munder!.children[0]!.querySelector('mi'); + expect(limMi!.textContent).toBe('lim'); + expect(limMi!.getAttribute('mathvariant')).toBe('normal'); + + // Limit expression: "n" — must stay italic (no mathvariant attribute set). + // SD-2538 regression: convertFunction used to recurse into the + // and force mathvariant="normal" on every , including this one. + const nMi = munder!.children[1]!.querySelector('mi'); + expect(nMi!.textContent).toBe('n'); + expect(nMi!.getAttribute('mathvariant')).toBeNull(); }); }); @@ -2565,8 +2917,17 @@ describe('m:limUpp converter', () => { const mover = result!.querySelector('mover'); expect(mover).not.toBeNull(); expect(mover!.children.length).toBe(2); - expect(mover!.children[0]!.textContent).toBe('lim'); - expect(mover!.children[1]!.textContent).toBe('x'); + + // Base: "lim" — upright via m:sty=p. Limit variable "x" — italic default. + // Symmetric to the m:limLow-in-m:func case; pins the 'mover' entry of + // MATH_VARIANT_BOUNDARY_ELEMENTS. + const limMi = mover!.children[0]!.querySelector('mi'); + expect(limMi!.textContent).toBe('lim'); + expect(limMi!.getAttribute('mathvariant')).toBe('normal'); + + const xMi = mover!.children[1]!.querySelector('mi'); + expect(xMi!.textContent).toBe('x'); + expect(xMi!.getAttribute('mathvariant')).toBeNull(); }); }); @@ -3929,3 +4290,344 @@ describe('m:m converter', () => { expect(mtable!.textContent).toBe('ab'); }); }); +describe('m:box converter', () => { + it('converts m:box to ', () => { + const omml = { + name: 'm:oMath', + elements: [ + { + name: 'm:box', + elements: [ + { + name: 'm:e', + elements: [{ name: 'm:r', elements: [{ name: 'm:t', elements: [{ type: 'text', text: 'x' }] }] }], + }, + ], + }, + ], + }; + const result = convertOmmlToMathml(omml, doc); + expect(result).not.toBeNull(); + expect(result!.querySelector('mrow')).not.toBeNull(); + expect(result!.textContent).toBe('x'); + }); + + it('returns null for empty m:box', () => { + const omml = { + name: 'm:oMath', + elements: [{ name: 'm:box', elements: [] }], + }; + const result = convertOmmlToMathml(omml, doc); + expect(result).toBeNull(); + }); + + it('drops m:boxPr children (opEmu / noBreak / aln / diff are not yet mapped)', () => { + // Pins current scope: we render and silently ignore boxPr semantics. + // When opEmu or noBreak grow real MathML mappings, this test should fail + // and be updated — that failure is the point. + const omml = { + name: 'm:oMath', + elements: [ + { + name: 'm:box', + elements: [ + { + name: 'm:boxPr', + elements: [ + { name: 'm:opEmu', attributes: { 'm:val': '1' } }, + { name: 'm:noBreak', attributes: { 'm:val': '1' } }, + { name: 'm:aln' }, + { name: 'm:diff', attributes: { 'm:val': '1' } }, + ], + }, + { + name: 'm:e', + elements: [{ name: 'm:r', elements: [{ name: 'm:t', elements: [{ type: 'text', text: '==' }] }] }], + }, + ], + }, + ], + }; + const result = convertOmmlToMathml(omml, doc); + expect(result).not.toBeNull(); + expect(result!.querySelector('mrow')).not.toBeNull(); + expect(result!.querySelector('menclose')).toBeNull(); + expect(result!.textContent).toBe('=='); + }); +}); + +describe('m:borderBox converter', () => { + it('converts m:borderBox to by default', () => { + const omml = { + name: 'm:oMath', + elements: [ + { + name: 'm:borderBox', + elements: [ + { + name: 'm:e', + elements: [{ name: 'm:r', elements: [{ name: 'm:t', elements: [{ type: 'text', text: 'E' }] }] }], + }, + ], + }, + ], + }; + const result = convertOmmlToMathml(omml, doc); + expect(result).not.toBeNull(); + const menclose = result!.querySelector('menclose'); + expect(menclose).not.toBeNull(); + expect(menclose!.getAttribute('notation')).toBe('box'); + expect(menclose!.textContent).toBe('E'); + }); + + it('hides top and bottom sides (notation="left right")', () => { + const omml = { + name: 'm:oMath', + elements: [ + { + name: 'm:borderBox', + elements: [ + { + name: 'm:borderBoxPr', + elements: [ + { name: 'm:hideTop', attributes: { 'm:val': '1' } }, + { name: 'm:hideBot', attributes: { 'm:val': '1' } }, + ], + }, + { + name: 'm:e', + elements: [{ name: 'm:r', elements: [{ name: 'm:t', elements: [{ type: 'text', text: 'x' }] }] }], + }, + ], + }, + ], + }; + const result = convertOmmlToMathml(omml, doc); + const menclose = result!.querySelector('menclose'); + expect(menclose).not.toBeNull(); + // Exact string — production order is top/bottom/left/right, so a side-swap regression fails here. + expect(menclose!.getAttribute('notation')).toBe('left right'); + }); + + it('adds strike notations', () => { + const omml = { + name: 'm:oMath', + elements: [ + { + name: 'm:borderBox', + elements: [ + { + name: 'm:borderBoxPr', + elements: [ + { name: 'm:hideTop', attributes: { 'm:val': '1' } }, + { name: 'm:hideBot', attributes: { 'm:val': '1' } }, + { name: 'm:hideLeft', attributes: { 'm:val': '1' } }, + { name: 'm:hideRight', attributes: { 'm:val': '1' } }, + { name: 'm:strikeH', attributes: { 'm:val': '1' } }, + ], + }, + { + name: 'm:e', + elements: [{ name: 'm:r', elements: [{ name: 'm:t', elements: [{ type: 'text', text: 'y' }] }] }], + }, + ], + }, + ], + }; + const result = convertOmmlToMathml(omml, doc); + const menclose = result!.querySelector('menclose'); + expect(menclose).not.toBeNull(); + expect(menclose!.getAttribute('notation')).toBe('horizontalstrike'); + }); + + it('falls back to when all borders hidden and no strikes', () => { + const omml = { + name: 'm:oMath', + elements: [ + { + name: 'm:borderBox', + elements: [ + { + name: 'm:borderBoxPr', + elements: [ + { name: 'm:hideTop', attributes: { 'm:val': '1' } }, + { name: 'm:hideBot', attributes: { 'm:val': '1' } }, + { name: 'm:hideLeft', attributes: { 'm:val': '1' } }, + { name: 'm:hideRight', attributes: { 'm:val': '1' } }, + ], + }, + { + name: 'm:e', + elements: [{ name: 'm:r', elements: [{ name: 'm:t', elements: [{ type: 'text', text: 'q' }] }] }], + }, + ], + }, + ], + }; + const result = convertOmmlToMathml(omml, doc); + const menclose = result!.querySelector('menclose'); + expect(menclose).toBeNull(); + expect(result!.textContent).toBe('q'); + }); + + // ── ST_OnOff variants (ECMA-376 §22.9.2.7) ──────────────────────────────── + // isOn accepts "1", "true", "on", and bare tags; rejects "0" / "false" / "off". + // Annex L.6.1.3 itself uses m:val="on" even though the normative enum is {0,1,true,false}. + + const makeBorderBox = (hideTopFlag: Record) => ({ + name: 'm:oMath', + elements: [ + { + name: 'm:borderBox', + elements: [ + { name: 'm:borderBoxPr', elements: [{ name: 'm:hideTop', ...hideTopFlag }] }, + { + name: 'm:e', + elements: [{ name: 'm:r', elements: [{ name: 'm:t', elements: [{ type: 'text', text: 'x' }] }] }], + }, + ], + }, + ], + }); + + it('treats m:val="true" as on (ST_OnOff)', () => { + const result = convertOmmlToMathml(makeBorderBox({ attributes: { 'm:val': 'true' } }), doc); + expect(result!.querySelector('menclose')!.getAttribute('notation')).toBe('bottom left right'); + }); + + it('treats m:val="on" as on (Annex L.6.1.3 form)', () => { + const result = convertOmmlToMathml(makeBorderBox({ attributes: { 'm:val': 'on' } }), doc); + expect(result!.querySelector('menclose')!.getAttribute('notation')).toBe('bottom left right'); + }); + + it('treats bare as on (spec default val=1)', () => { + const result = convertOmmlToMathml(makeBorderBox({}), doc); + expect(result!.querySelector('menclose')!.getAttribute('notation')).toBe('bottom left right'); + }); + + it('treats m:val="0" as off (top remains visible)', () => { + const result = convertOmmlToMathml(makeBorderBox({ attributes: { 'm:val': '0' } }), doc); + expect(result!.querySelector('menclose')!.getAttribute('notation')).toBe('box'); + }); + + it('treats m:val="false" as off', () => { + const result = convertOmmlToMathml(makeBorderBox({ attributes: { 'm:val': 'false' } }), doc); + expect(result!.querySelector('menclose')!.getAttribute('notation')).toBe('box'); + }); + + // ── Strike directions ───────────────────────────────────────────────────── + // BLTR (bottom-left → top-right = "/") maps to updiagonalstrike. + // TLBR (top-left → bottom-right = "\") maps to downdiagonalstrike. + // The directional naming is counter-intuitive — these tests pin it. + + const makeStrike = (strikeName: string) => ({ + name: 'm:oMath', + elements: [ + { + name: 'm:borderBox', + elements: [ + { + name: 'm:borderBoxPr', + elements: [ + { name: 'm:hideTop', attributes: { 'm:val': '1' } }, + { name: 'm:hideBot', attributes: { 'm:val': '1' } }, + { name: 'm:hideLeft', attributes: { 'm:val': '1' } }, + { name: 'm:hideRight', attributes: { 'm:val': '1' } }, + { name: strikeName, attributes: { 'm:val': '1' } }, + ], + }, + { + name: 'm:e', + elements: [{ name: 'm:r', elements: [{ name: 'm:t', elements: [{ type: 'text', text: 'a' }] }] }], + }, + ], + }, + ], + }); + + it('maps m:strikeBLTR to notation="updiagonalstrike" (/ direction)', () => { + const result = convertOmmlToMathml(makeStrike('m:strikeBLTR'), doc); + expect(result!.querySelector('menclose')!.getAttribute('notation')).toBe('updiagonalstrike'); + }); + + it('maps m:strikeTLBR to notation="downdiagonalstrike" (\\ direction)', () => { + const result = convertOmmlToMathml(makeStrike('m:strikeTLBR'), doc); + expect(result!.querySelector('menclose')!.getAttribute('notation')).toBe('downdiagonalstrike'); + }); + + it('maps m:strikeV to notation="verticalstrike"', () => { + const result = convertOmmlToMathml(makeStrike('m:strikeV'), doc); + expect(result!.querySelector('menclose')!.getAttribute('notation')).toBe('verticalstrike'); + }); + + it('combines multiple strikes in a fixed order', () => { + const omml = { + name: 'm:oMath', + elements: [ + { + name: 'm:borderBox', + elements: [ + { + name: 'm:borderBoxPr', + elements: [ + { name: 'm:strikeBLTR', attributes: { 'm:val': '1' } }, + { name: 'm:strikeH', attributes: { 'm:val': '1' } }, + { name: 'm:strikeTLBR', attributes: { 'm:val': '1' } }, + { name: 'm:strikeV', attributes: { 'm:val': '1' } }, + ], + }, + { + name: 'm:e', + elements: [{ name: 'm:r', elements: [{ name: 'm:t', elements: [{ type: 'text', text: 'a' }] }] }], + }, + ], + }, + ], + }; + const result = convertOmmlToMathml(omml, doc); + expect(result!.querySelector('menclose')!.getAttribute('notation')).toBe( + 'box updiagonalstrike horizontalstrike downdiagonalstrike verticalstrike', + ); + }); + + it('combines partial hide flags with a strike (hideTop + strikeH)', () => { + const omml = { + name: 'm:oMath', + elements: [ + { + name: 'm:borderBox', + elements: [ + { + name: 'm:borderBoxPr', + elements: [ + { name: 'm:hideTop', attributes: { 'm:val': '1' } }, + { name: 'm:strikeH', attributes: { 'm:val': '1' } }, + ], + }, + { + name: 'm:e', + elements: [{ name: 'm:r', elements: [{ name: 'm:t', elements: [{ type: 'text', text: 'a' }] }] }], + }, + ], + }, + ], + }; + const result = convertOmmlToMathml(omml, doc); + expect(result!.querySelector('menclose')!.getAttribute('notation')).toBe('bottom left right horizontalstrike'); + }); + + it('returns null when m:e is empty (no bordered-but-empty )', () => { + const omml = { + name: 'm:oMath', + elements: [ + { + name: 'm:borderBox', + elements: [{ name: 'm:borderBoxPr', elements: [{ name: 'm:strikeH', attributes: { 'm:val': '1' } }] }], + }, + ], + }; + const result = convertOmmlToMathml(omml, doc); + // oMath still renders but has no children because borderBox dropped itself. + expect(result).toBeNull(); + }); +}); diff --git a/packages/layout-engine/painters/dom/src/features/math/omml-to-mathml.ts b/packages/layout-engine/painters/dom/src/features/math/omml-to-mathml.ts index e2f8f016f9..2f49e3d5f7 100644 --- a/packages/layout-engine/painters/dom/src/features/math/omml-to-mathml.ts +++ b/packages/layout-engine/painters/dom/src/features/math/omml-to-mathml.ts @@ -29,6 +29,8 @@ import { convertPhantom, convertGroupCharacter, convertMatrix, + convertBox, + convertBorderBox, } from './converters/index.js'; export const MATHML_NS = 'http://www.w3.org/1998/Math/MathML'; @@ -66,9 +68,8 @@ const MATH_OBJECT_REGISTRY: Record = { 'm:sSubSup': convertSubSuperscript, // Sub-superscript (both) 'm:sPre': convertPreSubSuperscript, // Pre-sub-superscript (left of base) - // ── Not yet implemented (community contributions welcome) ──────────────── - 'm:borderBox': null, // Border box (border around math content) - 'm:box': null, // Box (invisible grouping container) + 'm:borderBox': convertBorderBox, // Border box (border around math content) + 'm:box': convertBox, // Box (invisible grouping container) 'm:groupChr': convertGroupCharacter, // Group character (overbrace, underbrace) }; diff --git a/packages/layout-engine/painters/dom/src/features/math/types.ts b/packages/layout-engine/painters/dom/src/features/math/types.ts index f1e8d90b71..00c31ea60f 100644 --- a/packages/layout-engine/painters/dom/src/features/math/types.ts +++ b/packages/layout-engine/painters/dom/src/features/math/types.ts @@ -43,4 +43,4 @@ export type MathObjectConverter = ( node: OmmlJsonNode, doc: Document, convertChildren: (children: OmmlJsonNode[]) => DocumentFragment, -) => Element | null; +) => Node | null; diff --git a/packages/layout-engine/painters/dom/src/index.test.ts b/packages/layout-engine/painters/dom/src/index.test.ts index 2ba2389f57..3a1e8f57bf 100644 --- a/packages/layout-engine/painters/dom/src/index.test.ts +++ b/packages/layout-engine/painters/dom/src/index.test.ts @@ -1787,6 +1787,130 @@ describe('DomPainter', () => { const emptySpan = mount.querySelector('.superdoc-line span.superdoc-empty-run') as HTMLElement | null; expect(emptySpan?.dataset.pmStart).toBe('1'); expect(emptySpan?.dataset.pmEnd).toBe('1'); + // Empty-run must set explicit fontSize so it doesn't inherit fontSize:0 from the line + expect(emptySpan?.style.fontSize).toBe('18px'); + }); + + it('sets fallback fontSize on field annotation without explicit fontSize', () => { + const block: FlowBlock = { + kind: 'paragraph', + id: 'fa-no-fontsize', + runs: [ + { + kind: 'fieldAnnotation', + variant: 'text', + displayLabel: 'Client Name', + fieldId: 'F1', + fieldType: 'text', + fieldColor: '#980043', + pmStart: 0, + pmEnd: 1, + }, + ], + }; + const measure: Measure = { + kind: 'paragraph', + lines: [{ fromRun: 0, fromChar: 0, toRun: 0, toChar: 0, width: 100, ascent: 12, descent: 4, lineHeight: 20 }], + totalHeight: 20, + }; + const testLayout: Layout = { + pageSize: layout.pageSize, + pages: [ + { + number: 1, + fragments: [{ kind: 'para', blockId: 'fa-no-fontsize', fromLine: 0, toLine: 1, x: 10, y: 10, width: 200 }], + }, + ], + }; + const painter = createTestPainter({ blocks: [block], measures: [measure] }); + painter.paint(testLayout, mount); + + const annotation = mount.querySelector('.annotation') as HTMLElement | null; + // Must always set fontSize so it doesn't inherit fontSize:0 from the line. + // Falls back to 16px (browser default) when run has no explicit fontSize. + expect(annotation?.style.fontSize).toBe('16px'); + }); + + it('converts numeric fontSize to pt on field annotation', () => { + const block: FlowBlock = { + kind: 'paragraph', + id: 'fa-numeric-fontsize', + runs: [ + { + kind: 'fieldAnnotation', + variant: 'text', + displayLabel: 'Client Name', + fieldId: 'F1', + fieldType: 'text', + fieldColor: '#980043', + fontSize: 14, + pmStart: 0, + pmEnd: 1, + }, + ], + }; + const measure: Measure = { + kind: 'paragraph', + lines: [{ fromRun: 0, fromChar: 0, toRun: 0, toChar: 0, width: 100, ascent: 12, descent: 4, lineHeight: 20 }], + totalHeight: 20, + }; + const testLayout: Layout = { + pageSize: layout.pageSize, + pages: [ + { + number: 1, + fragments: [ + { kind: 'para', blockId: 'fa-numeric-fontsize', fromLine: 0, toLine: 1, x: 10, y: 10, width: 200 }, + ], + }, + ], + }; + const painter = createTestPainter({ blocks: [block], measures: [measure] }); + painter.paint(testLayout, mount); + + const annotation = mount.querySelector('.annotation') as HTMLElement | null; + // Numeric fontSize is converted to pt units. + expect(annotation?.style.fontSize).toBe('14pt'); + }); + + it('sets explicit fontSize on math run wrapper', () => { + const block: FlowBlock = { + kind: 'paragraph', + id: 'math-block', + runs: [ + { + kind: 'math', + ommlJson: {}, + textContent: 'x+1', + width: 40, + height: 14, + pmStart: 0, + pmEnd: 1, + }, + ], + }; + const measure: Measure = { + kind: 'paragraph', + lines: [{ fromRun: 0, fromChar: 0, toRun: 0, toChar: 0, width: 40, ascent: 10, descent: 4, lineHeight: 18 }], + totalHeight: 18, + }; + const testLayout: Layout = { + pageSize: layout.pageSize, + pages: [ + { + number: 1, + fragments: [{ kind: 'para', blockId: 'math-block', fromLine: 0, toLine: 1, x: 10, y: 10, width: 200 }], + }, + ], + }; + const painter = createTestPainter({ blocks: [block], measures: [measure] }); + painter.paint(testLayout, mount); + + const mathWrapper = mount.querySelector('.sd-math') as HTMLElement | null; + // Must set fontSize so fallback text doesn't inherit fontSize:0 from the line. + // Uses browser default (16px) rather than run.height, which would render tall + // expressions at 80–100px for the plain-text fallback path. + expect(mathWrapper?.style.fontSize).toBe('16px'); }); it('renders image fragments', () => { diff --git a/packages/layout-engine/painters/dom/src/renderer-column-separators.test.ts b/packages/layout-engine/painters/dom/src/renderer-column-separators.test.ts new file mode 100644 index 0000000000..6b8c2e27e2 --- /dev/null +++ b/packages/layout-engine/painters/dom/src/renderer-column-separators.test.ts @@ -0,0 +1,247 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { createDomPainter } from './index.js'; +import type { ColumnRegion, Layout, Page } from '@superdoc/contracts'; + +// These tests pin down DomPainter's column-separator rendering: +// - the fallback path (page.columns only, no mid-page regions) +// - the region-aware path (page.columnRegions supersedes page.columns) +// - all five early-return guards inside renderColumnSeparators +// The layout-engine tests cover which data reaches the painter; these tests +// cover what the painter does with it. + +const buildPage = (overrides: Partial = {}): Page => ({ + number: 1, + fragments: [], + margins: { top: 96, right: 96, bottom: 96, left: 96 }, + ...overrides, +}); + +const buildLayout = (page: Page, pageSize = { w: 816, h: 1056 }): Layout => ({ + pageSize, + pages: [page], +}); + +const querySeparators = (mount: HTMLElement): HTMLDivElement[] => { + // Separators are the only 1px-wide absolutely-positioned divs added to a page. + // Scoping by the inline styles keeps this brittle-free against unrelated + // absolute-positioned overlays (rulers, selection, floats). + return Array.from(mount.querySelectorAll('div')).filter((el) => { + const s = el.style; + return s.position === 'absolute' && s.width === '1px' && s.backgroundColor === '#000000'; + }) as HTMLDivElement[]; +}; + +const paintOnce = (layout: Layout, mount: HTMLElement): void => { + const painter = createDomPainter({ blocks: [], measures: [] }); + painter.paint(layout, mount); +}; + +describe('DomPainter renderColumnSeparators', () => { + let mount: HTMLElement; + + beforeEach(() => { + mount = document.createElement('div'); + document.body.appendChild(mount); + }); + + afterEach(() => { + mount.remove(); + }); + + describe('fallback path (page.columns only)', () => { + it('draws a single separator centered in the gap for 2 equal columns', () => { + const page = buildPage({ columns: { count: 2, gap: 48, withSeparator: true } }); + paintOnce(buildLayout(page), mount); + + const seps = querySeparators(mount); + expect(seps).toHaveLength(1); + // pageWidth=816, margins=96 → contentWidth=624, columnWidth=(624-48)/2=288. + // separator x = leftMargin + columnWidth + gap/2 = 96 + 288 + 24 = 408. + expect(seps[0].style.left).toBe('408px'); + expect(seps[0].style.top).toBe('96px'); + // height = pageHeight - top - bottom = 1056 - 96 - 96 = 864. + expect(seps[0].style.height).toBe('864px'); + }); + + it('draws count-1 separators for 3 equal columns', () => { + const page = buildPage({ columns: { count: 3, gap: 48, withSeparator: true } }); + paintOnce(buildLayout(page), mount); + + const seps = querySeparators(mount); + expect(seps).toHaveLength(2); + // columnWidth = (624 - 48*2) / 3 = 176. + // sep 0: 96 + 176 + 48/2 = 296. sep 1: 96 + 2*176 + 48 + 48/2 = 520. + expect(seps.map((s) => s.style.left)).toEqual(['296px', '520px']); + }); + + it('uses explicit column widths when drawing separators for page.columns', () => { + const page = buildPage({ + columns: { count: 2, gap: 48, widths: [200, 952], equalWidth: false, withSeparator: true }, + }); + paintOnce(buildLayout(page), mount); + + const seps = querySeparators(mount); + expect(seps).toHaveLength(1); + // contentWidth=624, availableWidth=576. Explicit widths [200, 952] are + // normalized to [100, 476], so the separator belongs at 96 + 100 + 24 = 220. + expect(seps[0].style.left).toBe('220px'); + }); + + it('renders nothing when withSeparator is false', () => { + const page = buildPage({ columns: { count: 2, gap: 48, withSeparator: false } }); + paintOnce(buildLayout(page), mount); + + expect(querySeparators(mount)).toHaveLength(0); + }); + + it('renders nothing when withSeparator is omitted (undefined)', () => { + const page = buildPage({ columns: { count: 2, gap: 48 } }); + paintOnce(buildLayout(page), mount); + + expect(querySeparators(mount)).toHaveLength(0); + }); + + it('renders nothing for single-column pages', () => { + const page = buildPage({ columns: { count: 1, gap: 0, withSeparator: true } }); + paintOnce(buildLayout(page), mount); + + expect(querySeparators(mount)).toHaveLength(0); + }); + + it('renders nothing when page has neither columns nor columnRegions', () => { + paintOnce(buildLayout(buildPage()), mount); + expect(querySeparators(mount)).toHaveLength(0); + }); + + it('renders nothing when page.margins is missing', () => { + const page: Page = { + number: 1, + fragments: [], + columns: { count: 2, gap: 48, withSeparator: true }, + }; + paintOnce(buildLayout(page), mount); + expect(querySeparators(mount)).toHaveLength(0); + }); + + it('renders nothing when columnWidth collapses to <=1px', () => { + // Pathological case: tiny page with a huge gap leaves no room for columns. + const page = buildPage({ + margins: { top: 10, right: 10, bottom: 10, left: 10 }, + columns: { count: 2, gap: 100, withSeparator: true }, + }); + paintOnce(buildLayout(page, { w: 110, h: 200 }), mount); + // contentWidth=90, columnWidth=(90-100)/2=-5 → guard fires. + expect(querySeparators(mount)).toHaveLength(0); + }); + }); + + describe('region-aware path (page.columnRegions)', () => { + it('draws per-region separators bounded by each region yStart/yEnd', () => { + const regions: ColumnRegion[] = [ + { yStart: 96, yEnd: 400, columns: { count: 2, gap: 48, withSeparator: true } }, + { yStart: 400, yEnd: 700, columns: { count: 3, gap: 48, withSeparator: true } }, + ]; + // page.columns is set to the first region's config (matches what the + // layout engine does); the renderer must prefer columnRegions. + const page = buildPage({ + columns: regions[0].columns, + columnRegions: regions, + }); + paintOnce(buildLayout(page), mount); + + const seps = querySeparators(mount); + // Region 0: 1 separator for 2-col. Region 1: 2 separators for 3-col. + expect(seps).toHaveLength(3); + + // Region 0 bounds. + expect(seps[0].style.top).toBe('96px'); + expect(seps[0].style.height).toBe('304px'); // 400 - 96 + expect(seps[0].style.left).toBe('408px'); + + // Region 1 bounds. + expect(seps[1].style.top).toBe('400px'); + expect(seps[1].style.height).toBe('300px'); // 700 - 400 + expect(seps[2].style.top).toBe('400px'); + expect(seps[2].style.height).toBe('300px'); + // 3-col positions computed fresh for region 1: 296px and 520px. + expect([seps[1].style.left, seps[2].style.left]).toEqual(['296px', '520px']); + }); + + it('skips regions whose withSeparator is false even if other regions render', () => { + const regions: ColumnRegion[] = [ + { yStart: 96, yEnd: 400, columns: { count: 2, gap: 48, withSeparator: true } }, + { yStart: 400, yEnd: 700, columns: { count: 2, gap: 48, withSeparator: false } }, + { yStart: 700, yEnd: 960, columns: { count: 2, gap: 48, withSeparator: true } }, + ]; + const page = buildPage({ columnRegions: regions }); + paintOnce(buildLayout(page), mount); + + const seps = querySeparators(mount); + expect(seps).toHaveLength(2); + // Only regions 0 and 2 produce output. + expect(seps.map((s) => s.style.top)).toEqual(['96px', '700px']); + expect(seps.map((s) => s.style.height)).toEqual(['304px', '260px']); + }); + + it('skips single-column regions', () => { + const regions: ColumnRegion[] = [ + { yStart: 96, yEnd: 400, columns: { count: 1, gap: 0, withSeparator: true } }, + { yStart: 400, yEnd: 700, columns: { count: 2, gap: 48, withSeparator: true } }, + ]; + const page = buildPage({ columnRegions: regions }); + paintOnce(buildLayout(page), mount); + + const seps = querySeparators(mount); + expect(seps).toHaveLength(1); + expect(seps[0].style.top).toBe('400px'); + }); + + it('skips regions with non-positive height', () => { + const regions: ColumnRegion[] = [ + { yStart: 96, yEnd: 96, columns: { count: 2, gap: 48, withSeparator: true } }, + { yStart: 96, yEnd: 500, columns: { count: 2, gap: 48, withSeparator: true } }, + ]; + const page = buildPage({ columnRegions: regions }); + paintOnce(buildLayout(page), mount); + + const seps = querySeparators(mount); + expect(seps).toHaveLength(1); + expect(seps[0].style.height).toBe('404px'); + }); + + it('prefers columnRegions over page.columns when both are present', () => { + // page.columns says "no separator", but columnRegions says "draw one". + // The regions should win — they represent the authoritative per-region + // state, page.columns only represents the page-start config. + const page = buildPage({ + columns: { count: 2, gap: 48, withSeparator: false }, + columnRegions: [{ yStart: 96, yEnd: 960, columns: { count: 2, gap: 48, withSeparator: true } }], + }); + paintOnce(buildLayout(page), mount); + + const seps = querySeparators(mount); + expect(seps).toHaveLength(1); + expect(seps[0].style.top).toBe('96px'); + expect(seps[0].style.height).toBe('864px'); + }); + + it('uses explicit column widths when drawing separators for columnRegions', () => { + const page = buildPage({ + columnRegions: [ + { + yStart: 96, + yEnd: 500, + columns: { count: 2, gap: 48, widths: [200, 952], equalWidth: false, withSeparator: true }, + }, + ], + }); + paintOnce(buildLayout(page), mount); + + const seps = querySeparators(mount); + expect(seps).toHaveLength(1); + expect(seps[0].style.top).toBe('96px'); + expect(seps[0].style.height).toBe('404px'); + expect(seps[0].style.left).toBe('220px'); + }); + }); +}); diff --git a/packages/layout-engine/painters/dom/src/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index 28115ea837..ce6869b26d 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -1,5 +1,6 @@ import type { ChartDrawing, + ColumnLayout, CustomGeometryData, DrawingBlock, DrawingFragment, @@ -61,6 +62,7 @@ import { calculateJustifySpacing, computeLinePmRange, getCellSpacingPx, + normalizeColumnLayout, normalizeBaselineShift, resolveBaseFontSizeForVerticalText, shouldApplyJustify, @@ -84,12 +86,14 @@ import { import { assertFragmentPmPositions, assertPmPositions } from './pm-position-validation.js'; import { createRulerElement, ensureRulerStyles, generateRulerDefinitionFromPx } from './ruler/index.js'; import { + BROWSER_DEFAULT_FONT_SIZE, CLASS_NAMES, containerStyles, containerStylesHorizontal, ensureFieldAnnotationStyles, ensureImageSelectionStyles, ensureLinkStyles, + ensureMathMencloseStyles, ensurePrintStyles, ensureSdtContainerStyles, ensureTrackChangeStyles, @@ -1674,6 +1678,7 @@ export class DomPainter { ensureFieldAnnotationStyles(doc); ensureSdtContainerStyles(doc); ensureImageSelectionStyles(doc); + ensureMathMencloseStyles(doc); if (!this.isSemanticFlow && this.options.ruler?.enabled) { ensureRulerStyles(doc); } @@ -2230,6 +2235,8 @@ export class DomPainter { ); }); this.renderDecorationsForPage(el, page, pageIndex); + this.renderColumnSeparators(el, page, width, height); + return el; } @@ -2310,6 +2317,92 @@ export class DomPainter { } } + private renderColumnSeparators(pageEl: HTMLElement, page: Page, pageWidth: number, pageHeight: number): void { + if (!this.doc) return; + if (!page.margins) return; + + const leftMargin = page.margins.left ?? 0; + const rightMargin = page.margins.right ?? 0; + const topMargin = page.margins.top ?? 0; + const bottomMargin = page.margins.bottom ?? 0; + const contentWidth = pageWidth - leftMargin - rightMargin; + + // Prefer columnRegions (per-region configs for pages with continuous + // section breaks that change column layout mid-page). Fall back to a + // single region derived from page.columns so pages without mid-page + // changes keep working unchanged. + const regions = + page.columnRegions ?? + (page.columns + ? [ + { + yStart: topMargin, + yEnd: pageHeight - bottomMargin, + columns: page.columns, + }, + ] + : []); + + for (const region of regions) { + const { columns, yStart, yEnd } = region; + if (!columns.withSeparator) continue; + if (columns.count <= 1) continue; + + const regionHeight = yEnd - yStart; + if (regionHeight <= 0) continue; + + const separatorPositions = this.getColumnSeparatorPositions(columns, leftMargin, contentWidth); + if (separatorPositions.length === 0) continue; + + for (const separatorX of separatorPositions) { + const separatorEl = this.doc.createElement('div'); + + separatorEl.style.position = 'absolute'; + separatorEl.style.left = `${separatorX}px`; + separatorEl.style.top = `${yStart}px`; + separatorEl.style.height = `${regionHeight}px`; + separatorEl.style.width = '1px'; + separatorEl.style.backgroundColor = '#000000'; + separatorEl.style.pointerEvents = 'none'; + pageEl.appendChild(separatorEl); + } + } + } + + private getColumnSeparatorPositions(columns: ColumnLayout, leftMargin: number, contentWidth: number): number[] { + const hasExplicitWidths = Array.isArray(columns.widths) && columns.widths.length > 0; + + if (!hasExplicitWidths) { + const equalWidth = (contentWidth - columns.gap * (columns.count - 1)) / columns.count; + if (equalWidth <= 1) return []; + + const separatorPositions: number[] = []; + for (let index = 0; index < columns.count - 1; index += 1) { + separatorPositions.push(leftMargin + (index + 1) * equalWidth + index * columns.gap + columns.gap / 2); + } + return separatorPositions; + } + + const normalizedColumns = normalizeColumnLayout(columns, contentWidth); + if (normalizedColumns.count <= 1) return []; + + const columnWidths = + normalizedColumns.widths ?? Array.from({ length: normalizedColumns.count }, () => normalizedColumns.width); + // A 1px separator only makes sense when every participating column is wider than the separator itself. + if (columnWidths.some((columnWidth) => columnWidth <= 1)) return []; + + const separatorPositions: number[] = []; + let cursorX = leftMargin; + + for (let index = 0; index < normalizedColumns.count - 1; index += 1) { + const currentColumnWidth = columnWidths[index] ?? normalizedColumns.width; + separatorPositions.push(cursorX + currentColumnWidth + normalizedColumns.gap / 2); + cursorX += currentColumnWidth + normalizedColumns.gap; + } + + return separatorPositions; + } + private renderDecorationsForPage(pageEl: HTMLElement, page: Page, pageIndex: number): void { if (this.isSemanticFlow) return; this.renderDecorationSection(pageEl, page, pageIndex, 'header'); @@ -2816,6 +2909,8 @@ export class DomPainter { }); this.renderDecorationsForPage(el, page, pageIndex); + this.renderColumnSeparators(el, page, pageSize.w, pageSize.h); + return { element: el, fragments: fragmentStates }; } @@ -5170,6 +5265,12 @@ export class DomPainter { // Let browser auto-size to MathML content; estimated dimensions are for layout only wrapper.style.minWidth = `${run.width}px`; wrapper.style.minHeight = `${run.height}px`; + // Restore font-size so the plain-text fallback renders at a reasonable size + // (the line container sets fontSize: 0 to eliminate the CSS strut). MathML + // has its own internal scaling, so this only matters for the textContent + // fallback path. run.height would make tall expressions (fractions, equation + // arrays) render at 80–100px — use the browser default instead. + wrapper.style.fontSize = BROWSER_DEFAULT_FONT_SIZE; wrapper.dataset.layoutEpoch = String(this.layoutEpoch ?? 0); const mathEl = convertOmmlToMathml(run.ommlJson, this.doc); @@ -5442,6 +5543,7 @@ export class DomPainter { // Position and z-index on the image only (not the line) so resize overlay can stack above. img.style.position = 'relative'; img.style.zIndex = '1'; + img.style.maxWidth = '100%'; } // Apply rotation and flip transforms from OOXML a:xfrm @@ -5666,12 +5768,20 @@ export class DomPainter { } } - // Apply typography to the annotation element + // Apply typography to the annotation element. + // Always set a font-size so the annotation never inherits fontSize: 0 from + // the line container (which zeroes it to eliminate the CSS strut). When the + // run has no explicit fontSize, fall back to BROWSER_DEFAULT_FONT_SIZE (the + // browser default that was previously inherited before the strut fix). if (run.fontFamily) { annotation.style.fontFamily = run.fontFamily; } - if (run.fontSize) { - const fontSize = typeof run.fontSize === 'number' ? `${run.fontSize}pt` : run.fontSize; + { + const fontSize = run.fontSize + ? typeof run.fontSize === 'number' + ? `${run.fontSize}pt` + : run.fontSize + : BROWSER_DEFAULT_FONT_SIZE; annotation.style.fontSize = fontSize; } if (run.textColor) { @@ -5875,6 +5985,9 @@ export class DomPainter { if (lineRange.pmEnd != null) { span.dataset.pmEnd = String(lineRange.pmEnd); } + // Restore font-size so the   remains a visible caret target + // (the line container sets fontSize: 0 to eliminate the CSS strut). + span.style.fontSize = `${line.lineHeight}px`; span.innerHTML = ' '; el.appendChild(span); } diff --git a/packages/layout-engine/painters/dom/src/styles.test.ts b/packages/layout-engine/painters/dom/src/styles.test.ts index d1170bf9f4..bcdd59e772 100644 --- a/packages/layout-engine/painters/dom/src/styles.test.ts +++ b/packages/layout-engine/painters/dom/src/styles.test.ts @@ -1,5 +1,18 @@ import { describe, expect, it } from 'vitest'; -import { ensureSdtContainerStyles } from './styles.js'; +import { ensureSdtContainerStyles, lineStyles } from './styles.js'; + +describe('lineStyles', () => { + it('sets height and lineHeight from the argument', () => { + const styles = lineStyles(24); + expect(styles.height).toBe('24px'); + expect(styles.lineHeight).toBe('24px'); + }); + + it('sets fontSize to 0 to eliminate the CSS strut', () => { + const styles = lineStyles(20); + expect(styles.fontSize).toBe('0'); + }); +}); describe('ensureSdtContainerStyles', () => { it('suppresses structured-content hover backgrounds in viewing mode, including grouped hover', () => { diff --git a/packages/layout-engine/painters/dom/src/styles.ts b/packages/layout-engine/painters/dom/src/styles.ts index 2f8cb1e498..b548d37107 100644 --- a/packages/layout-engine/painters/dom/src/styles.ts +++ b/packages/layout-engine/painters/dom/src/styles.ts @@ -1,5 +1,12 @@ import { DOM_CLASS_NAMES } from './constants.js'; +/** + * Fallback font-size applied to child elements inside a line container that + * carry no explicit fontSize. Matches the browser default so rendering is + * preserved after the strut-elimination fix (fontSize: '0' on lines). + */ +export const BROWSER_DEFAULT_FONT_SIZE = '16px'; + export const CLASS_NAMES = { container: 'superdoc-layout', page: 'superdoc-page', @@ -90,6 +97,15 @@ export const fragmentStyles: Partial = { export const lineStyles = (lineHeight: number): Partial => ({ lineHeight: `${lineHeight}px`, height: `${lineHeight}px`, + // Eliminate the CSS "strut" created by the inherited font-size (typically + // the browser default 16px). Without this, the strut shifts normal-flow + // inline children down via baseline alignment, while absolutely-positioned + // children (used for tab-aligned segments) are unaffected — causing + // tab-indented first lines to appear shifted up relative to continuation + // lines. All text-bearing child elements set their own explicit font-size; + // elements that don't (empty-run, math wrapper, field annotation wrapper) + // are patched individually in renderer.ts. + fontSize: '0', position: 'relative', display: 'block', whiteSpace: 'pre', @@ -630,12 +646,85 @@ const IMAGE_SELECTION_STYLES = ` } `; +const MATH_MENCLOSE_STYLES = ` +/* MathML polyfill. + * + * MathML 3 defined with borders, strikes, and other + * enclosure notations. MathML Core (the subset shipped in Chrome 109+, 2023) + * dropped — the WG moved its rendering to CSS/SVG. Firefox and + * WebKit also do not paint it. Without this polyfill, m:borderBox content + * imports correctly (the notation attribute is right) but renders invisibly. + * + * Each notation token is composable: "box horizontalstrike" draws the box + * border and a horizontal strike together. Diagonal strikes layer through + * CSS custom properties so X patterns (both diagonals) stack correctly. + * + * @spec MathML 3 §3.3.8 menclose + */ +menclose { + display: inline-block; + position: relative; + padding: 0.15em 0.25em; + + --sd-menclose-stroke: currentColor; + --sd-menclose-h: none; + --sd-menclose-v: none; + --sd-menclose-up: none; + --sd-menclose-down: none; +} + +menclose[notation~="box"] { border: 1px solid var(--sd-menclose-stroke); } +menclose[notation~="roundedbox"] { border: 1px solid var(--sd-menclose-stroke); border-radius: 0.3em; } +menclose[notation~="top"] { border-top: 1px solid var(--sd-menclose-stroke); } +menclose[notation~="bottom"] { border-bottom: 1px solid var(--sd-menclose-stroke); } +menclose[notation~="left"] { border-left: 1px solid var(--sd-menclose-stroke); } +menclose[notation~="right"] { border-right: 1px solid var(--sd-menclose-stroke); } + +menclose[notation~="horizontalstrike"] { + --sd-menclose-h: linear-gradient(var(--sd-menclose-stroke), var(--sd-menclose-stroke)) no-repeat center / 100% 1px; +} +menclose[notation~="verticalstrike"] { + --sd-menclose-v: linear-gradient(var(--sd-menclose-stroke), var(--sd-menclose-stroke)) no-repeat center / 1px 100%; +} +/* Gradient direction is perpendicular to the stripe it produces. + * "to bottom right" → stripe runs bottom-left → top-right (visually "/") = updiagonalstrike. + * "to top right" → stripe runs top-left → bottom-right (visually "\") = downdiagonalstrike. + */ +menclose[notation~="updiagonalstrike"] { + --sd-menclose-up: linear-gradient( + to bottom right, + transparent calc(50% - 0.5px), + var(--sd-menclose-stroke) calc(50% - 0.5px), + var(--sd-menclose-stroke) calc(50% + 0.5px), + transparent calc(50% + 0.5px) + ); +} +menclose[notation~="downdiagonalstrike"] { + --sd-menclose-down: linear-gradient( + to top right, + transparent calc(50% - 0.5px), + var(--sd-menclose-stroke) calc(50% - 0.5px), + var(--sd-menclose-stroke) calc(50% + 0.5px), + transparent calc(50% + 0.5px) + ); +} + +menclose::after { + content: ""; + position: absolute; + inset: 0; + pointer-events: none; + background: var(--sd-menclose-h), var(--sd-menclose-v), var(--sd-menclose-up), var(--sd-menclose-down); +} +`; + let printStylesInjected = false; let linkStylesInjected = false; let trackChangeStylesInjected = false; let sdtContainerStylesInjected = false; let fieldAnnotationStylesInjected = false; let imageSelectionStylesInjected = false; +let mathMencloseStylesInjected = false; export const ensurePrintStyles = (doc: Document | null | undefined) => { if (printStylesInjected || !doc) return; @@ -696,3 +785,17 @@ export const ensureImageSelectionStyles = (doc: Document | null | undefined) => doc.head?.appendChild(styleEl); imageSelectionStylesInjected = true; }; + +/** + * Injects the MathML polyfill into the document head. Required + * because no browser paints menclose natively (MathML Core dropped it). See + * MATH_MENCLOSE_STYLES for the full rationale. + */ +export const ensureMathMencloseStyles = (doc: Document | null | undefined) => { + if (mathMencloseStylesInjected || !doc) return; + const styleEl = doc.createElement('style'); + styleEl.setAttribute('data-superdoc-math-menclose-styles', 'true'); + styleEl.textContent = MATH_MENCLOSE_STYLES; + doc.head?.appendChild(styleEl); + mathMencloseStylesInjected = true; +}; diff --git a/packages/layout-engine/painters/dom/src/table/renderTableCell.test.ts b/packages/layout-engine/painters/dom/src/table/renderTableCell.test.ts index 6836c4319d..b8a44538dd 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableCell.test.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableCell.test.ts @@ -1454,7 +1454,7 @@ describe('renderTableCell', () => { const lineEl = paraWrapper.firstElementChild as HTMLElement; const markerEl = lineEl.querySelector('.superdoc-paragraph-marker') as HTMLElement; - expect(markerEl.style.fontFamily).toBe('"Times New Roman", sans-serif'); + expect(markerEl.style.fontFamily).toBe('"Times New Roman", serif'); expect(markerEl.style.fontSize).toBe('18px'); expect(markerEl.style.fontWeight).toBe('bold'); expect(markerEl.style.fontStyle).toBe('italic'); diff --git a/packages/layout-engine/pm-adapter/src/attributes/borders.ts b/packages/layout-engine/pm-adapter/src/attributes/borders.ts index c1a78ba32f..3977700118 100644 --- a/packages/layout-engine/pm-adapter/src/attributes/borders.ts +++ b/packages/layout-engine/pm-adapter/src/attributes/borders.ts @@ -324,6 +324,11 @@ export function extractCellPadding(cellAttrs: Record): BoxSpaci export const normalizeParagraphBorders = (value: unknown): ParagraphAttrs['borders'] | undefined => { if (!value || typeof value !== 'object') return undefined; const source = value as Record; + // Note: w:bar is intentionally not in this list. We tested in Word and it + // never draws w:bar on screen — it just keeps the value in the file when + // saving. The spec lets apps skip it, and Word does. SuperDoc does too, by + // default. If you have a real document where bar needs to be drawn, open + // an issue with the use case before adding 'bar' here. const sides: Array<'top' | 'right' | 'bottom' | 'left' | 'between'> = ['top', 'right', 'bottom', 'left', 'between']; const borders: ParagraphAttrs['borders'] = {}; diff --git a/packages/layout-engine/pm-adapter/src/converters/paragraph.test.ts b/packages/layout-engine/pm-adapter/src/converters/paragraph.test.ts index c701136ccd..dfa44dbf3c 100644 --- a/packages/layout-engine/pm-adapter/src/converters/paragraph.test.ts +++ b/packages/layout-engine/pm-adapter/src/converters/paragraph.test.ts @@ -119,7 +119,7 @@ import { const DEFAULT_HYPERLINK_CONFIG: HyperlinkConfig = { enableRichHyperlinks: false }; const DEFAULT_TEST_FONT_FAMILY = 'Arial, sans-serif'; const DEFAULT_TEST_FONT_SIZE_PX = (16 * 96) / 72; -const FALLBACK_FONT_FAMILY = 'Times New Roman, sans-serif'; +const FALLBACK_FONT_FAMILY = 'Times New Roman, serif'; const FALLBACK_FONT_SIZE_PX = 12; let defaultConverterContext: ConverterContext = { translatedNumbering: {}, diff --git a/packages/layout-engine/pm-adapter/src/index.test.ts b/packages/layout-engine/pm-adapter/src/index.test.ts index 415dbb87b2..4f387dd52b 100644 --- a/packages/layout-engine/pm-adapter/src/index.test.ts +++ b/packages/layout-engine/pm-adapter/src/index.test.ts @@ -70,7 +70,7 @@ describe('toFlowBlocks', () => { runs: [ { text: 'Hello world', - fontFamily: 'Times New Roman, sans-serif', + fontFamily: 'Times New Roman, serif', }, ], }); @@ -114,7 +114,7 @@ describe('toFlowBlocks', () => { }); expect(blocks[0].runs[0]).toMatchObject({ - fontFamily: 'Times New Roman, sans-serif', + fontFamily: 'Times New Roman, serif', }); expect(blocks[0].runs[0]?.fontSize).toBeCloseTo(14, 5); }); @@ -903,6 +903,7 @@ describe('toFlowBlocks', () => { expect((contentBreak as FlowBlock).columns).toEqual({ count: 2, gap: 101.53333333333333, + withSeparator: false, widths: [72, 497.26666666666665], equalWidth: false, }); @@ -1077,7 +1078,7 @@ describe('toFlowBlocks', () => { expect(multiColumnBreak).toBeDefined(); expect((multiColumnBreak as FlowBlock).attrs?.requirePageBoundary).toBeUndefined(); // Gap is in pixels (0.5in = 48px @96DPI) - expect((multiColumnBreak as FlowBlock).columns).toEqual({ count: 2, gap: 48 }); + expect((multiColumnBreak as FlowBlock).columns).toEqual({ count: 2, gap: 48, withSeparator: false }); }); it('interprets missing w:num in w:cols as a single-column layout change', () => { @@ -1110,7 +1111,7 @@ describe('toFlowBlocks', () => { const allBreaks = getSectionBreaks(blocks, { includeFirst: true }); const tailBreak = allBreaks.find((b) => b.attrs?.sectionIndex === 0); expect(tailBreak).toBeDefined(); - expect((tailBreak as never).columns).toEqual({ count: 1, gap: 48 }); + expect((tailBreak as never).columns).toEqual({ count: 1, gap: 48, withSeparator: false }); }); describe('Regression tests for section property bug fixes', () => { @@ -1158,7 +1159,7 @@ describe('toFlowBlocks', () => { expect(firstBreak).toBeDefined(); expect(secondBreak).toBeDefined(); // Both have w:space="720" which means single column - expect((firstBreak as FlowBlock).columns).toEqual({ count: 1, gap: 48 }); + expect((firstBreak as FlowBlock).columns).toEqual({ count: 1, gap: 48, withSeparator: false }); expect((secondBreak as FlowBlock).type).toBe('continuous'); // Second sectPr }); @@ -1198,7 +1199,7 @@ describe('toFlowBlocks', () => { // Should emit the section break despite paragraph having content expect(contentBreak).toBeDefined(); - expect((contentBreak as FlowBlock).columns).toEqual({ count: 2, gap: 48 }); + expect((contentBreak as FlowBlock).columns).toEqual({ count: 2, gap: 48, withSeparator: false }); }); it('detects column changes from single to multi to single column', () => { diff --git a/packages/layout-engine/pm-adapter/src/integration.test.ts b/packages/layout-engine/pm-adapter/src/integration.test.ts index e239d81850..065b52b128 100644 --- a/packages/layout-engine/pm-adapter/src/integration.test.ts +++ b/packages/layout-engine/pm-adapter/src/integration.test.ts @@ -19,6 +19,7 @@ import twoColumnFixture from './fixtures/two-column-two-page.json'; import tabsDecimalFixture from './fixtures/tabs-decimal.json'; import tabsCenterEndFixture from './fixtures/tabs-center-end.json'; import paragraphPPrVariationsFixture from './fixtures/paragraph_pPr_variations.json'; +import { twipsToPx } from './utilities.js'; const DEFAULT_CONVERTER_CONTEXT = { docx: {}, @@ -292,7 +293,12 @@ describe('PM → FlowBlock → Measure integration', () => { const decimalMeasure = expectParagraphMeasure(await measureBlock(blocks[0], 400)); const controlMeasure = expectParagraphMeasure(await measureBlock(controlBlocks[0], 400)); - expect(decimalMeasure.lines[0].width).toBeLessThanOrEqual(controlMeasure.lines[0].width); + const rightAlignedStopTwips = blocks[0].attrs?.tabs?.find((stop) => stop.val === 'end')?.pos; + if (typeof rightAlignedStopTwips === 'number') { + expect(decimalMeasure.lines[0].width).toBeCloseTo(twipsToPx(rightAlignedStopTwips), 2); + } + // Decimal-aligned measurement should reserve at least as much width as the control case + expect(decimalMeasure.lines[0].width).toBeGreaterThanOrEqual(controlMeasure.lines[0].width); }); it('derives default decimal separator from document language when not explicitly set', async () => { diff --git a/packages/layout-engine/pm-adapter/src/sections/analysis.test.ts b/packages/layout-engine/pm-adapter/src/sections/analysis.test.ts index 37c60496b5..ac860ce7e6 100644 --- a/packages/layout-engine/pm-adapter/src/sections/analysis.test.ts +++ b/packages/layout-engine/pm-adapter/src/sections/analysis.test.ts @@ -756,7 +756,7 @@ describe('analysis', () => { footerPx: 50, pageSizePx: { w: 12240, h: 15840 }, orientation: 'landscape', - columnsPx: { count: 2, gap: 100 }, + columnsPx: { count: 2, gap: 100, withSeparator: false }, headerRefs: { default: 'header1' }, footerRefs: { default: 'footer1' }, numbering: { format: 'decimal', start: 1 }, @@ -770,7 +770,7 @@ describe('analysis', () => { margins: { header: 100, footer: 50 }, pageSize: { w: 12240, h: 15840 }, orientation: 'landscape', - columns: { count: 2, gap: 100 }, + columns: { count: 2, gap: 100, withSeparator: false }, headerRefs: { default: 'header1' }, footerRefs: { default: 'footer1' }, numbering: { format: 'decimal', start: 1 }, @@ -962,6 +962,32 @@ describe('analysis', () => { expect(result!.numbering).toEqual({ format: 'decimal', start: 5 }); }); + it('should have column separator flag set to true when present in extracted data', () => { + const bodySectPr: SectPrElement = { type: 'element', name: 'w:sectPr' }; + + vi.mocked(extractionModule.extractSectionData).mockReturnValue({ + titlePg: false, + columnsPx: { count: 2, gap: 48, withSeparator: true }, + }); + + const result = createFinalSectionFromBodySectPr(bodySectPr, 0, 10, 0); + + expect(result!.columns).toEqual({ count: 2, gap: 48, withSeparator: true }); + }); + + it('should have column separator flag set to false when present as "false" in extracted data', () => { + const bodySectPr: SectPrElement = { type: 'element', name: 'w:sectPr' }; + + vi.mocked(extractionModule.extractSectionData).mockReturnValue({ + titlePg: false, + columnsPx: { count: 2, gap: 48, withSeparator: false }, + }); + + const result = createFinalSectionFromBodySectPr(bodySectPr, 0, 10, 0); + + expect(result!.columns).toEqual({ count: 2, gap: 48, withSeparator: false }); + }); + it('should respect body section type from extracted data', () => { const bodySectPr: SectPrElement = { type: 'element', name: 'w:sectPr' }; diff --git a/packages/layout-engine/pm-adapter/src/sections/extraction.test.ts b/packages/layout-engine/pm-adapter/src/sections/extraction.test.ts index 7f3fa6beec..ce6a124e96 100644 --- a/packages/layout-engine/pm-adapter/src/sections/extraction.test.ts +++ b/packages/layout-engine/pm-adapter/src/sections/extraction.test.ts @@ -250,6 +250,7 @@ describe('extraction', () => { expect(result?.columnsPx).toEqual({ count: 2, gap: 48, // 720 twips = 0.5 inches = 48 pixels + withSeparator: false, }); }); @@ -281,6 +282,7 @@ describe('extraction', () => { expect(result?.columnsPx).toEqual({ count: 2, gap: 101.53333333333333, + withSeparator: false, widths: [72, 497.26666666666665], equalWidth: false, }); @@ -374,6 +376,129 @@ describe('extraction', () => { }); }); + // ==================== extractSectionData - column separator (w:sep) tests ==================== + describe('extractSectionData - column separator', () => { + it('should include separator when w:sep="1"', () => { + const para: PMNode = { + type: 'paragraph', + attrs: { + paragraphProperties: { + sectPr: { + type: 'element', + name: 'w:sectPr', + elements: [ + { + name: 'w:cols', + attributes: { 'w:num': '2', 'w:space': '720', 'w:sep': '1' }, + }, + ], + }, + }, + }, + }; + const result = extractSectionData(para); + + expect(result).not.toBeNull(); + expect(result?.columnsPx).toEqual({ + count: 2, + gap: 48, + withSeparator: true, + }); + }); + + it('should include separator when w:sep="true"', () => { + const para: PMNode = { + type: 'paragraph', + attrs: { + paragraphProperties: { + sectPr: { + type: 'element', + name: 'w:sectPr', + elements: [ + { + name: 'w:cols', + attributes: { 'w:num': '2', 'w:space': '720', 'w:sep': 'true' }, + }, + ], + }, + }, + }, + }; + const result = extractSectionData(para); + + expect(result?.columnsPx?.withSeparator).toBe(true); + }); + + it('should include separator when w:sep="on"', () => { + const para: PMNode = { + type: 'paragraph', + attrs: { + paragraphProperties: { + sectPr: { + type: 'element', + name: 'w:sectPr', + elements: [ + { + name: 'w:cols', + attributes: { 'w:num': '2', 'w:space': '720', 'w:sep': 'on' }, + }, + ], + }, + }, + }, + }; + const result = extractSectionData(para); + + expect(result?.columnsPx?.withSeparator).toBe(true); + }); + + it('should not include separator when w:sep is absent', () => { + const para: PMNode = { + type: 'paragraph', + attrs: { + paragraphProperties: { + sectPr: { + type: 'element', + name: 'w:sectPr', + elements: [ + { + name: 'w:cols', + attributes: { 'w:num': '2', 'w:space': '720' }, + }, + ], + }, + }, + }, + }; + const result = extractSectionData(para); + + expect(result?.columnsPx).toEqual({ count: 2, gap: 48, withSeparator: false }); + }); + + it('should not include separator when w:sep="0"', () => { + const para: PMNode = { + type: 'paragraph', + attrs: { + paragraphProperties: { + sectPr: { + type: 'element', + name: 'w:sectPr', + elements: [ + { + name: 'w:cols', + attributes: { 'w:num': '2', 'w:space': '720', 'w:sep': '0' }, + }, + ], + }, + }, + }, + }; + const result = extractSectionData(para); + + expect(result?.columnsPx).toEqual({ count: 2, gap: 48, withSeparator: false }); + }); + }); + // ==================== parseColumnGap Tests ==================== describe('parseColumnGap', () => { it('should return default 0.5 inches when gapTwips is undefined', () => { diff --git a/packages/layout-engine/pm-adapter/src/sections/extraction.ts b/packages/layout-engine/pm-adapter/src/sections/extraction.ts index ca2e9979ef..da98daeec3 100644 --- a/packages/layout-engine/pm-adapter/src/sections/extraction.ts +++ b/packages/layout-engine/pm-adapter/src/sections/extraction.ts @@ -6,6 +6,7 @@ import type { PMNode } from '../types.js'; import type { ParagraphProperties, SectionVerticalAlign } from './types.js'; +import type { ColumnLayout } from '@superdoc/contracts'; const TWIPS_PER_INCH = 1440; const PX_PER_INCH = 96; @@ -42,6 +43,15 @@ export function parseColumnGap(gapTwips: string | number | undefined): number { return Number.isFinite(gap) ? gap / TWIPS_PER_INCH : DEFAULT_COLUMN_GAP_INCHES; } +/** + * Parse presence of column separator from w:sep attribute (can be '1', 'true' or 'on'). + * @param rawValue - Raw value from w:sep attribute + * @returns Presence of column separator + */ +export function parseColumnSeparator(rawValue: string | number | undefined): boolean { + return rawValue === '1' || rawValue === 'true' || rawValue === 'on' || rawValue === 1; +} + type SectionType = 'continuous' | 'nextPage' | 'evenPage' | 'oddPage'; type Orientation = 'portrait' | 'landscape'; type HeaderRefType = Partial>; @@ -209,13 +219,12 @@ function extractPageNumbering(elements: SectionElement[]): /** * Extract columns from element. */ -function extractColumns( - elements: SectionElement[], -): { count: number; gap: number; widths?: number[]; equalWidth?: boolean } | undefined { +function extractColumns(elements: SectionElement[]): ColumnLayout | undefined { const cols = elements.find((el) => el?.name === 'w:cols'); if (!cols?.attributes) return undefined; const count = parseColumnCount(cols.attributes['w:num'] as string | number | undefined); + const withSeparator = parseColumnSeparator(cols.attributes['w:sep'] as string | number | undefined); const equalWidthRaw = cols.attributes['w:equalWidth']; const equalWidth = equalWidthRaw === '0' || equalWidthRaw === 0 || equalWidthRaw === false @@ -233,9 +242,10 @@ function extractColumns( .filter((widthTwips) => Number.isFinite(widthTwips) && widthTwips > 0) .map((widthTwips) => (widthTwips / 1440) * PX_PER_INCH); - const result = { + const result: ColumnLayout = { count, gap: gapInches * PX_PER_INCH, + withSeparator, ...(widths.length > 0 ? { widths } : {}), ...(equalWidth !== undefined ? { equalWidth } : {}), }; @@ -308,7 +318,7 @@ export function extractSectionData(para: PMNode): { type?: SectionType; pageSizePx?: { w: number; h: number }; orientation?: Orientation; - columnsPx?: { count: number; gap: number; widths?: number[]; equalWidth?: boolean }; + columnsPx?: ColumnLayout; titlePg?: boolean; headerRefs?: HeaderRefType; footerRefs?: HeaderRefType; diff --git a/packages/layout-engine/pm-adapter/src/sections/index.ts b/packages/layout-engine/pm-adapter/src/sections/index.ts index b21f849c6a..64b41423fb 100644 --- a/packages/layout-engine/pm-adapter/src/sections/index.ts +++ b/packages/layout-engine/pm-adapter/src/sections/index.ts @@ -17,7 +17,7 @@ export type { export { SectionType, DEFAULT_PARAGRAPH_SECTION_TYPE, DEFAULT_BODY_SECTION_TYPE } from './types.js'; // Extraction -export { extractSectionData, parseColumnCount, parseColumnGap } from './extraction.js'; +export { extractSectionData, parseColumnCount, parseColumnGap, parseColumnSeparator } from './extraction.js'; // Analysis export { diff --git a/packages/layout-engine/pm-adapter/src/sections/types.ts b/packages/layout-engine/pm-adapter/src/sections/types.ts index bc8498ffed..a4134b48d0 100644 --- a/packages/layout-engine/pm-adapter/src/sections/types.ts +++ b/packages/layout-engine/pm-adapter/src/sections/types.ts @@ -5,6 +5,8 @@ * Includes section ranges, signatures, and OOXML structures. */ +import type { ColumnLayout } from '@superdoc/contracts'; + /** * Section types in Word documents. * Controls how section breaks create new pages. @@ -72,7 +74,7 @@ export type SectionSignature = { orientation?: 'portrait' | 'landscape'; headerRefs?: Partial>; footerRefs?: Partial>; - columnsPx?: { count: number; gap: number; widths?: number[]; equalWidth?: boolean }; + columnsPx?: ColumnLayout; numbering?: { format?: 'decimal' | 'lowerLetter' | 'upperLetter' | 'lowerRoman' | 'upperRoman' | 'numberInDash'; start?: number; @@ -105,7 +107,7 @@ export interface SectionRange { } | null; pageSize: { w: number; h: number } | null; orientation: 'portrait' | 'landscape' | null; - columns: { count: number; gap: number; widths?: number[]; equalWidth?: boolean } | null; + columns: ColumnLayout | null; type: SectionType; titlePg: boolean; headerRefs?: Partial>; diff --git a/packages/react/src/SuperDocEditor.test.tsx b/packages/react/src/SuperDocEditor.test.tsx index 6c469273ea..b944661d85 100644 --- a/packages/react/src/SuperDocEditor.test.tsx +++ b/packages/react/src/SuperDocEditor.test.tsx @@ -117,6 +117,46 @@ describe('SuperDocEditor', () => { { timeout: 5000 }, ); }); + + it('should route onTransaction through the latest callback after rerender', async () => { + const ref = createRef(); + const onReady = vi.fn(); + const firstOnTransaction = vi.fn(); + const secondOnTransaction = vi.fn(); + + const { rerender } = render(); + + await waitFor(() => expect(onReady).toHaveBeenCalled(), { timeout: 5000 }); + + const instance = ref.current?.getInstance(); + expect(instance).toBeTruthy(); + + const transactionEvent = { + editor: {}, + sourceEditor: {}, + transaction: { docChanged: true }, + surface: 'body', + }; + + const firstCallCountBeforeManualDispatch = firstOnTransaction.mock.calls.length; + (instance as any).config.onTransaction(transactionEvent); + + expect(firstOnTransaction).toHaveBeenLastCalledWith(transactionEvent); + expect(firstOnTransaction).toHaveBeenCalledTimes(firstCallCountBeforeManualDispatch + 1); + expect(secondOnTransaction).not.toHaveBeenCalled(); + + rerender(); + + expect(ref.current?.getInstance()).toBe(instance); + + const firstCallCountBeforeRerenderDispatch = firstOnTransaction.mock.calls.length; + const secondCallCountBeforeManualDispatch = secondOnTransaction.mock.calls.length; + (instance as any).config.onTransaction(transactionEvent); + + expect(firstOnTransaction).toHaveBeenCalledTimes(firstCallCountBeforeRerenderDispatch); + expect(secondOnTransaction).toHaveBeenLastCalledWith(transactionEvent); + expect(secondOnTransaction).toHaveBeenCalledTimes(secondCallCountBeforeManualDispatch + 1); + }); }); describe('onEditorDestroy', () => { @@ -175,6 +215,213 @@ describe('SuperDocEditor', () => { }); }); + describe('prop stability (SD-2635)', () => { + it('does not destroy/re-init when user prop is a new object literal with identical content', async () => { + const ref = createRef(); + const onReady = vi.fn(); + const onEditorDestroy = vi.fn(); + + const { rerender } = render( + , + ); + + await waitFor(() => expect(onReady).toHaveBeenCalled(), { timeout: 5000 }); + const instanceBefore = ref.current?.getInstance(); + expect(instanceBefore).toBeTruthy(); + + // Re-render with a *new* object literal carrying the same content — + // this is the idiomatic React pattern that used to trigger a full + // destroy + re-init loop before SD-2635. + rerender( + , + ); + + // Same underlying instance proves no destroy+rebuild happened. + expect(ref.current?.getInstance()).toBe(instanceBefore); + expect(onEditorDestroy).not.toHaveBeenCalled(); + }); + + it('does not destroy/re-init when users prop is a new array literal with identical content', async () => { + const ref = createRef(); + const onReady = vi.fn(); + const onEditorDestroy = vi.fn(); + + const { rerender } = render( + , + ); + + await waitFor(() => expect(onReady).toHaveBeenCalled(), { timeout: 5000 }); + const instanceBefore = ref.current?.getInstance(); + + rerender( + , + ); + + expect(ref.current?.getInstance()).toBe(instanceBefore); + expect(onEditorDestroy).not.toHaveBeenCalled(); + }); + + it('rebuilds and remounts a new instance when user prop value actually changes', async () => { + const ref = createRef(); + const onReady = vi.fn(); + const onEditorDestroy = vi.fn(); + + const { rerender } = render( + , + ); + + await waitFor(() => expect(onReady).toHaveBeenCalled(), { timeout: 5000 }); + const instanceBefore = ref.current?.getInstance(); + + rerender( + , + ); + + // Old instance torn down, new instance ready. + await waitFor(() => expect(onEditorDestroy).toHaveBeenCalled(), { timeout: 5000 }); + await waitFor(() => expect(onReady).toHaveBeenCalledTimes(2), { timeout: 5000 }); + expect(ref.current?.getInstance()).not.toBe(instanceBefore); + }); + + it('stays stable under StrictMode double-invocation on rerender', async () => { + const ref = createRef(); + const onReady = vi.fn(); + const onEditorDestroy = vi.fn(); + + const { rerender } = render( + + + , + ); + + await waitFor(() => expect(onReady).toHaveBeenCalled(), { timeout: 5000 }); + const instanceBefore = ref.current?.getInstance(); + const destroysBefore = onEditorDestroy.mock.calls.length; + + rerender( + + + , + ); + + expect(ref.current?.getInstance()).toBe(instanceBefore); + expect(onEditorDestroy.mock.calls.length).toBe(destroysBefore); + }); + + it('still rebuilds under StrictMode when user prop value actually changes', async () => { + // The same-content StrictMode test above proves memoization survives + // double-invocation. This test proves the positive path — a real + // value change under StrictMode still tears down and remounts. + const ref = createRef(); + const onReady = vi.fn(); + const onEditorDestroy = vi.fn(); + + const { rerender } = render( + + + , + ); + + await waitFor(() => expect(onReady).toHaveBeenCalled(), { timeout: 5000 }); + const instanceBefore = ref.current?.getInstance(); + + rerender( + + + , + ); + + await waitFor(() => expect(onEditorDestroy).toHaveBeenCalled(), { timeout: 5000 }); + await waitFor(() => expect(ref.current?.getInstance()).not.toBe(instanceBefore), { timeout: 5000 }); + }); + + it('rebuilds when a new modules object is passed, even if content looks equal', async () => { + // `modules` is intentionally kept on reference identity in the dep + // array because it can carry functions and live objects that a + // structural compare would miss. This test pins that contract — + // if a future refactor wraps `modules` in useStructuralMemo, this + // test will fail and flag the regression. + const ref = createRef(); + const onReady = vi.fn(); + const onEditorDestroy = vi.fn(); + + const { rerender } = render( + , + ); + + await waitFor(() => expect(onReady).toHaveBeenCalled(), { timeout: 5000 }); + const instanceBefore = ref.current?.getInstance(); + + rerender( + , + ); + + await waitFor(() => expect(onEditorDestroy).toHaveBeenCalled(), { timeout: 5000 }); + await waitFor(() => expect(onReady).toHaveBeenCalledTimes(2), { timeout: 5000 }); + expect(ref.current?.getInstance()).not.toBe(instanceBefore); + }); + }); + describe('unique IDs', () => { it('should generate unique container IDs for multiple instances', () => { const { container: container1 } = render(); diff --git a/packages/react/src/SuperDocEditor.tsx b/packages/react/src/SuperDocEditor.tsx index 33662a5555..b022552456 100644 --- a/packages/react/src/SuperDocEditor.tsx +++ b/packages/react/src/SuperDocEditor.tsx @@ -7,7 +7,7 @@ import { type CSSProperties, type ForwardedRef, } from 'react'; -import { useStableId } from './utils'; +import { useStableId, useMemoByValue } from './utils'; import type { CallbackProps, DocumentMode, @@ -17,6 +17,7 @@ import type { SuperDocReadyEvent, SuperDocEditorCreateEvent, SuperDocEditorUpdateEvent, + SuperDocTransactionEvent, SuperDocContentErrorEvent, SuperDocExceptionEvent, } from './types'; @@ -46,12 +47,13 @@ function SuperDocEditorInner(props: SuperDocEditorProps, ref: ForwardedRef(null); const toolbarContainerRef = useRef(null); @@ -78,6 +87,7 @@ function SuperDocEditorInner(props: SuperDocEditorProps, ref: ForwardedRef(null); @@ -185,6 +196,11 @@ function SuperDocEditorInner(props: SuperDocEditorProps, ref: ForwardedRef { + if (!destroyed) { + callbacksRef.current.onTransaction?.(event); + } + }, onContentError: (event: SuperDocContentErrorEvent) => { if (!destroyed) { callbacksRef.current.onContentError?.(event); @@ -223,8 +239,8 @@ function SuperDocEditorInner(props: SuperDocEditorProps, ref: ForwardedRef>[0]; +/** Surface where an editor event originated. */ +export type EditorSurface = 'body' | 'header' | 'footer'; -/** Event passed to onTransaction callback */ -export type SuperDocTransactionEvent = Parameters>[0]; +/** Event passed to onEditorUpdate callback. Mirrors superdoc's EditorUpdateEvent. */ +export interface SuperDocEditorUpdateEvent { + /** The primary editor associated with the update. For header/footer edits, this is the main body editor. */ + editor: Editor; + /** The editor instance that emitted the update. For body edits, this matches `editor`. */ + sourceEditor: Editor; + /** The surface where the edit originated. */ + surface: EditorSurface; + /** Relationship ID for header/footer edits. */ + headerId?: string | null; + /** Header/footer variant (`default`, `first`, `even`, `odd`) when available. */ + sectionType?: string | null; +} + +/** Event passed to onTransaction callback. Mirrors superdoc's EditorTransactionEvent. */ +export interface SuperDocTransactionEvent { + /** The primary editor associated with the transaction. For header/footer edits, this is the main body editor. */ + editor: Editor; + /** The editor instance that emitted the transaction. For body edits, this matches `editor`. */ + sourceEditor: Editor; + /** The ProseMirror transaction or transaction-like payload emitted by the source editor. */ + transaction: any; + /** Time spent applying the transaction, in milliseconds. */ + duration?: number; + /** The surface where the transaction originated. */ + surface: EditorSurface; + /** Relationship ID for header/footer edits. */ + headerId?: string | null; + /** Header/footer variant (`default`, `first`, `even`, `odd`) when available. */ + sectionType?: string | null; +} /** Event passed to onContentError callback */ export interface SuperDocContentErrorEvent { @@ -96,6 +125,7 @@ type ExplicitCallbackProps = | 'onEditorCreate' | 'onEditorDestroy' | 'onEditorUpdate' + | 'onTransaction' | 'onContentError' | 'onException'; @@ -116,6 +146,9 @@ export interface CallbackProps { /** Callback when document content is updated */ onEditorUpdate?: (event: SuperDocEditorUpdateEvent) => void; + /** Callback when a transaction is emitted */ + onTransaction?: (event: SuperDocTransactionEvent) => void; + /** Callback when there is a content parsing error */ onContentError?: (event: SuperDocContentErrorEvent) => void; diff --git a/packages/react/src/utils.test.ts b/packages/react/src/utils.test.ts new file mode 100644 index 0000000000..6e651fb7d3 --- /dev/null +++ b/packages/react/src/utils.test.ts @@ -0,0 +1,75 @@ +import { describe, it, expect } from 'vitest'; +import { renderHook } from '@testing-library/react'; +import { useMemoByValue } from './utils'; + +describe('useMemoByValue', () => { + it('returns the same reference across renders when content is unchanged', () => { + const initial = { name: 'Alex', email: 'alex@example.com' }; + const { result, rerender } = renderHook(({ value }) => useMemoByValue(value), { + initialProps: { value: initial }, + }); + + const first = result.current; + expect(first).toBe(initial); + + // Parent passes a fresh object literal with identical content + rerender({ value: { name: 'Alex', email: 'alex@example.com' } }); + expect(result.current).toBe(first); // same reference — critical for effect deps + + // And again, still stable + rerender({ value: { name: 'Alex', email: 'alex@example.com' } }); + expect(result.current).toBe(first); + }); + + it('returns a new reference when the content actually changes', () => { + const { result, rerender } = renderHook(({ value }) => useMemoByValue(value), { + initialProps: { value: { name: 'Alex' } }, + }); + + const first = result.current; + rerender({ value: { name: 'Jamie' } }); + expect(result.current).not.toBe(first); + expect(result.current.name).toBe('Jamie'); + }); + + it('handles undefined and null stably', () => { + const { result, rerender } = renderHook(({ value }) => useMemoByValue(value as unknown), { + initialProps: { value: undefined }, + }); + + const first = result.current; + rerender({ value: undefined }); + expect(result.current).toBe(first); + + rerender({ value: null }); + expect(result.current).toBe(null); + }); + + it('stabilizes arrays the same way as objects', () => { + const { result, rerender } = renderHook(({ value }) => useMemoByValue(value), { + initialProps: { value: [{ id: 1 }, { id: 2 }] }, + }); + + const first = result.current; + rerender({ value: [{ id: 1 }, { id: 2 }] }); + expect(result.current).toBe(first); + + rerender({ value: [{ id: 1 }, { id: 3 }] }); + expect(result.current).not.toBe(first); + }); + + it('adopts a new reference on circular input (JSON.stringify throws)', () => { + const circularA: { self?: unknown; name: string } = { name: 'a' }; + circularA.self = circularA; + + const { result, rerender } = renderHook(({ value }) => useMemoByValue(value), { + initialProps: { value: circularA }, + }); + + const circularB: { self?: unknown; name: string } = { name: 'a' }; + circularB.self = circularB; + rerender({ value: circularB }); + // The compare throws; the hook falls back to adopting the new reference. + expect(result.current).toBe(circularB); + }); +}); diff --git a/packages/react/src/utils.ts b/packages/react/src/utils.ts index 6162d2b8f1..782e8fe7af 100644 --- a/packages/react/src/utils.ts +++ b/packages/react/src/utils.ts @@ -25,3 +25,42 @@ function useIdPolyfill(): string { */ export const useStableId: () => string = typeof (React as any).useId === 'function' ? (React as any).useId : useIdPolyfill; + +/** + * Returns a reference-stable version of `value` — identity only changes + * when the serialized content changes. + * + * Use for plain-data object/array props that feed into `useEffect` / + * `useMemo` dependency arrays when the consumer is likely to pass inline + * literals. Without this, every parent re-render produces a fresh + * reference and causes the effect to re-run even when the content is + * identical. + * + * Not suitable for values containing functions, class instances (Yjs + * Doc, Maps, Sets, Dates), or circular references — JSON.stringify + * drops or collapses those. The compare only runs when the incoming + * reference differs, so the steady-state cost is a single pointer check. + */ +export function useMemoByValue(value: T): T { + const lastRawRef = React.useRef(value); + const stableRef = React.useRef(value); + + if (lastRawRef.current !== value) { + if (!shallowJsonEqual(stableRef.current, value)) { + stableRef.current = value; + } + lastRawRef.current = value; + } + + return stableRef.current; +} + +function shallowJsonEqual(a: T, b: T): boolean { + if (a === b) return true; + if (a == null || b == null) return a === b; + try { + return JSON.stringify(a) === JSON.stringify(b); + } catch { + return false; + } +} diff --git a/packages/sdk/langs/python/superdoc/__init__.py b/packages/sdk/langs/python/superdoc/__init__.py index 9682645771..fa0b628a52 100644 --- a/packages/sdk/langs/python/superdoc/__init__.py +++ b/packages/sdk/langs/python/superdoc/__init__.py @@ -10,6 +10,7 @@ get_tool_catalog, list_tools, ) +from .transport import DEFAULT_STDOUT_BUFFER_LIMIT_BYTES __all__ = [ "SuperDocClient", @@ -17,6 +18,7 @@ "SuperDocDocument", "AsyncSuperDocDocument", "SuperDocError", + "DEFAULT_STDOUT_BUFFER_LIMIT_BYTES", "get_skill", "install_skill", "list_skills", diff --git a/packages/sdk/langs/python/superdoc/client.py b/packages/sdk/langs/python/superdoc/client.py index f187895b04..9291399518 100644 --- a/packages/sdk/langs/python/superdoc/client.py +++ b/packages/sdk/langs/python/superdoc/client.py @@ -22,6 +22,7 @@ DocOpenResult as GeneratedDocOpenResult, ) from .runtime import SuperDocAsyncRuntime, SuperDocSyncRuntime +from .transport import DEFAULT_STDOUT_BUFFER_LIMIT_BYTES UserIdentity = Dict[str, str] @@ -340,6 +341,9 @@ def __init__( request_timeout_ms: int | None = None, watchdog_timeout_ms: int = 30_000, max_queue_depth: int = 100, + # Raise if a single host response can exceed this size (e.g. reading + # very large documents); otherwise the default is safe. + stdout_buffer_limit_bytes: int = DEFAULT_STDOUT_BUFFER_LIMIT_BYTES, default_change_mode: Literal['direct', 'tracked'] | None = None, user: UserIdentity | None = None, ) -> None: @@ -350,6 +354,7 @@ def __init__( request_timeout_ms=request_timeout_ms, watchdog_timeout_ms=watchdog_timeout_ms, max_queue_depth=max_queue_depth, + stdout_buffer_limit_bytes=stdout_buffer_limit_bytes, default_change_mode=default_change_mode, user=user, ) diff --git a/packages/sdk/langs/python/superdoc/runtime.py b/packages/sdk/langs/python/superdoc/runtime.py index e303754502..25dc3727e0 100644 --- a/packages/sdk/langs/python/superdoc/runtime.py +++ b/packages/sdk/langs/python/superdoc/runtime.py @@ -14,7 +14,11 @@ from .embedded_cli import resolve_embedded_cli_path from .generated.contract import OPERATION_INDEX from .protocol import normalize_default_change_mode -from .transport import AsyncHostTransport, SyncHostTransport +from .transport import ( + DEFAULT_STDOUT_BUFFER_LIMIT_BYTES, + AsyncHostTransport, + SyncHostTransport, +) class SuperDocSyncRuntime: @@ -79,6 +83,7 @@ def __init__( request_timeout_ms: Optional[int] = None, watchdog_timeout_ms: int = 30_000, max_queue_depth: int = 100, + stdout_buffer_limit_bytes: int = DEFAULT_STDOUT_BUFFER_LIMIT_BYTES, default_change_mode: Optional[str] = None, user: Optional[Dict[str, str]] = None, ) -> None: @@ -93,6 +98,7 @@ def __init__( request_timeout_ms=request_timeout_ms, watchdog_timeout_ms=watchdog_timeout_ms, max_queue_depth=max_queue_depth, + stdout_buffer_limit_bytes=stdout_buffer_limit_bytes, default_change_mode=self._default_change_mode, user=user, ) diff --git a/packages/sdk/langs/python/superdoc/transport.py b/packages/sdk/langs/python/superdoc/transport.py index fec79e9c8e..d7c4a9ca0e 100644 --- a/packages/sdk/langs/python/superdoc/transport.py +++ b/packages/sdk/langs/python/superdoc/transport.py @@ -43,6 +43,12 @@ logger = logging.getLogger('superdoc.transport') +# Default stdout StreamReader buffer for the async transport. Host responses +# are single newline-delimited JSON lines, so this caps the largest individual +# response a caller can receive. Raise it if your workload routinely produces +# responses above this size (e.g. whole-document reads on very large docs). +DEFAULT_STDOUT_BUFFER_LIMIT_BYTES = 64 * 1024 * 1024 + # Opt-in debug logging via SUPERDOC_DEBUG=1 or SUPERDOC_LOG_LEVEL=debug. # Only configures the named logger — never mutates root logging config. _log_level = os.environ.get('SUPERDOC_LOG_LEVEL', '').lower() @@ -399,6 +405,7 @@ def __init__( request_timeout_ms: Optional[int] = None, watchdog_timeout_ms: int = 30_000, max_queue_depth: int = 100, + stdout_buffer_limit_bytes: int = DEFAULT_STDOUT_BUFFER_LIMIT_BYTES, default_change_mode: Optional[ChangeMode] = None, user: Optional[Dict[str, str]] = None, ) -> None: @@ -409,11 +416,13 @@ def __init__( self._request_timeout_ms = request_timeout_ms self._watchdog_timeout_ms = watchdog_timeout_ms self._max_queue_depth = max_queue_depth + self._stdout_buffer_limit_bytes = stdout_buffer_limit_bytes self._default_change_mode = default_change_mode self._user = user self._process: Optional[asyncio.subprocess.Process] = None self._reader_task: Optional[asyncio.Task] = None + self._cleanup_task: Optional[asyncio.Task] = None self._pending: Dict[int, asyncio.Future] = {} self._state = _State.DISCONNECTED self._next_request_id = 1 @@ -428,7 +437,22 @@ async def connect(self) -> None: async def dispose(self) -> None: """Gracefully shut down the host process.""" - if self._state == _State.DISCONNECTED or self._state == _State.DISPOSING: + if self._state == _State.DISCONNECTED: + return + if self._state == _State.DISPOSING: + # A reader-triggered cleanup is in flight (or an earlier teardown + # left state in DISPOSING briefly). Wait for it so the caller + # observes "host fully torn down" by the time dispose() returns. + # shield() so a cancelled dispose() doesn't interrupt _cleanup + # mid-flight and leak the host process. + existing = self._cleanup_task + if existing and not existing.done(): + try: + await asyncio.shield(existing) + except asyncio.CancelledError: + raise + except Exception: + pass return self._stopping = True @@ -507,6 +531,20 @@ async def invoke( async def _ensure_connected(self) -> None: """Lazy connect: spawn and handshake if not already connected.""" + # Drain any in-flight teardown before spawning a new host. Without + # this, a concurrent reader-triggered cleanup would still be running + # when _start_host reassigns self._process / self._reader_task; the + # cleanup task would then cancel the fresh reader and kill the fresh + # process. shield() so we don't cancel the cleanup if our caller is. + cleanup = self._cleanup_task + if cleanup and not cleanup.done(): + try: + await asyncio.shield(cleanup) + except asyncio.CancelledError: + raise + except Exception: + pass + if self._state == _State.CONNECTED and self._process and self._process.returncode is None: return @@ -531,12 +569,15 @@ async def _start_host(self) -> None: args = [*prefix_args, 'host', '--stdio'] try: + # ``limit`` raises asyncio's StreamReader buffer above its 64 KiB + # default; host responses are single JSON lines and can exceed it. self._process = await asyncio.create_subprocess_exec( command, *args, stdin=asyncio.subprocess.PIPE, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.DEVNULL, env={**os.environ, **self._env}, + limit=self._stdout_buffer_limit_bytes, ) logger.debug('Host spawned (pid=%s, bin=%s).', self._process.pid, self._cli_bin) except Exception as exc: @@ -582,7 +623,29 @@ async def _reader_loop(self) -> None: try: while True: - raw = await process.stdout.readline() + try: + raw = await process.stdout.readline() + except ValueError as exc: + # asyncio.StreamReader.readline() re-raises LimitOverrunError + # from readuntil() as ValueError when a single line exceeds + # `limit` (see CPython asyncio/streams.py). The host is still + # alive — schedule cleanup so a later dispose() doesn't + # short-circuit on DISCONNECTED state. Scoped to readline() + # only so unrelated ValueErrors from dispatch aren't + # reclassified as a buffer-limit error. _schedule_cleanup + # is a no-op when _stopping is set (graceful dispose path). + logger.debug('Reader loop buffer overflow: %s', exc) + self._schedule_cleanup(SuperDocError( + 'Host response exceeded stdout buffer limit. ' + 'Raise stdout_buffer_limit_bytes to accommodate larger responses.', + code=HOST_PROTOCOL_ERROR, + details={ + 'message': str(exc), + 'stdout_buffer_limit_bytes': self._stdout_buffer_limit_bytes, + }, + )) + return + if not raw: # EOF — process died. break @@ -614,16 +677,16 @@ async def _reader_loop(self) -> None: except Exception as exc: logger.debug('Reader loop error: %s', exc) - # Reader exited (EOF or error) — reject all pending futures. - if not self._stopping: - exit_code = process.returncode - error = SuperDocError( - 'Host process disconnected.', - code=HOST_DISCONNECTED, - details={'exit_code': exit_code, 'signal': None}, - ) - self._reject_all_pending(error) - self._state = _State.DISCONNECTED + # Reader exited (EOF or unexpected error) — tear down the process so + # no orphaned host is left running, then reject pending futures. + # _schedule_cleanup is a no-op when _stopping is set (graceful + # dispose path) so we don't race the dispose teardown. + exit_code = process.returncode + self._schedule_cleanup(SuperDocError( + 'Host process disconnected.', + code=HOST_DISCONNECTED, + details={'exit_code': exit_code, 'signal': None}, + )) async def _send_request(self, method: str, params: Any, watchdog_ms: int) -> Any: """Send a JSON-RPC request and await the matching response future.""" @@ -687,34 +750,88 @@ def _reject_all_pending(self, error: SuperDocError) -> None: future.set_exception(error) async def _kill_and_reset(self) -> None: - """Kill the host process and reset to DISCONNECTED.""" - await self._cleanup( - SuperDocError('Host process disconnected.', code=HOST_DISCONNECTED), - ) - - async def _cleanup(self, error: Optional[SuperDocError]) -> None: - """Cancel reader, kill process, reject pending, reset state.""" - if self._reader_task and not self._reader_task.done(): - self._reader_task.cancel() + """Kill the host process and reset to DISCONNECTED. + + Coordinates with `_schedule_cleanup` so callers (e.g. `_send_request` + on watchdog timeout or stdin write failure) don't run a parallel + `_cleanup` that races a reader-triggered cleanup on + `_reject_all_pending` and `process.kill`. If a cleanup is already in + flight, await it; otherwise own a fresh task in the same slot so a + later concurrent caller sees us instead of starting its own. + + shield() the await so caller cancellation (e.g. an `invoke()` task + that times out and is then cancelled by the user) does NOT propagate + into `_cleanup` — interrupting cleanup mid-flight would leak the + subprocess and wedge state in DISPOSING. + """ + existing = self._cleanup_task + if existing and not existing.done(): try: - await self._reader_task - except (asyncio.CancelledError, Exception): + await asyncio.shield(existing) + except asyncio.CancelledError: + raise + except Exception: pass - self._reader_task = None + return + self._state = _State.DISPOSING + task = asyncio.create_task(self._cleanup( + SuperDocError('Host process disconnected.', code=HOST_DISCONNECTED), + )) + self._cleanup_task = task + try: + await asyncio.shield(task) + except asyncio.CancelledError: + raise + except Exception: + pass + def _schedule_cleanup(self, error: SuperDocError) -> None: + """Fire-and-forget teardown from inside the reader task. + + Why a separate task: `_cleanup` cancels and awaits `self._reader_task`. + Awaiting it from inside the reader itself would deadlock — so we punt + to a fresh task, and by the time it runs the reader has already + returned (so cancel+await is a no-op). + + Synchronously flips state to DISPOSING so concurrent `invoke()` callers + observe the failed transport immediately rather than passing the + CONNECTED fast path and blocking on a future the dead reader can never + resolve until `watchdog_timeout_ms`. + + Skips when `_stopping` is set: a graceful `dispose()` is already + tearing down, and a parallel cleanup task would race on + `_reject_all_pending` and `process.kill`. + + Idempotent: if a cleanup is already in flight, subsequent errors are + dropped — the first one wins. Callers may observe completion via + `self._cleanup_task`. + """ + if self._stopping: + return + if self._cleanup_task and not self._cleanup_task.done(): + return + self._state = _State.DISPOSING + self._cleanup_task = asyncio.create_task(self._cleanup(error)) + + async def _cleanup(self, error: Optional[SuperDocError]) -> None: + """Cancel reader, kill process, reject pending, reset state. + + Capture handles and flip user-visible state SYNCHRONOUSLY at the top + before any awaits. That way, even if cancellation arrives during + `process.wait()`, observers see a consistent "torn down" transport + (state DISCONNECTED, _process None, pending futures rejected) rather + than a half-disposed one. The async work below is best-effort + process reaping. + """ + # Snapshot and clear before any await so concurrent callers see a + # fully torn-down transport from this point on. + reader_task = self._reader_task process = self._process - if process: - try: - process.kill() - except Exception: - pass - try: - await asyncio.wait_for(process.wait(), timeout=2) - except (asyncio.TimeoutError, Exception): - pass + self._reader_task = None self._process = None + self._state = _State.DISCONNECTED - if error: + if error is not None: self._reject_all_pending(error) else: # Dispose path — reject remaining with generic disconnect. @@ -722,4 +839,31 @@ async def _cleanup(self, error: Optional[SuperDocError]) -> None: SuperDocError('Host process was disposed.', code=HOST_DISCONNECTED), ) - self._state = _State.DISCONNECTED + try: + if reader_task and not reader_task.done(): + reader_task.cancel() + try: + await reader_task + except (asyncio.CancelledError, Exception): + pass + + if process: + try: + process.kill() + except Exception: + pass + try: + await asyncio.wait_for(process.wait(), timeout=2) + except (asyncio.TimeoutError, asyncio.CancelledError, Exception): + pass + finally: + # Release the task handle if we are the in-flight cleanup task, + # so introspection doesn't surface a stale done handle and the + # next teardown gets a fresh slot. Skip when called inline (e.g. + # from dispose) — that current task is not our cleanup task. + try: + current = asyncio.current_task() + except RuntimeError: + current = None + if current is not None and self._cleanup_task is current: + self._cleanup_task = None diff --git a/packages/sdk/langs/python/tests/test_transport.py b/packages/sdk/langs/python/tests/test_transport.py index d71bafa186..f54c0c7a71 100644 --- a/packages/sdk/langs/python/tests/test_transport.py +++ b/packages/sdk/langs/python/tests/test_transport.py @@ -24,7 +24,11 @@ HOST_TIMEOUT, SuperDocError, ) -from superdoc.transport import AsyncHostTransport, SyncHostTransport +from superdoc.transport import ( + DEFAULT_STDOUT_BUFFER_LIMIT_BYTES, + AsyncHostTransport, + SyncHostTransport, +) MOCK_HOST = os.path.join(os.path.dirname(__file__), 'mock_host.py') @@ -447,9 +451,24 @@ async def test_host_crash(self): try: transport = AsyncHostTransport(cli, startup_timeout_ms=5_000) await transport.connect() + process = transport._process + assert process is not None with pytest.raises(SuperDocError) as exc_info: await transport.invoke(_TEST_OP, {'query': 'test'}) - assert exc_info.value.code in (HOST_DISCONNECTED, HOST_TIMEOUT) + # The reader-loop EOF branch now goes through _schedule_cleanup, + # which rejects the pending future synchronously enough that the + # invoke() never has to fall back to the watchdog timeout. + assert exc_info.value.code == HOST_DISCONNECTED + + # Cleanup must tear the process down — pre-fix, the inline + # _reject_all_pending + state flip left the process orphaned. + cleanup_task = transport._cleanup_task + if cleanup_task is not None: + await cleanup_task + assert transport._process is None + assert transport.state == 'DISCONNECTED' + await process.wait() + assert process.returncode is not None finally: _cleanup_wrapper(cli) @@ -493,3 +512,432 @@ async def test_reuse_after_dispose(self): await transport.dispose() finally: _cleanup_wrapper(cli2) + + +class TestAsyncLargeResponse: + """Responses larger than the StreamReader buffer must not crash the reader.""" + + @pytest.mark.asyncio + async def test_response_above_asyncio_default_streamreader_limit(self): + big_payload = 'x' * (200 * 1024) + cli = _mock_cli_bin({ + 'handshake': 'ok', + 'responses': [{'data': {'content': big_payload}}], + }) + try: + transport = AsyncHostTransport(cli, startup_timeout_ms=5_000) + await transport.connect() + result = await transport.invoke(_TEST_OP, {'query': 'big'}) + assert result == {'content': big_payload} + assert transport.state == 'CONNECTED' + await transport.dispose() + finally: + _cleanup_wrapper(cli) + + @pytest.mark.asyncio + async def test_response_above_custom_buffer_limit_raises_protocol_error(self): + # Setting stdout_buffer_limit_bytes below the response size should + # surface HOST_PROTOCOL_ERROR (actionable) rather than + # HOST_DISCONNECTED (misleading — the host is still alive), and the + # error should carry a hint to raise the buffer limit. + from superdoc.errors import HOST_PROTOCOL_ERROR + + big_payload = 'x' * (200 * 1024) + cli = _mock_cli_bin({ + 'handshake': 'ok', + 'responses': [{'data': {'content': big_payload}}], + }) + try: + transport = AsyncHostTransport( + cli, + startup_timeout_ms=5_000, + stdout_buffer_limit_bytes=64 * 1024, + ) + await transport.connect() + process = transport._process + assert process is not None + with pytest.raises(SuperDocError) as exc_info: + await transport.invoke(_TEST_OP, {'query': 'big'}) + assert exc_info.value.code == HOST_PROTOCOL_ERROR + assert 'stdout_buffer_limit_bytes' in str(exc_info.value) + + # The host process must be torn down — not just the transport + # state flipped to DISCONNECTED. Otherwise dispose() short-circuits + # and leaves an orphaned host running. + cleanup_task = transport._cleanup_task + if cleanup_task is not None: + await cleanup_task + assert transport._process is None + assert transport.state == 'DISCONNECTED' + # The captured handle should be reaped by _cleanup; await wait() + # rather than reading returncode to avoid a CI-timing flake if the + # 2 s wait inside _cleanup didn't finish reaping in time. + await process.wait() + assert process.returncode is not None + + # dispose() after an overflow must be a safe no-op: state and + # process stay as cleanup left them, no exception is raised, and + # a second dispose() is also safe. + await transport.dispose() + assert transport.state == 'DISCONNECTED' + assert transport._process is None + await transport.dispose() + assert transport.state == 'DISCONNECTED' + finally: + _cleanup_wrapper(cli) + + @pytest.mark.asyncio + async def test_client_threads_stdout_buffer_limit_to_transport(self): + # End-to-end wiring check: the public AsyncSuperDocClient constructor + # must thread stdout_buffer_limit_bytes through SuperDocAsyncRuntime + # into AsyncHostTransport. Without this, a silent drop in client.py + # or runtime.py would leave the existing overflow test passing while + # the public API reverts to the asyncio 64 KiB default. + from superdoc.client import AsyncSuperDocClient + + cli = _mock_cli_bin({'handshake': 'ok'}) + try: + client = AsyncSuperDocClient( + env={'SUPERDOC_CLI_BIN': cli}, + stdout_buffer_limit_bytes=64 * 1024, + ) + transport = client._runtime._transport + assert transport._stdout_buffer_limit_bytes == 64 * 1024 + finally: + _cleanup_wrapper(cli) + + +class TestAsyncCleanupLifecycle: + """Lock down the cleanup-task slot so its load-bearing invariants don't + silently regress: the dedupe guard, the _stopping suppression branch, + the _kill_and_reset coordination with reader-triggered cleanup, and the + _ensure_connected drain that prevents stale cleanup from killing a + freshly-spawned host. + """ + + @pytest.mark.asyncio + async def test_schedule_cleanup_dedupe_guard_drops_reentrant_call(self): + # If a cleanup task is already in flight, a second _schedule_cleanup + # must NOT replace it — that would cancel the in-flight teardown + # mid-flight and could leak the host process. + cli = _mock_cli_bin({'handshake': 'ok'}) + try: + transport = AsyncHostTransport(cli, startup_timeout_ms=5_000) + await transport.connect() + + slow = asyncio.create_task(asyncio.sleep(0.5)) + transport._cleanup_task = slow + + transport._schedule_cleanup( + SuperDocError('second', code=HOST_DISCONNECTED), + ) + # Slot must still point at the original task — second call dropped. + assert transport._cleanup_task is slow + + slow.cancel() + try: + await slow + except (asyncio.CancelledError, Exception): + pass + transport._cleanup_task = None + await transport.dispose() + finally: + _cleanup_wrapper(cli) + + @pytest.mark.asyncio + async def test_schedule_cleanup_skipped_when_stopping(self): + # When `dispose()` is in progress, `_stopping` is set; the production + # guard inside `_schedule_cleanup` must short-circuit so a reader + # overflow doesn't race the graceful teardown. (Earlier iterations + # of this test were tautological because the test re-checked + # `_stopping` before calling `_schedule_cleanup`. This version calls + # it unconditionally and asserts the production guard fires.) + cli = _mock_cli_bin({'handshake': 'ok'}) + try: + transport = AsyncHostTransport(cli, startup_timeout_ms=5_000) + await transport.connect() + transport._stopping = True + assert transport._cleanup_task is None + + transport._schedule_cleanup( + SuperDocError('overflow', code=HOST_PROTOCOL_ERROR), + ) + assert transport._cleanup_task is None + assert transport.state == 'CONNECTED' + + transport._stopping = False + await transport.dispose() + finally: + _cleanup_wrapper(cli) + + @pytest.mark.asyncio + async def test_kill_and_reset_awaits_in_flight_cleanup(self): + # If a reader-triggered cleanup is already running, _kill_and_reset + # must await it rather than spin up a parallel _cleanup that would + # race on _reject_all_pending and process.kill. + cli = _mock_cli_bin({'handshake': 'ok'}) + try: + transport = AsyncHostTransport(cli, startup_timeout_ms=5_000) + await transport.connect() + + # Replace _cleanup with a tracking stub so we can count entries + # and verify the second call observes the first task instead of + # creating a fresh one. Use Events for deterministic ordering + # rather than asyncio.sleep(0) (which is implementation-defined + # under uvloop / Python scheduling changes). + entry_count = 0 + started = asyncio.Event() + release = asyncio.Event() + real_cleanup = transport._cleanup + + async def tracking_cleanup(error): + nonlocal entry_count + entry_count += 1 + started.set() + # First entry blocks until the test releases it; subsequent + # entries (if any) would race past — failure mode for the bug. + await release.wait() + await real_cleanup(error) + + transport._cleanup = tracking_cleanup # type: ignore[assignment] + + transport._schedule_cleanup( + SuperDocError('reader-overflow', code=HOST_PROTOCOL_ERROR), + ) + await asyncio.wait_for(started.wait(), timeout=2.0) + assert entry_count == 1 + assert transport._cleanup_task is not None + assert not transport._cleanup_task.done() + + kill_task = asyncio.create_task(transport._kill_and_reset()) + # Give kill_task a chance to enter — but it must NOT start a + # second _cleanup (which would re-fire `started`). + await asyncio.sleep(0.05) + assert entry_count == 1 + assert not kill_task.done() + + release.set() + await kill_task + assert entry_count == 1 + assert transport.state == 'DISCONNECTED' + assert transport._cleanup_task is None + finally: + _cleanup_wrapper(cli) + + @pytest.mark.asyncio + async def test_dispose_waits_for_in_flight_cleanup(self): + # `dispose()` called while a reader-triggered cleanup is in flight + # must wait for it to finish, so the caller observes "fully torn + # down" by the time dispose returns. + cli = _mock_cli_bin({'handshake': 'ok'}) + try: + transport = AsyncHostTransport(cli, startup_timeout_ms=5_000) + await transport.connect() + + started = asyncio.Event() + release = asyncio.Event() + real_cleanup = transport._cleanup + + async def slow_cleanup(error): + started.set() + await release.wait() + await real_cleanup(error) + + transport._cleanup = slow_cleanup # type: ignore[assignment] + + transport._schedule_cleanup( + SuperDocError('reader-overflow', code=HOST_PROTOCOL_ERROR), + ) + await asyncio.wait_for(started.wait(), timeout=2.0) + assert transport.state == 'DISPOSING' + + dispose_task = asyncio.create_task(transport.dispose()) + await asyncio.sleep(0.05) + # dispose must still be waiting on the cleanup task. + assert not dispose_task.done() + + release.set() + await dispose_task + assert transport.state == 'DISCONNECTED' + assert transport._process is None + assert transport._cleanup_task is None + finally: + _cleanup_wrapper(cli) + + @pytest.mark.asyncio + async def test_ensure_connected_drains_in_flight_cleanup_before_spawn(self): + # Round-3 regression: without this drain, `_start_host` reassigns + # `self._process` while a stale `_cleanup` task is still scheduled; + # the cleanup then kills the freshly-spawned process. + cli = _mock_cli_bin({'handshake': 'ok'}) + try: + transport = AsyncHostTransport(cli, startup_timeout_ms=5_000) + await transport.connect() + old_process = transport._process + assert old_process is not None + + started = asyncio.Event() + release = asyncio.Event() + real_cleanup = transport._cleanup + + async def slow_cleanup(error): + started.set() + await release.wait() + await real_cleanup(error) + + transport._cleanup = slow_cleanup # type: ignore[assignment] + + transport._schedule_cleanup( + SuperDocError('reader-overflow', code=HOST_PROTOCOL_ERROR), + ) + await asyncio.wait_for(started.wait(), timeout=2.0) + + connect_task = asyncio.create_task(transport.connect()) + await asyncio.sleep(0.05) + # connect() must be blocked on the in-flight cleanup, not racing + # ahead to spawn a fresh process the cleanup would then kill. + assert not connect_task.done() + + release.set() + await connect_task + new_process = transport._process + assert new_process is not None + assert new_process is not old_process + # The fresh process must NOT have been killed by the stale cleanup. + assert new_process.returncode is None + assert transport.state == 'CONNECTED' + await transport.dispose() + finally: + _cleanup_wrapper(cli) + + @pytest.mark.asyncio + async def test_kill_and_reset_caller_cancellation_does_not_cancel_cleanup(self): + # Round-3 regression: without `asyncio.shield`, cancelling the + # awaiter of `_kill_and_reset` propagates into the cleanup task, + # interrupting it mid-flight before `_process` is fully reaped and + # leaving state wedged in DISPOSING. + cli = _mock_cli_bin({'handshake': 'ok'}) + try: + transport = AsyncHostTransport(cli, startup_timeout_ms=5_000) + await transport.connect() + + started = asyncio.Event() + release = asyncio.Event() + real_cleanup = transport._cleanup + + async def slow_cleanup(error): + started.set() + try: + await release.wait() + except asyncio.CancelledError: + # If shield works, this should NOT fire. Re-raise so the + # test's assertion catches the regression. + raise + await real_cleanup(error) + + transport._cleanup = slow_cleanup # type: ignore[assignment] + + kill_task = asyncio.create_task(transport._kill_and_reset()) + await asyncio.wait_for(started.wait(), timeout=2.0) + + kill_task.cancel() + with pytest.raises(asyncio.CancelledError): + await kill_task + + # Cleanup must keep running despite kill_task being cancelled. + assert transport._cleanup_task is not None + assert not transport._cleanup_task.done() + + release.set() + await transport._cleanup_task + assert transport.state == 'DISCONNECTED' + assert transport._process is None + assert transport._cleanup_task is None + finally: + _cleanup_wrapper(cli) + + +class TestAsyncOverflowConcurrency: + """Concurrency scenarios for the buffer-overflow path.""" + + @pytest.mark.asyncio + async def test_overflow_rejects_all_pending_invokes(self): + # Codex/Opus round-3 gap: every pending future must be rejected with + # HOST_PROTOCOL_ERROR — not just the one whose response overflowed. + # A regression where _reject_all_pending only rejects pending[msg.id] + # would silently leave concurrent callers hanging until watchdog. + big_payload = 'x' * (200 * 1024) + cli = _mock_cli_bin({ + 'handshake': 'ok', + 'responses': [ + {'data': {'content': big_payload}}, + {'data': {'v': 2}}, + {'data': {'v': 3}}, + ], + }) + try: + transport = AsyncHostTransport( + cli, + startup_timeout_ms=5_000, + stdout_buffer_limit_bytes=64 * 1024, + watchdog_timeout_ms=10_000, + ) + await transport.connect() + tasks = [ + asyncio.ensure_future(transport.invoke(_TEST_OP, {'query': f'q{i}'})) + for i in range(3) + ] + results = await asyncio.gather(*tasks, return_exceptions=True) + assert all(isinstance(r, SuperDocError) for r in results), results + assert all(r.code == HOST_PROTOCOL_ERROR for r in results) + # Every error must carry the actionable hint, not just the first. + assert all('stdout_buffer_limit_bytes' in str(r) for r in results) + assert transport._pending == {} + assert transport.state == 'DISCONNECTED' + await transport.dispose() + finally: + _cleanup_wrapper(cli) + + @pytest.mark.asyncio + async def test_reconnect_after_buffer_overflow(self): + # Sync transport has test_reconnect_after_failure; async previously + # only had reconnect-after-explicit-dispose. After reader-triggered + # cleanup the transport must be reusable for a fresh invoke without + # leaving _cleanup_task / _connecting / _process in a wedged state. + cli1 = _mock_cli_bin({ + 'handshake': 'ok', + 'responses': [{'data': {'content': 'x' * (200 * 1024)}}], + }) + transport = None + try: + transport = AsyncHostTransport( + cli1, + startup_timeout_ms=5_000, + stdout_buffer_limit_bytes=64 * 1024, + ) + await transport.connect() + with pytest.raises(SuperDocError) as exc_info: + await transport.invoke(_TEST_OP, {'query': 'big'}) + assert exc_info.value.code == HOST_PROTOCOL_ERROR + cleanup_task = transport._cleanup_task + if cleanup_task is not None: + await cleanup_task + assert transport.state == 'DISCONNECTED' + assert transport._cleanup_task is None + finally: + _cleanup_wrapper(cli1) + + cli2 = _mock_cli_bin({ + 'handshake': 'ok', + 'responses': [{'data': {'v': 'reconnected'}}], + }) + try: + # Reuse the transport — point at a healthy host with default buffer. + transport._cli_bin = cli2 + transport._stdout_buffer_limit_bytes = DEFAULT_STDOUT_BUFFER_LIMIT_BYTES + result = await transport.invoke(_TEST_OP, {'query': 'again'}) + assert result == {'v': 'reconnected'} + assert transport.state == 'CONNECTED' + await transport.dispose() + finally: + _cleanup_wrapper(cli2) diff --git a/packages/super-editor/src/editors/v1/components/context-menu/ContextMenu.vue b/packages/super-editor/src/editors/v1/components/context-menu/ContextMenu.vue index 53dd19da12..83a761b27a 100644 --- a/packages/super-editor/src/editors/v1/components/context-menu/ContextMenu.vue +++ b/packages/super-editor/src/editors/v1/components/context-menu/ContextMenu.vue @@ -1,5 +1,7 @@