fix(translate): keep bilingual translations beside tall floats instead of stranding them below (#2047)

A block translation renders as `display: inline-block` so its decoration hugs
the text. That makes it an atomic inline: when a float leaves the line too
narrow, the browser drops the whole box below the float instead of wrapping
text beside it. Against a tall float the translation is stranded that far
below its own paragraph, stretching the paragraph into a page-tall blank gap.

#1188 added a `data-read-frog-float-wrap` override for this, but decided it by
scanning the paragraph's DOM siblings for a floated element. A float only has
to share a block formatting context to intrude — it does not have to be near
the paragraph in the tree. On ja.wikipedia the body is
`div.mw-parser-output > section > section > p` while the infobox floats out of
the outer section's sibling, so the sibling scan never saw it and every
paragraph below the lead section was left displaced. Measured on the 毛沢東
article: the 生い立ち translation landed 5643px below its paragraph.

Decide by measuring the rendered result instead. A Range spanning everything
before the wrapper gives the source text's last-line bottom; if the translated
node sits more than `margin-top + line-height` below it, the box was displaced.
That is structure-agnostic, so ancestor and cousin floats are covered too, and
it costs two rect reads instead of a per-sibling subtree scan.

`flowSource` existed only to locate the paragraph for the old scan, so its
plumbing is removed.

Verified on ja.wikipedia 毛沢東 (bilingual, zh): worst gap across all
translated paragraphs 5643px -> 14px, zero stranded. Line boxes confirm the
mechanism — 859px (full column, ignoring the float) -> 515px (shortened beside
the infobox). Instrumented rAF shows all 14 float-wrapped nodes are decided
before the first paint, so no displaced frame is ever shown.

jsdom implements no layout and omits `Range.prototype.getBoundingClientRect`
entirely, so vitest.setup.ts stubs it (guarded — that file also loads for
node-environment tests).

Co-authored-by: ananaBMaster <68643891+ananaBMaster@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

.changeset/olive-pugs-attack.md

@@ -0,0 +1,5 @@
+---
+"@read-frog/extension": patch
+---
+
+fix: keep bilingual translations beside tall floats instead of stranding them below

src/utils/host/__tests__/translate.integration.test.tsx

@@ -1834,8 +1834,38 @@ describe("translate", () => {
           expectNodeLabels(node.children[1]!, [BLOCK_ATTRIBUTE, PARAGRAPH_ATTRIBUTE])
         })
       })
-      describe("block translations beside floated siblings", () => {
-        it("bilingual mode: marks block translation for float wrap when inline-block would drop below the float", async () => {
+      describe("block translations displaced below a float", () => {
+        // The translation is an inline-block: a float can push the whole box
+        // below itself instead of wrapping text beside it. Detection is by
+        // measurement, so these tests drive the two rects it reads — the source
+        // text preceding the wrapper (a Range) and the translated node.
+        function mockLayout({
+          sourceBottom,
+          translatedTop,
+        }: {
+          sourceBottom: number
+          translatedTop: number
+        }) {
+          const rangeSpy = vi
+            .spyOn(Range.prototype, "getBoundingClientRect")
+            .mockImplementation(() =>
+              createRect({ top: sourceBottom - 20, left: 0, width: 600, height: 20 }),
+            )
+          const rectSpy = vi
+            .spyOn(HTMLElement.prototype, "getBoundingClientRect")
+            .mockImplementation(function (this: HTMLElement) {
+              if (this.classList.contains(BLOCK_CONTENT_CLASS)) {
+                return createRect({ top: translatedTop, left: 0, width: 500, height: 40 })
+              }
+              return createRect({ top: 0, left: 0, width: 200, height: 20 })
+            })
+          return () => {
+            rangeSpy.mockRestore()
+            rectSpy.mockRestore()
+          }
+        }
+
+        it("bilingual mode: marks block translation for float wrap when it drops below the float", async () => {
           render(
             <div data-testid="test-node">
               <figure data-testid="float-node" style={{ float: "right" }}>
@@ -1846,27 +1876,14 @@ describe("translate", () => {
           )
           const node = screen.getByTestId("test-node")
           const paragraph = screen.getByTestId("paragraph")
-          const floatNode = screen.getByTestId("float-node")
-
-          const rectSpy = vi
-            .spyOn(HTMLElement.prototype, "getBoundingClientRect")
-            .mockImplementation(function (this: HTMLElement) {
-              if (this === floatNode) {
-                return createRect({ top: 80, left: 600, width: 200, height: 320 })
-              }
-              if (this === paragraph) {
-                return createRect({ top: 100, left: 0, width: 600, height: 60 })
-              }
-              if (this.classList.contains(BLOCK_CONTENT_CLASS)) {
-                return createRect({ top: 420, left: 0, width: 500, height: 40 })
-              }
-              return createRect({ top: 0, left: 0, width: 200, height: 20 })
-            })
 
+          // Source text ends at 160, translation lands at 420 — far past the
+          // float, well beyond one line of normal spacing.
+          const restore = mockLayout({ sourceBottom: 160, translatedTop: 420 })
           try {
             await removeOrShowPageTranslation("bilingual", true)
           } finally {
-            rectSpy.mockRestore()
+            restore()
           }
 
           expectNodeLabels(node, [BLOCK_ATTRIBUTE])
@@ -1876,7 +1893,7 @@ describe("translate", () => {
           expect(translatedContent).toHaveAttribute(FLOAT_WRAP_ATTRIBUTE, "true")
         })
 
-        it("bilingual mode: leaves block translation unchanged when the translated node stays beside the float", async () => {
+        it("bilingual mode: leaves block translation unchanged when it stays on the next line", async () => {
           render(
             <div data-testid="test-node">
               <figure data-testid="float-node" style={{ float: "right" }}>
@@ -1886,27 +1903,14 @@ describe("translate", () => {
             </div>,
           )
           const paragraph = screen.getByTestId("paragraph")
-          const floatNode = screen.getByTestId("float-node")
-
-          const rectSpy = vi
-            .spyOn(HTMLElement.prototype, "getBoundingClientRect")
-            .mockImplementation(function (this: HTMLElement) {
-              if (this === floatNode) {
-                return createRect({ top: 80, left: 600, width: 200, height: 320 })
-              }
-              if (this === paragraph) {
-                return createRect({ top: 420, left: 0, width: 600, height: 60 })
-              }
-              if (this.classList.contains(BLOCK_CONTENT_CLASS)) {
-                return createRect({ top: 500, left: 0, width: 500, height: 40 })
-              }
-              return createRect({ top: 0, left: 0, width: 200, height: 20 })
-            })
 
+          // Translation opens the line right after the source text: an
+          // ordinary top-margin gap, not a float drop.
+          const restore = mockLayout({ sourceBottom: 160, translatedTop: 168 })
           try {
             await removeOrShowPageTranslation("bilingual", true)
           } finally {
-            rectSpy.mockRestore()
+            restore()
           }
 
           const wrapper = expectTranslationWrapper(paragraph, "bilingual")

src/utils/host/translate/core/translation-modes.ts

@@ -205,7 +205,7 @@ async function translateVirtualParagraph(
   forceBlockTranslation: boolean,
   forceRetranslation: boolean = false,
 ): Promise<void> {
-  const { flowSource, unit, wrapper } = entry
+  const { unit, wrapper } = entry
   const isCurrent = () => isVirtualParagraphGroupCurrent(group, wrapper)
   if (!isCurrent()) return
 
@@ -243,7 +243,7 @@ async function translateVirtualParagraph(
 
   await insertTranslatedNodeIntoWrapper(
     wrapper,
-    { flowSource, isCurrent, layoutSource: group.layoutSource, sourceText: unit.text },
+    { isCurrent, layoutSource: group.layoutSource, sourceText: unit.text },
     translatedText,
     config.translate.translationNodeStyle,
     config,
@@ -312,7 +312,7 @@ async function translateVirtualParagraphs(
 
   let inserted: ReturnType<typeof insertVirtualParagraphWrappers>["inserted"]
   try {
-    ;({ inserted } = insertVirtualParagraphWrappers(entries, layoutSource, group.splitRecords))
+    ;({ inserted } = insertVirtualParagraphWrappers(entries, group.splitRecords))
   } catch (error) {
     disposeVirtualParagraphGroup(group)
     throw error
@@ -650,7 +650,6 @@ export async function translateNodesBilingualMode(
     await insertTranslatedNodeIntoWrapper(
       translatedWrapperNode,
       {
-        flowSource: insertionTarget,
         isCurrent,
         layoutSource,
         styleSources: transNodes,

src/utils/host/translate/dom/__tests__/virtual-paragraph-lifecycle.test.ts

@@ -43,7 +43,7 @@ function createSplitGroup(
     unit: unit(index, source, offset),
     wrapper: wrappers[index]!,
   }))
-  const { splitRecords } = insertVirtualParagraphWrappers(entries, layoutSource)
+  const { splitRecords } = insertVirtualParagraphWrappers(entries)
   const group: VirtualParagraphGroup = {
     id,
     walkId: id,

src/utils/host/translate/dom/translation-insertion.ts

@@ -7,7 +7,6 @@ import {
   FLOAT_WRAP_ATTRIBUTE,
   INLINE_CONTENT_CLASS,
   NOTRANSLATE_CLASS,
-  PARAGRAPH_ATTRIBUTE,
 } from "../../../constants/dom-labels"
 import { isHTMLElement, isNaturalBlockTransNode, isNaturalInlineTransNode } from "../../dom/filter"
 import { getOwnerDocument } from "../../dom/node"
@@ -15,7 +14,6 @@ import { decorateTranslationNode } from "../ui/decorate-translation"
 import { isForceInlineTranslation, isShortInlineTranslationText } from "../ui/translation-utils"
 
 interface TranslationInsertionContext {
-  flowSource: TransNode
   layoutSource: TransNode
   /** Nodes whose source text is represented by this wrapper. */
   styleSources?: readonly TransNode[]
@@ -40,50 +38,68 @@ function sourceRunMatchesSelector(sources: readonly TransNode[], selector: strin
   })
 }
 
-function isFloatedElement(element: HTMLElement): boolean {
-  const floatValue = window.getComputedStyle(element).float
-  return floatValue === "left" || floatValue === "right"
-}
+function resolveLineHeight(style: CSSStyleDeclaration): number | null {
+  const lineHeight = Number.parseFloat(style.lineHeight)
+  if (Number.isFinite(lineHeight) && lineHeight > 0) return lineHeight
 
-function hasVisibleLayoutBox(element: HTMLElement): boolean {
-  const rect = element.getBoundingClientRect()
-  return rect.width > 0 && rect.height > 0
-}
-
-function findActiveFloatSibling(paragraphElement: HTMLElement): HTMLElement | null {
-  const flowContainer = paragraphElement.parentElement
-  if (!flowContainer) return null
-
-  const paragraphRect = paragraphElement.getBoundingClientRect()
-
-  for (const sibling of flowContainer.children) {
-    if (!isHTMLElement(sibling)) continue
-    if (sibling === paragraphElement || sibling.contains(paragraphElement)) continue
-
-    const floatCandidates = [sibling, ...sibling.querySelectorAll<HTMLElement>("*")]
-    for (const candidate of floatCandidates) {
-      if (!isFloatedElement(candidate) || !hasVisibleLayoutBox(candidate)) continue
-
-      const floatRect = candidate.getBoundingClientRect()
-      const verticallyAffectsParagraph =
-        paragraphRect.top < floatRect.bottom - 1 && paragraphRect.bottom > floatRect.top + 1
-      if (verticallyAffectsParagraph) return candidate
-    }
-  }
+  // `line-height: normal` is a keyword, not a length; approximate from the font.
+  const fontSize = Number.parseFloat(style.fontSize)
+  if (Number.isFinite(fontSize) && fontSize > 0) return fontSize * 1.5
 
   return null
 }
 
-function shouldWrapInsideFloatFlow(targetNode: TransNode): boolean {
-  const paragraphElement = isHTMLElement(targetNode)
-    ? targetNode.hasAttribute(PARAGRAPH_ATTRIBUTE)
-      ? targetNode
-      : targetNode.closest<HTMLElement>(`[${PARAGRAPH_ATTRIBUTE}]`)
-    : targetNode.parentElement?.closest<HTMLElement>(`[${PARAGRAPH_ATTRIBUTE}]`)
-  if (!paragraphElement) return false
+/** Bottom of everything that precedes the wrapper inside its parent. */
+function measureContentBottomBeforeWrapper(wrapper: HTMLElement): number | null {
+  const host = wrapper.parentElement
+  if (!host) return null
 
-  const activeFloat = findActiveFloatSibling(paragraphElement)
-  return !!activeFloat
+  const range = getOwnerDocument(host).createRange()
+  range.setStart(host, 0)
+  range.setEndBefore(wrapper)
+  const rect = range.getBoundingClientRect()
+  // Nothing precedes the wrapper (or the host is not laid out): all-zero rect.
+  if (rect.width <= 0 && rect.height <= 0) return null
+
+  return rect.bottom
+}
+
+/**
+ * A block translation renders as `inline-block` (translation-node-preset.css) so
+ * its decoration hugs the text. That makes it an atomic inline: when a float
+ * leaves the line too narrow, the browser drops the entire box below the float
+ * rather than wrapping the text beside it. Against a tall float — a Wikipedia
+ * infobox easily runs a few thousand pixels — the translation is stranded that
+ * far below the paragraph it belongs to, leaving a huge blank gap.
+ *
+ * Detect the drop by measuring where the translation actually landed instead of
+ * hunting for the float in the DOM. The float is frequently nowhere near the
+ * paragraph in the tree: on ja.wikipedia the infobox floats out of a sibling of
+ * an ancestor `<section>`, so a scan of the paragraph's own siblings never sees
+ * it. Layout truth is structure-agnostic and costs two rect reads.
+ */
+function isDisplacedBelowFloat(translatedNode: HTMLElement): boolean {
+  const wrapper = translatedNode.parentElement
+  if (!wrapper) return false
+
+  const contentBottom = measureContentBottomBeforeWrapper(wrapper)
+  if (contentBottom === null) return false
+
+  const translatedRect = translatedNode.getBoundingClientRect()
+  if (translatedRect.height <= 0) return false
+
+  const style = window.getComputedStyle(translatedNode)
+  const lineHeight = resolveLineHeight(style)
+  // Without font metrics there is no scale to judge the gap against, and a zero
+  // threshold would flag every ordinary translation. Leave the layout alone.
+  if (lineHeight === null) return false
+  const marginTop = Number.parseFloat(style.marginTop) || 0
+
+  // Undisplaced, the translation opens the line right after the source text, so
+  // the gap is just its top margin plus line leading. Allowing a whole extra
+  // line keeps normal spacing well clear of the threshold while any real float
+  // drop — at minimum the float's remaining height — stays far above it.
+  return translatedRect.top - contentBottom > marginTop + lineHeight
 }
 
 export function addInlineTranslation(
@@ -110,7 +126,6 @@ export function addBlockTranslation(
 export async function insertTranslatedNodeIntoWrapper(
   translatedWrapperNode: HTMLElement,
   {
-    flowSource,
     layoutSource,
     styleSources,
     sourceText,
@@ -179,7 +194,7 @@ export async function insertTranslatedNodeIntoWrapper(
 
   if (
     translatedNode.classList.contains(BLOCK_CONTENT_CLASS) &&
-    shouldWrapInsideFloatFlow(flowSource)
+    isDisplacedBelowFloat(translatedNode)
   ) {
     translatedNode.setAttribute(FLOAT_WRAP_ATTRIBUTE, "true")
   }

src/utils/host/translate/dom/virtual-paragraph-insertion.ts

@@ -1,17 +1,12 @@
 import type { TextSplitRecord } from "../core/translation-state"
 import type { VirtualParagraphUnit } from "./paragraph-segmentation"
-import type { TransNode } from "@/types/dom"
-import { isTextNode, isTransNode } from "../../dom/filter"
+import { isTextNode } from "../../dom/filter"
 
 export interface VirtualParagraphWrapperEntry {
   unit: VirtualParagraphUnit
   wrapper: HTMLElement
 }
 
-export interface InsertedVirtualParagraph extends VirtualParagraphWrapperEntry {
-  flowSource: TransNode
-}
-
 function insertWrapperAtBoundary(
   { container, offset }: VirtualParagraphUnit["insertionBoundary"],
   wrapper: HTMLElement,
@@ -69,9 +64,8 @@ function insertWrapperAtBoundary(
  */
 export function insertVirtualParagraphWrappers(
   entries: VirtualParagraphWrapperEntry[],
-  layoutSource: HTMLElement,
   splitRecordTarget: TextSplitRecord[] = [],
-): { inserted: InsertedVirtualParagraph[]; splitRecords: TextSplitRecord[] } {
+): { inserted: VirtualParagraphWrapperEntry[]; splitRecords: TextSplitRecord[] } {
   const splitRecords = new Map(splitRecordTarget.map((record) => [record.source, record] as const))
 
   for (const entry of [...entries].reverse()) {
@@ -88,13 +82,5 @@ export function insertVirtualParagraphWrappers(
     record.tailValuesAfterSplit = record.createdTails.map((tail) => tail.data)
   }
 
-  const inserted = entries.map((entry): InsertedVirtualParagraph => {
-    const previousSibling = entry.wrapper.previousSibling
-    return {
-      ...entry,
-      flowSource: previousSibling && isTransNode(previousSibling) ? previousSibling : layoutSource,
-    }
-  })
-
-  return { inserted, splitRecords: splitRecordTarget }
+  return { inserted: entries, splitRecords: splitRecordTarget }
 }

vitest.setup.ts

@@ -109,6 +109,27 @@ vi.mock("wxt/testing/fake-browser", async () => {
   return actual
 })
 
+// jsdom implements no layout, so it omits Range.getBoundingClientRect entirely
+// (Element.getBoundingClientRect it does stub, returning zeros). Every browser
+// ships it. Match jsdom's own convention with a zero rect so layout probes
+// short-circuit instead of throwing; tests that exercise them spy on this.
+// (Guarded: this setup file also runs for node-environment test files.)
+if (typeof Range !== "undefined" && typeof Range.prototype.getBoundingClientRect !== "function") {
+  Range.prototype.getBoundingClientRect = function () {
+    return {
+      top: 0,
+      left: 0,
+      right: 0,
+      bottom: 0,
+      width: 0,
+      height: 0,
+      x: 0,
+      y: 0,
+      toJSON: () => ({}),
+    }
+  }
+}
+
 // JSDom + Vitest don't play well with each other. Long story short - default
 // TextEncoder produces Uint8Array objects that are _different_ from the global
 // Uint8Array objects, so some functions that compare their types explode.