#2212·read-frog

[BUG] Attribute/reveal route still walks a revealed node as its own walk root inside a translated paragraph (#2185 for the notranslate family)

Author: mengxi-reamCreated Sep 18, 2026Updated Sep 18, 2026
Labelsbugon-hold

Summary

PR #2186 stops handleMutationRecords from walking an added node as its own walk root when that node sits inside a registered bilingual source — the #2185 defect. The sibling attribute/reveal branch was left unguarded, deliberately, and it reproduces the same defect for one family of blockers: an element that stops matching isDontWalkIntoButTranslateAsChildElement (the notranslate family) while inside an already-translated paragraph.

The naive fix — applying the same guard there — is wrong and would strand content. The correct fix is narrower. This issue records both.

References are against cdcee15c (head of #2186).

Where

findEnclosingBilingualLayoutSource is imported and called in exactly one place in src: page-translation.ts:1192, inside the rec.type === "childList" branch. The sibling branch has no such check:

typescript
} else if (this.isWalkabilityAttributeMutation(rec)) {
  const el = rec.target
  if (!isHTMLElement(el)) continue
  for (const unblocked of this.collectNewlyWalkableSubtrees(el, config)) {
    void this.observeTopLevelParagraphs(unblocked, config)   // ← fresh walk root

observeTopLevelParagraphs is exactly the "fresh walk root" primitive #2186 identifies as the root cause: it computes "top-level paragraph" relative to its walk root, and walkNode (traversal.ts:185-190) labels any element with a non-blank direct Text child, inline spans included.

Nothing downstream rejects the promoted unit:

  • observeParagraphUnit (page-translation.ts:752-800) does no enclosing-source check.
  • The walker's already-translated guard (translation-walker.ts:62-66) is element.querySelector(...) — self and descendants only. The enclosing wrapper is a sibling of the revealed span, so it does not fire.
  • The run-level wrapper lookup at translation-modes.ts:1060 is parentNode.closest(...), which is null for a span whose sibling holds the wrapper.

Verified by execution (temporary jsdom probe against page-translation-mutations.test.ts, since removed): revealing a cached-blocked span inside a registered source produces a fresh walk root on the span, sets data-read-frog-paragraph on it, calls IntersectionObserver.observe(span), and schedules zero retranslations of the enclosing source.

Why the enclosing source never rescues it

This is the part that makes the obvious fix wrong.

collectHostText (translation-state.ts:277) counts hidden and notranslate text — it only excludes translation wrappers. But collectRawSource (paragraph-segmentation.ts:173-188) drops isDontWalkIntoAndDontTranslateAsChildElement children entirely.

So revealing a blocked element changes what is translatable without changing collectHostText. The enclosing source stays current, findStaleBilingualLayoutSource never resolves it, and retranslateChangedSource is never scheduled.

That cuts both ways, and the two blocker families need opposite treatment:

revealed element its text before the reveal walking it is
display:none / hidden family (isDontWalkIntoAndDontTranslateAsChildElement) never in the enclosing translation a rescue — must keep happening
notranslate family (isDontWalkIntoButTranslateAsChildElement) already rendered by the enclosing wrapper, verbatim, as a preserved atomic chunk (paragraph-segmentation.ts:184-188) a duplicate — the bug

Applying findEnclosingBilingualLayoutSource wholesale here would kill the first row — revealed accordion / "show more" content would be stranded permanently, exactly the dead zone #2186's own comments warn about for translationOnly.

Reproduction

xml
<p id="summary">Original <span id="brand" class="notranslate">Acme Corp</span> summary</p>
  1. Translate the page in bilingual mode. #summary gets one wrapper; "Acme Corp" appears verbatim inside it (preserved atomic chunk).
  2. document.getElementById('brand').classList.remove('notranslate')
  3. #brand transitions blocked → walkable, collectNewlyWalkableSubtrees returns it, and it is walked as its own root.
  4. Count document.querySelectorAll('#summary .read-frog-translated-content-wrapper').length — expected 1, observed 2, the second inside #summary at #brand, re-translating a fragment the first wrapper already rendered.

Note #brand must be in walkBlockedElementsCache for didChangeToWalkable (page-translation.ts:907-919) to return true. That happens on the initial walk via onBlockedElement (page-translation.ts:699), so a pre-existing blocked descendant qualifies — the addition does not need to be new.

Not confirmed in a real browser. Step 4's wrapper count is inferred from the guard analysis above; the jsdom probe could not prove it because translateNodesBilingualMode is mocked in that suite. Worth confirming before closing.

Proposed fix

Inside the collectNewlyWalkableSubtrees loop, skip observeTopLevelParagraphs(unblocked, config) when both:

  1. findEnclosingBilingualLayoutSource(unblocked) resolves, and
  2. the element was blocked by the isDontWalkIntoButTranslateAsChildElement family — i.e. its text was already folded into the enclosing source's chunk stream.

Keep observeIsolatedDescendantsMutations(unblocked, config) either way, as the childList branch does.

Condition (2) needs the reason an element was cached as blocked, which is not currently stored — walkBlockedElementsCache (page-translation.ts:145) is a plain WeakSet. Note cacheWalkBlockedElement has five call sites (:699, :913, :928, :1080, :1375), and two of them (:928, :1080) decide "blocked" through the collapsed isWalkBlockedElement boolean with no reason available to thread. So threading a reason through onBlockedElement only covers one call site; recomputing the family at cache time is the only option that covers all five.

An alternative worth weighing (not validated): force the enclosing source stale on this transition and let retranslateChangedSource re-run it. That would repair placement too, but its interaction with the #1831 tamper/churn caps was not checked, and retranslateChangedSource's behavior when collectHostText is unchanged was not traced.

Suggested tests

In page-translation-mutations.test.ts — its mock walk now mirrors the real labelling rule, so an inline span can be promoted under it:

  • Registered source containing a blocked span, span revealed → assert observer.observe is never called with the span, and the enclosing source is not retranslated.
  • Same shape with a hidden-family blocker → assert the span is walked (the rescue must survive).

Two hazards:

  • Prefer a distinct class (e.g. np) over the literal notranslate when configuring mockIsDontWalkIntoButTranslateAsChildElement; two existing tests do key on the literal and compensate with a mockHasNoWalkAncestor implementation, which is easy to get subtly wrong.
  • The existing retains blocked stale sources until %s case reveals an ancestor of a registered source, not a descendant inside one — it is not a counterexample to "no coverage for a reveal inside a registered source".

Confidence

The code chain is verified line by line at cdcee15c, plus one executed jsdom probe (above). Real-world frequency is unmeasured — I have no site that demonstrably removes a notranslate class mid-session. The display:none/hidden family is far more common and is not affected, so this is plausibly rarer than #2185 itself. Prioritize accordingly.

Follow-up to #2186 / #2185.