fix(html): make code-tools toggle work when code-copy is enabled - #14707
Conversation
The code-copy scaffold added in 1.8.15 wraps each code block in a div.code-copy-outer-scaffold, which left the "Show All Code" and "Hide All Code" handler matching nothing: its selectors assumed .sourceCode was a direct child of <details> and of .cell, and it reached the <details> via .parentElement. Key the selectors off details.code-fold and div.sourceCode.cell-code instead, which the scaffold cannot displace. Apply the same selector to the server-side toggle detection, which until now only worked because that post-processor happens to run before the scaffold is inserted.
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
cderv
left a comment
There was a problem hiding this comment.
Thanks @mcanouil.
One thing before merge: we're missing a test that actually exercises the toggle. The smoke fixture locks the DOM shape, but since this PR doesn't touch the render pipeline, the pre-fix version produces identical HTML — so the fixture passes with or without the fix. I confirmed it by reverting the two changed files to their pre-fix state and re-running the new fixture still passes green, so it wouldn't have caught the original regression.
The behavior is pure client-side JS, so it needs a Playwright test that clicks Show All Code / Hide All Code and asserts the <details> open/close and the hidden↔unhidden swap on hidden cells, with code-copy enabled. I'll add that before merging.
…rto-dev#13583) The smoke fixture only asserts on rendered HTML, which is byte-identical before and after this fix (the render pipeline is untouched), so it passes on the buggy selectors too and cannot guard the behavior. This Playwright test exercises the actual client-side JS: it clicks Show All Code / Hide All Code and asserts every <details> opens/closes and the echo:false cell's source swaps hidden<->unhidden, with code-copy enabled. It fails on the pre-fix selectors (details never open) and passes on the fix. Uses an {ojs} cell so the fixture needs no Python/R kernel.
|
Follow-up: added the Playwright test and pushed it to the branch.
It's a real regression guard: it fails on the pre-fix code (the |
|
Thanks, not yet used to playwright testing. |
|
No problem. 🤖 should know how to write them with the context present. We don't need always some playwright, but for things that impact JS usually we need to verify if the regression can be tested without in-browser behavior or not. Some times our Deno test framework can't test. I'll merge. Thanks again ! |
|
Claude might have caught this, unfortunately, I did it without (quite limited subscription at the moment) |
The
code-copyscaffold introduced in 1.8.15 wraps each code block in adiv.code-copy-outer-scaffold, which broke the "Show All Code" / "Hide All Code" handler's direct-child selectors. The menu rendered but clicks did nothing. Selectors now key offdetails.code-foldanddiv.sourceCode.cell-code, which the scaffold cannot displace.<details>.echo: falsecells underkeep-hidden: true, broken the same way but not reported (found/verified during Claude's review).filename, and folded blocks inside tabsets, callouts and listings, which the old direct-child selector never reached.Verified in headless Chromium by clicking both menu items on a rendered document: all folded blocks open and close together and the hidden block flips both ways. Patching the selectors back to their previous form on the same page reproduces the inert behaviour.
Closes #13583.