fix(browser): 修复区域框选的三处 review 问题

.scratch/browser-annotation-v2/PRD.md

@@ -92,6 +92,7 @@ Codex 默认是 Enter 直发、`Cmd+Enter` 攒（#22719 下大量反对）；Pac
 - 区域不「占有」锚点：点击锚点元素仍新建元素评论，多个区域可共享同一锚点。
 - 拖动中 Esc / `pointercancel` / 松开在窗口外（`buttons === 0`）都取消框选，随后的 click 被吞；任一边小于 4px 视为误触。已保存区域用虚线轮廓，元素用实线。模式标签改为「Click or drag to annotate · Esc to exit」。
 - 格式化：区域条目标题为 `#n [screenshot] area in \`selector\` (tag)`，**总是**输出 `- area: W×H at (x, y)`（截图有外扩边距、无框线，模型需要确切边界）；composer chip 为 `#n Area · 摘要`。
+- （#455 review 修订）`area` 的偏移按锚点滚动内容计：保存时加上锚点 `scrollLeft`/`scrollTop`（锚点为文档滚动元素时除外，其 rect 已随页面滚动），否则锚点自身滚动内容时轮廓会漂。带截图的区域条目同时给出截图内坐标与视口坐标（`- area: W×H at (sx, sy) in its screenshot; (x, y) in the viewport`），截图按 `resolveCommentCropRect`（已移入 `packages/core`）的同样规则裁出。
 
 ## 切片
 

apps/desktop/electron/browser-annotation-overlay.test.ts

@@ -1624,4 +1624,106 @@ describe("annotation overlay — area selection", () => {
       vi.useRealTimers();
     }
   });
+
+  it("stores the anchor's own scroll in the offset when the area is saved", async () => {
+    const h = harness();
+
+    document.body.innerHTML =
+      '<div id="scrollbox"><p id="start">inside</p></div>';
+    const box = place(document.getElementById("scrollbox")!, 40, 40, 200, 200);
+    const start = place(document.getElementById("start")!, 50, 50, 20, 10);
+    vi.spyOn(box, "scrollTop", "get").mockReturnValue(100);
+
+    h.overlay.setDesignMode(true);
+    drag(start, 60, 60, 160, 110);
+    typeDraft(h, "inside the scroller");
+    pressKey(editorInput(h), "Enter");
+    await nextFrames();
+
+    // The box does not move when its content scrolls, so the offset has to
+    // carry scrollTop to keep naming the same content.
+    expect(h.saved[0]!.annotation).toMatchObject({
+      selector: "#scrollbox",
+      rect: { x: 60, y: 60, width: 100, height: 50 },
+      area: { x: 20, y: 120, width: 100, height: 50 },
+    });
+  });
+
+  it("moves the outline with the content when the anchor scrolls its content", async () => {
+    const h = harness();
+
+    document.body.innerHTML = '<div id="scrollbox"><p>x</p></div>';
+    const box = place(document.getElementById("scrollbox")!, 40, 40, 200, 200);
+    const scrollTop = vi.spyOn(box, "scrollTop", "get").mockReturnValue(100);
+
+    // area.y = 120 in content coords: 40 + 120 - 100 = 60 on screen.
+    h.overlay.syncAnnotations([
+      {
+        ...areaAnnotation("srv-a", 1, "#scrollbox"),
+        tag: "div",
+        rect: { x: 50, y: 60, width: 40, height: 20 },
+        area: { x: 10, y: 120, width: 40, height: 20 },
+      },
+    ]);
+
+    const outline = h.shadow().querySelector<HTMLElement>(
+      '[data-slot="annotation-outline"]',
+    )!;
+    expect(outline.style.top).toBe("60px");
+
+    // The anchor's rect is unchanged but its content moved: a mark pinned
+    // to the box alone would slide over different content.
+    scrollTop.mockReturnValue(130);
+    window.dispatchEvent(new Event("scroll"));
+    await nextFrames();
+
+    expect(outline.style.top).toBe("30px");
+  });
+
+  it("lets a badge open after a drag's trailing click landed on it", async () => {
+    const h = harness();
+    place(document.querySelector("main")!, 0, 0, 600, 400);
+    const button = place(document.getElementById("cta")!);
+
+    h.overlay.setDesignMode(true);
+    await annotate(h, button, "saved mark");
+    const badge = badges(h)[0]!;
+
+    // A drag released under the area threshold, with its trailing click
+    // landing on a saved badge: the badge swallows it — and the swallow
+    // clears the flag itself, so the next click is not eaten.
+    pointer(button, "pointerdown", 50, 50, { button: 0, buttons: 1 });
+    pointer(button, "pointermove", 200, 150, { buttons: 1 });
+    pointer(button, "pointerup", 200, 52, { button: 0, buttons: 0 });
+    badge.click();
+
+    expect(editorAbsent(h)).toBe(true);
+
+    badge.click();
+
+    expect(editorVisible(h)).toBe(true);
+    expect(editorInput(h).value).toBe("saved mark");
+  });
+
+  it("re-arms the click swallow on a pointerdown over overlay chrome", async () => {
+    const h = harness();
+    place(document.querySelector("main")!, 0, 0, 600, 400);
+    const button = place(document.getElementById("cta")!);
+
+    h.overlay.setDesignMode(true);
+    await annotate(h, button, "saved mark");
+    const badge = badges(h)[0]!;
+
+    // Same dead drag, but the next gesture starts on the badge itself: a
+    // pointerdown over overlay chrome must still reset the swallow — the
+    // page-target early return cannot be where it is re-armed.
+    pointer(button, "pointerdown", 50, 50, { button: 0, buttons: 1 });
+    pointer(button, "pointermove", 200, 150, { buttons: 1 });
+    pointer(button, "pointerup", 200, 52, { button: 0, buttons: 0 });
+    pointer(badge, "pointerdown", 0, 0, { button: 0, buttons: 1 });
+    badge.click();
+
+    expect(editorVisible(h)).toBe(true);
+    expect(editorInput(h).value).toBe("saved mark");
+  });
 });

apps/desktop/electron/browser-annotation-overlay.ts

@@ -1,4 +1,5 @@
 import {
+  anchorScroll,
   describeAnnotatedArea,
   describeAnnotatedElement,
   maxCommentLength,
@@ -870,16 +871,19 @@ export function createAnnotationOverlay(options: {
    * The mark's viewport rect: the element's own box, or for an area the box
    * the saved offset lands at from wherever the anchor re-measures — what
    * keeps a region pinned to its place inside a scrolled or reflowed anchor.
+   * Offsets are stored in the anchor's scrolled-content coordinates, so its
+   * own scroll comes back out here.
    */
   function markedRect(
     element: Element,
     area: BrowserAnnotationElement["area"] | null,
   ): ViewportRect {
     const anchor = element.getBoundingClientRect();
+    const scroll = area ? anchorScroll(element) : { x: 0, y: 0 };
 
     return toViewportRect(
-      anchor.left + (area?.x ?? 0),
-      anchor.top + (area?.y ?? 0),
+      anchor.left + (area?.x ?? 0) - scroll.x,
+      anchor.top + (area?.y ?? 0) - scroll.y,
       area?.width ?? anchor.width,
       area?.height ?? anchor.height,
     );
@@ -1111,8 +1115,10 @@ export function createAnnotationOverlay(options: {
     styleBadge(badge);
     badge.addEventListener("click", () => {
       // A drag releasing over a badge fires this too — its click is
-      // swallowed like the one that lands on page elements.
+      // swallowed like the one that lands on page elements. The flag is
+      // consumed, not held: it guards only this one trailing click.
       if (suppressClick) {
+        suppressClick = false;
         return;
       }
       const entry = entries.get(annotation.id);
@@ -1651,6 +1657,13 @@ export function createAnnotationOverlay(options: {
       return;
     }
 
+    // A new press re-arms the trailing-click swallow even when it lands on
+    // our own chrome — the early return below must not leave a stale flag
+    // eating the badge's next click.
+    if (event.type === "pointerdown") {
+      suppressClick = false;
+    }
+
     const target = pageTarget(event);
 
     // Our own badges and editor have to keep working, so their events are
@@ -1665,8 +1678,7 @@ export function createAnnotationOverlay(options: {
 
     if (event.type === "pointerdown") {
       // A press on a page target arms the drag: whether it stays a click is
-      // the next pointermove's call. The trailing-click swallow re-arms too.
-      suppressClick = false;
+      // the next pointermove's call.
       const pointer = event as MouseEvent;
       if (pointer.button === 0) {
         dragStart = { x: pointer.clientX, y: pointer.clientY, target };
@@ -1730,10 +1742,13 @@ export function createAnnotationOverlay(options: {
 
     const anchor = resolveAreaAnchor(start.target, rect);
     const anchorRect = anchor.getBoundingClientRect();
+    // Scrolled-content coordinates, same rule as describeAnnotatedArea: the
+    // anchor's own scroll belongs to the offset, not the box position.
+    const scroll = anchorScroll(anchor);
 
     requestEditor(anchor, undefined, {
-      x: rect.left - anchorRect.left,
-      y: rect.top - anchorRect.top,
+      x: rect.left - anchorRect.left + scroll.x,
+      y: rect.top - anchorRect.top + scroll.y,
       width: rect.width,
       height: rect.height,
     });

apps/desktop/electron/browser-annotation.test.ts

@@ -220,6 +220,78 @@ describe("describeAnnotatedArea", () => {
     // The anchor's text would describe the element, not the marked region.
     expect(annotation).not.toHaveProperty("text");
   });
+
+  it("counts the anchor's own scroll in the offset — the mark tracks content, not the box", () => {
+    mount(
+      '<div id="scrollbox" style="overflow:auto"><p>inside</p></div>',
+    );
+    const anchor = document.getElementById("scrollbox")!;
+    vi.spyOn(anchor, "getBoundingClientRect").mockReturnValue({
+      x: 40,
+      y: 40,
+      left: 40,
+      top: 40,
+      width: 200,
+      height: 200,
+      right: 240,
+      bottom: 240,
+      toJSON: () => ({}),
+    } as DOMRect);
+    vi.spyOn(anchor, "scrollTop", "get").mockReturnValue(120);
+    vi.spyOn(anchor, "scrollLeft", "get").mockReturnValue(10);
+
+    // A scrolled anchor's border box does not move — without the scroll
+    // term the offset would point at different content after every scroll.
+    const annotation = describeAnnotatedArea(
+      anchor,
+      { left: 60, top: 90, width: 50, height: 30 },
+      1,
+      "a1",
+    );
+
+    expect(annotation.area).toEqual({ x: 30, y: 170, width: 50, height: 30 });
+  });
+
+  it("skips the scroll term when the anchor is the document's scrolling element", () => {
+    // The scrolling element's rect already moves with the page scroll, so
+    // adding scrollTop would count it twice. jsdom leaves
+    // document.scrollingElement unimplemented — it is mocked directly.
+    const anchor = document.documentElement;
+    vi.spyOn(anchor, "getBoundingClientRect").mockReturnValue({
+      x: 0,
+      y: -100,
+      left: 0,
+      top: -100,
+      width: 800,
+      height: 800,
+      right: 800,
+      bottom: 700,
+      toJSON: () => ({}),
+    } as DOMRect);
+    vi.spyOn(anchor, "scrollTop", "get").mockReturnValue(100);
+    const own = Object.getOwnPropertyDescriptor(document, "scrollingElement");
+    Object.defineProperty(document, "scrollingElement", {
+      configurable: true,
+      value: anchor,
+    });
+
+    try {
+      const annotation = describeAnnotatedArea(
+        anchor,
+        { left: 10, top: 20, width: 50, height: 30 },
+        1,
+        "a1",
+      );
+
+      expect(annotation.area).toEqual({ x: 10, y: 120, width: 50, height: 30 });
+    } finally {
+      if (own) {
+        Object.defineProperty(document, "scrollingElement", own);
+      } else {
+        Reflect.deleteProperty(document, "scrollingElement");
+      }
+    }
+  });
 });
 
 describe("acceptBrowserAnnotationMessage", () => {

apps/desktop/electron/browser-annotation.ts

@@ -306,12 +306,25 @@ export function describeAnnotatedElement(
   };
 }
 
+/**
+ * How much of an anchor's scrolled content sits hidden above-left of its
+ * border box: its own scroll offsets — except for the document's scrolling
+ * element, whose box already rides the page scroll, so adding its scrollTop
+ * would count the same scroll twice.
+ */
+export function anchorScroll(anchor: Element) {
+  return anchor === anchor.ownerDocument.scrollingElement
+    ? { x: 0, y: 0 }
+    : { x: anchor.scrollLeft, y: anchor.scrollTop };
+}
+
 /**
  * An area comment: `selector`/`tag`/`source` describe the anchor — the
  * smallest element fully containing the dragged rect — while `rect` is the
- * area's own viewport box and `area` its offset from the anchor's, so a
- * restore can re-derive the rect from wherever the anchor re-measures. No
- * `text`: the anchor's content would describe the element, not the region.
+ * area's own viewport box and `area` its offset from the anchor's border
+ * box in scrolled-content coordinates (`anchorScroll`, so the mark tracks
+ * the content, not the box). No `text`: the anchor's content would describe
+ * the element, not the region.
  */
 export function describeAnnotatedArea(
   anchor: Element,
@@ -320,6 +333,7 @@ export function describeAnnotatedArea(
   id: string,
 ): BrowserAnnotationElement {
   const anchorRect = anchor.getBoundingClientRect();
+  const scroll = anchorScroll(anchor);
   const source = readSource(anchor);
 
   return {
@@ -334,8 +348,8 @@ export function describeAnnotatedArea(
       height: Math.round(areaRect.height),
     },
     area: {
-      x: Math.round(areaRect.left - anchorRect.left),
-      y: Math.round(areaRect.top - anchorRect.top),
+      x: Math.round(areaRect.left - anchorRect.left + scroll.x),
+      y: Math.round(areaRect.top - anchorRect.top + scroll.y),
       width: Math.round(areaRect.width),
       height: Math.round(areaRect.height),
     },

apps/desktop/electron/browser-host.test.ts

@@ -14,7 +14,6 @@ import {
   maxSessionComments,
   normalizeBrowserUrl,
   resolveBrowserViewBounds,
-  resolveCommentCropRect,
   urlKey,
   type BrowserHostView,
 } from "./browser-host";
@@ -1258,40 +1257,4 @@ describe("urlKey", () => {
   });
 });
 
-describe("resolveCommentCropRect", () => {
-  it("expands the element by 48px and grows to the minimum size", () => {
-    expect(
-      resolveCommentCropRect(
-        { x: 400, y: 300, width: 40, height: 20 },
-        { width: 1440, height: 900 },
-      ),
-    ).toEqual({ x: 260, y: 210, width: 320, height: 200 });
-  });
-
-  it("keeps a large element's expanded bounds", () => {
-    expect(
-      resolveCommentCropRect(
-        { x: 400, y: 300, width: 500, height: 400 },
-        { width: 1440, height: 900 },
-      ),
-    ).toEqual({ x: 352, y: 252, width: 596, height: 496 });
-  });
 
-  it("shifts an off-edge crop inside before shrinking it", () => {
-    expect(
-      resolveCommentCropRect(
-        { x: 1380, y: 10, width: 40, height: 20 },
-        { width: 1440, height: 900 },
-      ),
-    ).toEqual({ x: 1120, y: 0, width: 320, height: 200 });
-  });
-
-  it("clips a crop bigger than the viewport to it", () => {
-    expect(
-      resolveCommentCropRect(
-        { x: 100, y: 100, width: 900, height: 800 },
-        { width: 400, height: 300 },
-      ),
-    ).toEqual({ x: 0, y: 0, width: 400, height: 300 });
-  });
-});