From 7a44dc9bd155082460a75c4c8842d0b8258e91c6 Mon Sep 17 00:00:00 2001 From: Sangjoon Han Date: Tue, 28 Jul 2026 21:22:08 +0900 Subject: [PATCH] fix: resolve Obsidian plugin-review findings, and close the gap that let them through MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two findings from the plugin review, plus the reason our own gate missed both. - `el()` builds elements with Obsidian's `createEl` helper instead of `document.createElement` (`obsidianmd/prefer-create-el`). The free function, not `Node.createEl` — the latter appends to the node it is called on, and `el()` must return something detached. - The inert footnote marker's dotted underline uses the `text-decoration-line` / `text-decoration-style` longhands. Obsidian's CSS check flags the multi-value `text-decoration` shorthand as only partially supported at our `minAppVersion`. Why CI was green while both stood: - `eslint-plugin-obsidianmd` was pinned at 0.3.0, which does not carry the `prefer-create-el` rule the reviewer ran. Bumped to 0.4.1, which reproduces the finding exactly. - That rule reports at *warning* severity, and `eslint` exits 0 on warnings, so even on 0.4.1 the gate would have passed. `npm run lint` now runs with `--max-warnings=0`. Verified the hardened gate actually bites: reverting `el()` to `activeDocument.createElement` fails `npm run lint`. The e2e bootstrap gained a `createEl`/`Node.prototype.createEl` stub written to the real signature — the second argument is a class string or a DomElementInfo — rather than shaped to whatever the bundle happens to call. A stub fitted to the implementation is how a green suite hid a broken feature earlier in this release. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 26 ++++++++++++++++- manifest.json | 2 +- package-lock.json | 43 +++++++++++++++++++++++++---- package.json | 4 +-- src/main.ts | 12 +++++--- styles.css | 6 +++- tests/e2e/diagram-fit.test.mjs | 22 +++++++++++++++ tests/e2e/table-fullscreen.test.mjs | 22 +++++++++++++++ versions.json | 3 +- 9 files changed, 124 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bbe103d..d825ba6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [1.2.1] - 2026-07-28 + +Compliance fixes raised by Obsidian's plugin review. No change in behaviour. + +### Fixed + +- Elements are created through Obsidian's `createEl` helper instead of + `document.createElement`, per the plugin guidelines + (`obsidianmd/prefer-create-el`). +- The inert footnote marker's dotted underline is written with the + `text-decoration-line` / `text-decoration-style` longhands. Obsidian's CSS + check flags the multi-value `text-decoration` shorthand as only partially + supported at this plugin's `minAppVersion`. + +### Internal — recurrence prevention + +- `eslint-plugin-obsidianmd` moves 0.3.0 → 0.4.1. Both findings above are rules + the local gate already had in principle, but the pinned 0.3.0 did not yet + carry them, so CI could not have caught either one before submission. +- `npm run lint` now runs with `--max-warnings=0`. `prefer-create-el` reports at + *warning* severity, so the gate exited 0 while the violation stood; warnings + now fail the build the same as errors. + ## [1.2.0] - 2026-07-28 ### Added @@ -192,7 +215,8 @@ Initial public release. mode), not only in Reading view. The table being actively edited is left untouched. -[Unreleased]: https://github.com/Post-Math/Lookout/compare/1.2.0...HEAD +[Unreleased]: https://github.com/Post-Math/Lookout/compare/1.2.1...HEAD +[1.2.1]: https://github.com/Post-Math/Lookout/compare/1.2.0...1.2.1 [1.2.0]: https://github.com/Post-Math/Lookout/compare/1.1.7...1.2.0 [#28]: https://github.com/Post-Math/Lookout/issues/28 [1.1.3]: https://github.com/Post-Math/Lookout/compare/1.1.2...1.1.3 diff --git a/manifest.json b/manifest.json index 48b2865..af9869b 100644 --- a/manifest.json +++ b/manifest.json @@ -1,7 +1,7 @@ { "id": "lookout", "name": "Lookout", - "version": "1.2.0", + "version": "1.2.1", "minAppVersion": "1.6.6", "description": "Survey wide content instead of scrolling sideways. Pan and zoom Mermaid diagrams (wheel, Ctrl+wheel, or buttons), fit them to the frame, and open diagrams or wide tables full-screen.", "author": "Post-Math", diff --git a/package-lock.json b/package-lock.json index 54c47e0..d677b5a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,7 +12,7 @@ "@types/node": "^20.14.0", "esbuild": "^0.25.0", "eslint": "^9.39.4", - "eslint-plugin-obsidianmd": "^0.3.0", + "eslint-plugin-obsidianmd": "^0.4.1", "obsidian": "^1.6.0", "playwright-core": "^1.48.0", "tslib": "^2.6.0", @@ -487,6 +487,36 @@ "node": ">=18" } }, + "node_modules/@eslint-community/eslint-plugin-eslint-comments": { + "version": "4.7.2", + "resolved": "https://registry.npmjs.org/@eslint-community/eslint-plugin-eslint-comments/-/eslint-plugin-eslint-comments-4.7.2.tgz", + "integrity": "sha512-LF03qURSwEWm2dz5wtdDCzNk+7Opl0X7q6I3undsaIuNsEiNvRV3BCtqu14Q/6Pzg1tBj44LcxpW2EpSLZStZw==", + "dev": true, + "license": "MIT", + "dependencies": { + "escape-string-regexp": "^4.0.0", + "ignore": "^7.0.5" + }, + "engines": { + "node": "^12.22.0 || ^14.17.0 || >=16.0.0" + }, + "funding": { + "url": "https://opencollective.com/eslint" + }, + "peerDependencies": { + "eslint": "^6.0.0 || ^7.0.0 || ^8.0.0 || ^9.0.0 || ^10.0.0" + } + }, + "node_modules/@eslint-community/eslint-plugin-eslint-comments/node_modules/ignore": { + "version": "7.0.6", + "resolved": "https://registry.npmjs.org/ignore/-/ignore-7.0.6.tgz", + "integrity": "sha512-BAg6QkE8W+TuQLrrw0Ugr7HegXduRuuj8/ti2kSOc+jz1dmx8/WNcjr6XGnq5YpDWxFwwaavqD0+jIUOKelTsw==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">= 4" + } + }, "node_modules/@eslint-community/eslint-utils": { "version": "4.9.1", "resolved": "https://registry.npmjs.org/@eslint-community/eslint-utils/-/eslint-utils-4.9.1.tgz", @@ -2316,12 +2346,13 @@ } }, "node_modules/eslint-plugin-obsidianmd": { - "version": "0.3.0", - "resolved": "https://registry.npmjs.org/eslint-plugin-obsidianmd/-/eslint-plugin-obsidianmd-0.3.0.tgz", - "integrity": "sha512-QvGDI6B2nxJBrsZKGTg31da2A/fEJNlnwN+fRZkaoPIu1QL3fYXUdpP7ThyMdr/0iTYQxifb9lt2X9cpydQx1w==", + "version": "0.4.1", + "resolved": "https://registry.npmjs.org/eslint-plugin-obsidianmd/-/eslint-plugin-obsidianmd-0.4.1.tgz", + "integrity": "sha512-Nv3593kVsFOS8kir/HWaJ5CR3xcyiPWEPyXst5Pib8mxRYYuLYPpJS2ALMoZXHulHyiGwoYW8AaxvLRc4rhrRg==", "dev": true, "license": "MIT", "dependencies": { + "@eslint-community/eslint-plugin-eslint-comments": "^4.7.2", "@eslint/config-helpers": "^0.4.2", "@eslint/js": "^9.30.1", "@eslint/json": "0.14.0", @@ -2330,7 +2361,7 @@ "@types/node": "20.12.12", "@typescript-eslint/types": "^8.33.1", "@typescript-eslint/utils": "^8.33.1", - "eslint": ">=9.0.0", + "eslint": ">=9.19.0", "eslint-plugin-depend": "1.3.1", "eslint-plugin-import": "^2.31.0", "eslint-plugin-json-schema-validator": "5.1.0", @@ -2351,7 +2382,7 @@ "peerDependencies": { "@eslint/js": "^9.30.1", "@eslint/json": "0.14.0", - "eslint": ">=9.0.0", + "eslint": ">=9.19.0", "obsidian": "1.8.7", "typescript-eslint": "^8.35.1" } diff --git a/package.json b/package.json index 0a0969c..477f200 100644 --- a/package.json +++ b/package.json @@ -8,7 +8,7 @@ "dev": "node esbuild.config.mjs", "build": "tsc --noEmit --skipLibCheck && node esbuild.config.mjs production", "typecheck": "tsc --noEmit --skipLibCheck", - "lint": "eslint \"src/**/*.ts\"", + "lint": "eslint --max-warnings=0 \"src/**/*.ts\"", "validate": "node scripts/validate.mjs", "test:e2e": "npm run test:e2e:table && npm run test:e2e:diagram", "test:e2e:table": "node tests/e2e/table-fullscreen.test.mjs", @@ -27,7 +27,7 @@ "@types/node": "^20.14.0", "esbuild": "^0.25.0", "eslint": "^9.39.4", - "eslint-plugin-obsidianmd": "^0.3.0", + "eslint-plugin-obsidianmd": "^0.4.1", "obsidian": "^1.6.0", "playwright-core": "^1.48.0", "tslib": "^2.6.0", diff --git a/src/main.ts b/src/main.ts index 66c888e..435a1b3 100644 --- a/src/main.ts +++ b/src/main.ts @@ -88,14 +88,18 @@ function svgIcon(name: IconName): SVGSVGElement { return svg; } -/** Create an element with an optional class. Tag-typed so callers keep `.type`, `.disabled`, … */ +/** + * Create a detached element with an optional class. Tag-typed so callers keep + * `.type`, `.disabled`, … Obsidian's `createEl` rather than + * `document.createElement`, per its plugin guidelines: `Node.createEl` would + * *append* to the node it is called on, so the free function is the one that + * returns something detached. + */ function el( tag: K, cls?: string ): HTMLElementTagNameMap[K] { - const node = activeDocument.createElement(tag); - if (cls) node.className = cls; - return node; + return createEl(tag, cls ? { cls } : undefined); } /** Clamp `v` into the inclusive `[lo, hi]` range. */ diff --git a/styles.css b/styles.css index d8d08ed..54e4b2e 100644 --- a/styles.css +++ b/styles.css @@ -446,7 +446,11 @@ .lookout-table-fs-scroll .lookout-fn-unresolved { color: var(--text-muted); cursor: default; - text-decoration: underline dotted; + /* Longhands, not the `text-decoration` shorthand: Obsidian's plugin CSS check + flags the multi-value shorthand as only partially supported at our + `minAppVersion`, while these two have been safe for far longer. */ + text-decoration-line: underline; + text-decoration-style: dotted; text-underline-offset: 2px; } diff --git a/tests/e2e/diagram-fit.test.mjs b/tests/e2e/diagram-fit.test.mjs index f3fedfd..cd5c812 100644 --- a/tests/e2e/diagram-fit.test.mjs +++ b/tests/e2e/diagram-fit.test.mjs @@ -35,6 +35,28 @@ const bootstrap = ` // Obsidian runtime augmentations the bundle relies on (absent outside Obsidian). globalThis.activeWindow = window; globalThis.activeDocument = document; + // Obsidian's DOM helper, faithful to the real signature: the second argument + // is a class string or a DomElementInfo. A stub shaped to whatever the code + // happens to ask for is how a green suite hid a broken feature before. + globalThis.createEl = function (tag, o, cb) { + const node = document.createElement(tag); + if (typeof o === "string") o = { cls: o }; + if (o) { + if (o.cls) node.className = Array.isArray(o.cls) ? o.cls.join(" ") : o.cls; + if (o.text) node.textContent = o.text; + if (o.attr) for (const k in o.attr) node.setAttribute(k, o.attr[k]); + if (o.title) node.title = o.title; + if (o.parent) o.prepend ? o.parent.prepend(node) : o.parent.appendChild(node); + } + if (cb) cb(node); + return node; + }; + Node.prototype.createEl = function (tag, o, cb) { + if (typeof o === "string") o = { cls: o }; + o = o || {}; + o.parent = this; + return globalThis.createEl(tag, o, cb); + }; const _setCssStyles = function (styles) { Object.assign(this.style, styles); }; const _setCssProps = function (props) { for (const k in props) this.style.setProperty(k, props[k]); }; HTMLElement.prototype.setCssStyles = _setCssStyles; diff --git a/tests/e2e/table-fullscreen.test.mjs b/tests/e2e/table-fullscreen.test.mjs index d436ab2..b2ff8ee 100644 --- a/tests/e2e/table-fullscreen.test.mjs +++ b/tests/e2e/table-fullscreen.test.mjs @@ -34,6 +34,28 @@ const bootstrap = ` // Obsidian runtime augmentations the bundle relies on (absent outside Obsidian). globalThis.activeWindow = window; globalThis.activeDocument = document; + // Obsidian's DOM helper, faithful to the real signature: the second argument + // is a class string or a DomElementInfo. A stub shaped to whatever the code + // happens to ask for is how a green suite hid a broken feature before. + globalThis.createEl = function (tag, o, cb) { + const node = document.createElement(tag); + if (typeof o === "string") o = { cls: o }; + if (o) { + if (o.cls) node.className = Array.isArray(o.cls) ? o.cls.join(" ") : o.cls; + if (o.text) node.textContent = o.text; + if (o.attr) for (const k in o.attr) node.setAttribute(k, o.attr[k]); + if (o.title) node.title = o.title; + if (o.parent) o.prepend ? o.parent.prepend(node) : o.parent.appendChild(node); + } + if (cb) cb(node); + return node; + }; + Node.prototype.createEl = function (tag, o, cb) { + if (typeof o === "string") o = { cls: o }; + o = o || {}; + o.parent = this; + return globalThis.createEl(tag, o, cb); + }; const _setCssStyles = function (styles) { Object.assign(this.style, styles); }; const _setCssProps = function (props) { for (const k in props) this.style.setProperty(k, props[k]); }; HTMLElement.prototype.setCssStyles = _setCssStyles; diff --git a/versions.json b/versions.json index 7da1bbd..fc76cd2 100644 --- a/versions.json +++ b/versions.json @@ -6,5 +6,6 @@ "1.1.5": "1.0.0", "1.1.6": "1.0.0", "1.1.7": "1.0.0", - "1.2.0": "1.6.6" + "1.2.0": "1.6.6", + "1.2.1": "1.6.6" }