From e7d2300cf89d9afc670a5f93ecbaa8ad4c3feb12 Mon Sep 17 00:00:00 2001
From: Menghuan1918 <menghuan2003@outlook.com>
Date: Mon, 28 Sep 2026 11:17:03 +0800
Subject: [PATCH] =?UTF-8?q?fix(review):=20=E6=8C=89=E7=8B=AC=E7=AB=8B=20re?=
 =?UTF-8?q?view=EF=BC=88Chromium=20=E5=AE=9E=E6=B5=8B=EF=BC=89=E4=BF=AE?=
 =?UTF-8?q?=E6=AD=A3=20#786=20=E7=9A=84=E6=8E=AA=E8=BE=9E=E3=80=81?=
 =?UTF-8?q?=E5=AE=88=E5=8D=AB=E4=B8=8E=E8=A6=86=E7=9B=96?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

reviewer 在本机跑 Chromium 逐条实测（同一批宿主规则 + 插件产物），结论：修复方向与形状正确
（`.mermaidModal` 预修复计算 `no-drag`、修复后 `none`；级联探针确实能把未修复产物判红），
但有几处措辞与守卫不到位。逐条处理：

1. **「app-region 会被继承」的说法在该引擎里不成立**：`[data-dsh-panel-host]` 不是 body
   子元素（DOM 是 `body > [data-dsh-better-sidebar] > [data-dsh-panel-host]`），lane 用的
   Chromium 不传播该属性（实测：`drag` 父元素里子元素读 `none`）。改写成「宿主是 body
   子元素；在 Electron 里该值会传到它内部的面板层（#772 的 CDP 实测），lane 的引擎不传播，
   所以面板层自己也声明同一条规则」——CSS 注释、指南 §12.1、探针注释三处同步。
2. **`regions.layer` 那条断言是真空断言**（修复前后都读 `none`）：保留为回归钉子，但在
   注释里明确标注它不具判别力、面板层的规则由 `panel-host-css.spec.ts` 钉住。
3. **给修复本身补行为级守护**：放大视图已经在同一 lane 里被真实打开过，于是在那里直接对
   `[data-mermaid-modal]`（以及它的按钮）读计算值——断言用**页面里宿主自己的** darwin
   规则（不再是探针复制的副本）。实测判别力：npm 0.22.0 上 `Received: "no-drag"`、修复版
   `none`。
4. **「交互层不要加 initial」与放大视图的矛盾**：放大视图是**遮罩层例外**——模态语义下
   只有其中控件必须保持 `no-drag`；残余代价（背景落在顶部拖拽带上时点背景会变拖窗）已写进
   指南，避免后来者按字面判据「修」掉其中一边。
5. **`.selectionPopup` 的负向守卫可被选择器列表绕过**（reviewer 实测：`.selectionPopup,.x
   { initial !important }` 能通过）：改成按**声明块**扫描——凡带 `-webkit-app-region:
   initial` 的块，其选择器文本都不得出现 `selectionPopup`。
6. **补上 `none` 关键字的坑**：字面量 `none` 计算成 `no-drag`（是扣减值），只有 `initial`
   /`unset`/`revert` 才是中性值——CSS 注释、指南与单元用例注释都已写明。
7. **未覆盖层给出结论**：`FileTree` 的 `.uploadDropChatHint` 是 body 直挂的装饰层，但只在
   OS 文件拖拽悬停期间存在且 `pointer-events:none`，收益极小而**有意不处理**（写进指南，
   不再让「覆盖清单」看起来是绝对集合）。
8. 顺带：`§12.2` 的同步集里 `tests/theme.spec.ts` 只守颜色（本轮无颜色改动），形状守护落在
   已解析该样式表的 `tests/panel-host-css.spec.ts` 上——指南那句已按此表述。

验证：typecheck / lint 通过；panel-host-css 4 例通过；本地挂载 lane 7 passed（含新的放大
视图计算值断言）；对 npm 0.22.0 的判别力实测：宿主探针红（`host` = `no-drag`）、放大视图
断言红（`modal` = `no-drag`），修复版全绿。
---
 docs/external-plugin-guide.md |  2 +-
 src/client/sidebar.module.css | 36 +++++++++++++++----------
 tests/e2e/mount.e2e.ts        | 51 +++++++++++++++++++++++++++--------
 tests/panel-host-css.spec.ts  | 29 +++++++++++++-------
 4 files changed, 83 insertions(+), 35 deletions(-)

diff --git a/docs/external-plugin-guide.md b/docs/external-plugin-guide.md
index bf9dcf9..0c45e8f 100644
--- a/docs/external-plugin-guide.md
+++ b/docs/external-plugin-guide.md
@@ -997,7 +997,7 @@ interface SettingsDescriptor {
   - **迁移**：`titleBarScheme` 无默认值；旧文档已有值（`titleBarCompat === true` 或 `titleBarStripPx` 非 40）→ 迁 `custom`；干净文档 → `auto`。
   - **用户空间 CSS**：预设/自定义 css 注入 `<style data-dsh-preset-css|data-dsh-custom-css>` 到 head 末尾（后写胜出；覆盖 JS 内联需 `!important`），fiber 卸载即移除。稳定寻址面：`[data-dsh-toggle-cluster]` / `[data-dsh-panel]` / `[data-dsh-bottom-panel]`。
   - **拖拽区退出**：交互 chrome（`.toggleCluster` / `.toggleButton` / `.tabBar`）统一 `-webkit-app-region: no-drag`（无边框壳拖拽带吞点击，#103/#111）。
-  - **拖拽区退出（视口层）**：宿主把每个**直挂 body 的子元素**都设成 `no-drag`（`html[data-platform=darwin] body > :not(#root)`，选择器含 id，只能靠 `!important` 压过），而 app-region **无视 `pointer-events`**——铺满视口的插件层因此会把它下面的每条 `[data-window-drag]` 拖拽带一起抵消（窗口拖一次就失效，同时丢掉 macOS 双击标题栏缩放，issue #772）。契约：装饰性的视口层用**中性值** `-webkit-app-region: initial !important` 退出计算（`initial` 的计算值 `none` 不扣减拖拽区），层内的面板/控件再声明 `no-drag` 保住点击；当前覆盖 `[data-dsh-better-sidebar]`、`[data-dsh-panel-host]`（`> *` 保持 `no-drag`）与 `.mermaidModal`（放大视图是第二个铺满视口的 body 直挂层）。**交互弹层（`.selectionPopup` 是 `<button>`，`FloatingWindow` / `AnchoredPopover` 自带指针拖拽）不要加 `initial`**：宿主 `:is(button, a, input, …)` 已给它们 `no-drag`，反过来声明 `initial !important` 会让按下变成拖窗（重演 #103/#111）。形状由 `tests/panel-host-css.spec.ts` 在 Linux 的 `pnpm test` 里守护（无 macOS runner），真实级联由挂载 lane 的探针按宿主两条规则逐字复现后断言计算值。
+  - **拖拽区退出（视口层）**：宿主把每个**直挂 body 的子元素**都设成 `no-drag`（`html[data-platform=darwin] body > :not(#root)`，选择器含 id，只能靠 `!important` 压过），而 app-region **无视 `pointer-events`**——插件宿主就是这样一个 body 子元素，在 Electron 里该值还会传到它内部的**铺满视口的面板层**（#772 的 CDP 实测；挂载 lane 用的浏览器引擎不传播该属性，所以面板层自己也声明了同一条规则），于是它把下面每条 `[data-window-drag]` 拖拽带一起抵消（窗口拖一次就失效，同时丢掉 macOS 双击标题栏缩放，issue #772）。契约：装饰性的视口层用**中性值** `-webkit-app-region: initial !important` 退出计算（`initial` 的计算值 `none` 不扣减拖拽区；注意**字面量 `none` 不是中性值**——它计算成 `no-drag`，是扣减值），层内的面板/控件再声明 `no-drag` 保住点击；当前覆盖 `[data-dsh-better-sidebar]`、`[data-dsh-panel-host]`（`> *` 保持 `no-drag`）与 `.mermaidModal`（放大视图是第二个**持久**铺满视口的 body 直挂层）。**交互弹层不要加 `initial`**（`.selectionPopup` 是 `<button>`，`FloatingWindow` / `AnchoredPopover` 自带指针拖拽、不是遮罩层）：宿主 `:is(button, a, input, …)` 已给它们 `no-drag`，反过来声明 `initial !important` 会让按下变成拖窗（重演 #103/#111）。放大视图是**遮罩层例外**——它是模态，只有其中的控件必须保持 `no-drag`；代价是它覆盖在顶部拖拽带上时，点背景会变成拖窗而不是关闭（为「放大图时窗口仍可拖」付的账）。**未纳入**：`FileTree` 的 `.uploadDropChatHint` 同样是 body 直挂的装饰层，但它只在 OS 文件拖拽悬停期间存在且 `pointer-events: none`，收益极小、暂不处理（`FloatingWindow` / `AnchoredPopover` 是交互层，按上面的判据本就该保持 `no-drag`）。形状由 `tests/panel-host-css.spec.ts` 在 Linux 的 `pnpm test` 里守护（无 macOS runner；`tests/theme.spec.ts` 只守颜色，故本轮不涉及），真实级联由挂载 lane 的探针按宿主规则断言计算值（面板宿主探针复现宿主两条 darwin 规则，放大视图探针直接用页面里宿主自己的规则）。
 - **z-index**：面板宿主层 25、按钮簇 45——低于 DSH ui-cordis 插件面板（30）与浮层栈（100/1000+），浮层天然盖住侧边栏。
 
 ### 12.2 注意事项
diff --git a/src/client/sidebar.module.css b/src/client/sidebar.module.css
index 5971e71..6c5f8c8 100644
--- a/src/client/sidebar.module.css
+++ b/src/client/sidebar.module.css
@@ -64,20 +64,28 @@
 }
 
 /* Desktop drag regions ignore pointer-events: the shell's base stylesheet
-   makes every direct body child `no-drag`, and this viewport-sized layer
-   inherited it, cancelling every window-drag strip beneath it once the
-   shell recomputed regions (window drags worked once, then stopped).
-   Take the host and layer out of the app-region computation; `!important`
-   because the shell's `body > :not(#root)` selector outranks ours. The
-   panels themselves stay no-drag so their controls keep receiving clicks.
-   The plugin's OTHER viewport-sized body child — the mermaid zoom modal —
-   carries the same reset on its own rule further down, or opening a diagram
-   would blank window drag (and the macOS double-click-title zoom) while it
-   is up. Deliberately NOT applied to the interactive popups (`.selectionPopup`
-   is a `<button>`; FloatingWindow / AnchoredPopover own pointer drags): the
-   shell's `:is(button, a, input, …)` rule already keeps those no-drag, and
-   `initial !important` would outrank it and start a window drag on press
-   instead — the click-swallow regression of issues #103/#111. */
+   makes every direct body child `no-drag`, and the plugin HOST is one — in
+   Electron that value also reaches the viewport-sized panel layer inside it
+   (#772's CDP reading; the browser engine used by the mount lane does not
+   propagate it, so the layer's own rule below is belt-and-braces), which is
+   what cancelled every window-drag strip beneath it once the shell
+   recomputed regions (window drags worked once, then stopped). Take the host
+   and layer out of the app-region computation; `!important` because the
+   shell's `body > :not(#root)` selector outranks ours. The panels themselves
+   stay no-drag so their controls keep receiving clicks.
+   The plugin's OTHER persistent viewport-sized body child — the mermaid zoom
+   modal — carries the same reset on its own rule further down, or opening a
+   diagram would blank window drag (and the macOS double-click-title zoom)
+   while it is up. It is a modal SCRIM, so it does get the reset even though
+   it is interactive: only its widgets (`<button>` …) need to stay no-drag,
+   and the residual is that its backdrop inside the shell's top drag band
+   starts a window drag instead of closing the modal — the deliberate trade
+   for keeping the window draggable while a diagram is enlarged.
+   Deliberately NOT applied to the other interactive popups (`.selectionPopup`
+   is a `<button>`; FloatingWindow / AnchoredPopover own pointer drags and are
+   not scrims): the shell's `:is(button, a, input, …)` rule already keeps those
+   no-drag, and `initial !important` would outrank it and start a window drag on
+   press instead — the click-swallow regression of issues #103/#111. */
 :global(html[data-platform] body > [data-dsh-better-sidebar]),
 :global([data-dsh-panel-host]) {
   -webkit-app-region: initial !important;
diff --git a/tests/e2e/mount.e2e.ts b/tests/e2e/mount.e2e.ts
index ad72eaa..c232fe1 100644
--- a/tests/e2e/mount.e2e.ts
+++ b/tests/e2e/mount.e2e.ts
@@ -517,6 +517,28 @@ test('plugin mounts into the DSH shell and survives a built-in tab sweep', async
   const modal = page.locator('[data-mermaid-modal]')
   await pane.locator('[data-mermaid-diagram] svg').first().click()
   await expect(modal, 'clicking the diagram must open the zoom modal').toHaveCount(1, { timeout: 10_000 })
+  // While it is up, the modal is the plugin's OTHER viewport-sized BODY CHILD:
+  // without the app-region reset the shell's blanket `body > :not(#root)`
+  // no-drag makes it cancel every window-drag strip (and the macOS
+  // double-click-title zoom) until it closes — issue #772. Measured against the
+  // SHELL's own darwin rules (present in this page's stylesheet, activated by
+  // the platform attribute) rather than a copy: the modal must compute the
+  // neutral `none`, while its toolbar buttons keep `no-drag` so a click on them
+  // is never a window drag.
+  const modalRegions = await page.evaluate(() => {
+    const previous = document.documentElement.dataset.platform
+    document.documentElement.dataset.platform = 'darwin'
+    const modalElement = document.querySelector('[data-mermaid-modal]')
+    const region = (element: Element | null): string => element === null
+      ? 'missing'
+      : getComputedStyle(element).getPropertyValue('-webkit-app-region').trim()
+    const result = { modal: region(modalElement), button: region(modalElement?.querySelector('button') ?? null) }
+    if (previous === undefined) delete document.documentElement.dataset.platform
+    else document.documentElement.dataset.platform = previous
+    return result
+  })
+  expect(modalRegions.modal, 'the zoom modal must opt out of app-region computation').toBe('none')
+  expect(modalRegions.button, 'its buttons must stay no-drag').toBe('no-drag')
   await page.keyboard.press('Escape')
   await expect(modal, 'Esc must close the zoom modal').toHaveCount(0, { timeout: 10_000 })
   await assertNoCrash()
@@ -615,15 +637,17 @@ test('conservative auto: URL stamps alone never modify the layout; plugin chrome
     return false
   })
   expect(hasNoDragRule, 'the bundle must ship the drag-region opt-out rule').toBe(true)
-  // The viewport-sized panel host must NOT inherit the shell's blanket
-  // `body > :not(#root) { no-drag }` — it would cancel every window-drag
-  // strip beneath it on macOS (issue #772). A text match only proves the
-  // declaration shipped, so this probe reproduces the SHELL's two darwin
-  // rules verbatim, flips the platform attribute the shell stamps, and reads
-  // the COMPUTED property: app-region is inherited and ignores
-  // pointer-events, so the loss shows on the host/layer — never on the strip
-  // itself (it computes `drag` in every configuration, which is why a fix
-  // validated only against the strip proves nothing).
+  // The plugin host is a direct body child, so on macOS it picks up the
+  // shell's blanket `body > :not(#root) { no-drag }` and — because app-region
+  // ignores pointer-events — the viewport-sized layer inside it cancelled every
+  // window-drag strip beneath it (issue #772). A text match only proves the
+  // declaration shipped, so this probe reproduces the SHELL's two darwin rules
+  // verbatim, flips the platform attribute the shell stamps, and reads the
+  // COMPUTED property. Two things to know when reading the expectations below:
+  // the strip itself computes `drag` in every configuration (a fix validated
+  // against the strip alone proves nothing), and `[data-dsh-panel-host]` is NOT
+  // a body child (it lives inside the host), so its own value is pinned by the
+  // unit guard rather than by this probe — see the `layer` expectation.
   const regions = await page.evaluate(() => {
     const style = document.createElement('style')
     style.textContent = [
@@ -660,8 +684,13 @@ test('conservative auto: URL stamps alone never modify the layout; plugin chrome
     return result
   })
   expect(regions.strip, 'the drag strip itself must keep computing `drag`').toBe('drag')
-  expect(regions.host, 'the plugin host must not inherit the shell no-drag').toBe('none')
-  expect(regions.layer, 'the panel host must not inherit the shell no-drag').toBe('none')
+  expect(regions.host, 'the plugin host must not take the shell blanket no-drag').toBe('none')
+  // NOT discriminating here, and kept as a regression nail only: the layer is
+  // not a body child, and this engine does not propagate the property into it,
+  // so it reads `none` pre- and post-fix. Its rule is pinned by
+  // tests/panel-host-css.spec.ts; in Electron (#772's CDP reading) the layer
+  // DOES end up no-drag through that propagation.
+  expect(regions.layer, 'the panel layer must keep its own reset').toBe('none')
   expect(regions.layerChild, 'panels stay no-drag so their controls keep receiving clicks').toBe('no-drag')
 })
 
diff --git a/tests/panel-host-css.spec.ts b/tests/panel-host-css.spec.ts
index f57f298..19da378 100644
--- a/tests/panel-host-css.spec.ts
+++ b/tests/panel-host-css.spec.ts
@@ -49,12 +49,14 @@ describe('panel host layer css', () => {
     // The shell's `html[data-platform=darwin] body > :not(#root) { no-drag }`
     // carries an id in its specificity, so only `initial !important` wins;
     // `initial` computes to the NEUTRAL `none` (does not subtract from the
-    // drag region) while `no-drag` is the subtracting value — hence the
-    // split: decorative viewport-sized layers opt out, their panels stay
-    // no-drag so controls keep receiving clicks. Dropping the `!important`
-    // silently re-breaks window drag / double-click-title zoom on macOS
-    // (there is no macOS runner to catch it), so pin the shape here where
-    // Linux `pnpm test` fails instead.
+    // drag region) while `no-drag` is the subtracting value — and the literal
+    // keyword `none` is NOT neutral (it computes to `no-drag`), which is
+    // exactly why this guard pins `initial`. Hence the split: decorative
+    // viewport-sized layers opt out, their panels stay no-drag so controls
+    // keep receiving clicks. Dropping the `!important` silently re-breaks
+    // window drag / double-click-title zoom on macOS (there is no macOS
+    // runner to catch it), so pin the shape here where Linux `pnpm test`
+    // fails instead.
     const text = stripComments(css)
     expect(text, 'the panel host must opt out of app-region computation').toMatch(
       /:global\(\[data-dsh-panel-host\]\)\s*\{[^}]*-webkit-app-region:\s*initial\s*!important/,
@@ -62,12 +64,21 @@ describe('panel host layer css', () => {
     expect(text, 'panels must stay no-drag').toMatch(
       /:global\(\[data-dsh-panel-host\]\)\s*>\s*\*\s*\{[^}]*-webkit-app-region:\s*no-drag/,
     )
-    expect(text, 'the mermaid zoom modal is the other viewport-sized body child').toMatch(
+    expect(text, 'the mermaid zoom modal is the other persistent viewport-sized body child').toMatch(
       /\.mermaidModal\s*\{[^}]*-webkit-app-region:\s*initial\s*!important/,
     )
     // The interactive popups must NOT get `initial`: they are buttons/handles
     // the shell already keeps no-drag, and outranking that rule makes a press
-    // start a window drag (the #103/#111 click-swallow class).
-    expect(text).not.toMatch(/\.selectionPopup\s*\{[^}]*-webkit-app-region:\s*initial/)
+    // start a window drag (the #103/#111 click-swallow class). Checked per
+    // DECLARATION BLOCK, not per single-selector rule: a selector list
+    // (`.selectionPopup, .other { … initial !important }`) would slip past a
+    // simple "class immediately followed by {" pattern.
+    const resets = [...text.matchAll(/([^{}]+)\{([^{}]*)\}/g)]
+      .filter(([, , body]) => /-webkit-app-region:\s*initial/.test(body ?? ''))
+    expect(resets.length, 'the file must declare the app-region opt-out').toBeGreaterThan(0)
+    const resetSelectors = resets.map(([, selector]) => selector ?? '').join(',')
+    for (const excluded of ['selectionPopup']) {
+      expect(resetSelectors, `${excluded} must not opt out of app-region`).not.toContain(excluded)
+    }
   })
 })