Skip to content

Web UI: duplicate a tab on middle/Ctrl/Shift/Cmd click of its title#109357

Merged
alexey-milovidov merged 15 commits into
masterfrom
web-ui-duplicate-tab
Jul 17, 2026
Merged

Web UI: duplicate a tab on middle/Ctrl/Shift/Cmd click of its title#109357
alexey-milovidov merged 15 commits into
masterfrom
web-ui-duplicate-tab

Conversation

@alexey-milovidov

@alexey-milovidov alexey-milovidov commented Jul 3, 2026

Copy link
Copy Markdown
Member

In the Web UI (play.html), middle-clicking a tab's title, or Ctrl/Shift/Cmd+clicking it (the conventional browser gesture for "open in a new tab"), now duplicates the tab. The copy carries over the query and parameters, is inserted right after the source tab, and becomes active.

The duplicate's title is the source title with a " N" suffix, N starting at 1 and incrementing until unique. The full source title is always used as the base — no trailing numeric suffix is stripped — so duplicating "Query A 2" yields "Query A 2 1", and a user-authored numeric suffix is preserved ("Incident 2025" becomes "Incident 2025 1", not "Incident 1"). A title-only heuristic can't reliably distinguish a user's trailing digits from a duplicate counter, so no collapsing is attempted.

Middle clicks fire auxclick rather than click, so both events are wired up, and the middle-click autoscroll is suppressed. On Linux/X11 the middle button additionally pastes the PRIMARY selection into the freshly-focused query editor; this is suppressed for exactly that one paste, so no clipboard content is appended after the duplicated query. A normal Ctrl+V (or Shift+Insert) is unaffected.

If a tab's title is being edited inline when a duplicate gesture fires, the pending edit is committed first so the copy is built from the final title rather than a stale value; tab-bar rerenders (including those triggered by query completion) also flush any open title edit.

Note: the PRIMARY-paste guard mirrors the mechanism in #109246. As that PR is not yet merged, this branch includes the same shared guard infrastructure to stay self-contained against master; whichever of the two merges second will need a small conflict resolution in that block.

Related: #109246

Changelog category (leave one):

  • Improvement

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

In the Web UI, middle-clicking a tab's title (or Ctrl/Shift/Cmd+clicking it) now duplicates the tab.

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

🤖 Generated with Claude Code

alexey-milovidov and others added 2 commits July 3, 2026 21:39
…e title

Middle-clicking the tab title, or Ctrl/Shift/Cmd+clicking it (the usual
browser gesture for "open in a new tab"), now duplicates the tab. The copy
carries over the query and parameters, is inserted right after the source
tab, and becomes active.

The duplicate's title is the source title with a " N" suffix, N starting at
1 and incrementing until unique. An existing trailing numeric suffix on the
source is stripped first, so duplicating "Query A 2" yields "Query A 3"
rather than "Query A 2 1".

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Duplicating a tab with a middle click focuses the query editor. On Linux/X11
the middle button additionally pastes the PRIMARY selection into the freshly
focused editor, appending clipboard junk after the duplicated query.

Guard against exactly that one paste, mirroring the mechanism in
#109246: `activateTab` arms
`suppress_middleclick_paste` around `query_area.focus()` for a genuine middle
click, the editor's `paste` handler swallows that paste, and a real keyboard
paste shortcut (Ctrl/Cmd+V or Shift+Insert) disarms the guard first so it is
never eaten. A short fallback timer bounds how long a never-consumed guard
can linger.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@clickhouse-gh

clickhouse-gh Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Workflow [PR], commit [527ce10]

Summary:


AI Review

Summary

This PR adds tab duplication in play.html for middle-click and platform new-tab modifier clicks, including the X11 PRIMARY-paste guard and the title-edit flush paths needed to keep duplicated tabs and pending renames consistent. I did not find a remaining correctness or contract issue in the current head after checking the code against the current PR description and all prior review threads.

Final Verdict
  • Status: ✅ Approve

@clickhouse-gh clickhouse-gh Bot added the pr-improvement Pull request with some product improvements label Jul 3, 2026
Comment thread programs/server/play.html Outdated
`duplicateTitle` collapsed any trailing ` N` on the source title before
numbering the copy, so it also rewrote ordinary user-authored names that
merely end in digits: duplicating `Incident 2025` produced `Incident 1`,
and `Release 24 7` produced `Release 24 1`.

Now the trailing ` N` is collapsed only when it is a real duplicate-family
suffix — the title with it removed is itself an existing tab (the family
root). So with a `Query A` tab present, duplicating `Query A 2` still yields
`Query A 3`, but `Incident 2025` (no `Incident` tab) is kept whole and
becomes `Incident 2025 1`, never corrupting a meaningful numeric ending.

Addresses the AI review on
#109357.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@alexey-milovidov
alexey-milovidov requested a review from pamarcos July 6, 2026 04:57
@pamarcos

pamarcos commented Jul 6, 2026

Copy link
Copy Markdown
Member

Feedback from GPT-5.5:
duplicateTitle still conflates a user-authored numeric suffix with a duplicate counter. With tabs named Incident and Incident 2025, duplicating Incident 2025 can create Incident 1. Please preserve the original title family, for example Incident 2025 1, or otherwise make duplicate lineage unambiguous.

@pamarcos pamarcos self-assigned this Jul 6, 2026
Address review feedback from @pamarcos (GPT-5.5) and the AI review on
#109357.

`duplicateTitle` used to strip a trailing ` N` from the source title when the
stripped base was itself an open tab, to keep a duplicate family flat
(`Query A 2` -> `Query A 3`). But a title-only heuristic cannot distinguish a
duplicate-family suffix from user-authored trailing digits: with both `Incident`
and `Incident 2025` open, duplicating `Incident 2025` stripped ` 2025`, saw
`Incident` was taken, and produced `Incident 1` - silently dropping the
authored `2025` and joining the unrelated `Incident` family.

Stop stripping. The whole source title is now always the family base, so the
copy of `Incident 2025` is `Incident 2025 1` and `Release 24 7` is
`Release 24 7 1`. This keeps the duplicate lineage unambiguous and never
rewrites a meaningful numeric ending.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@alexey-milovidov

Copy link
Copy Markdown
Member Author

Thanks — fixed in 793109b. duplicateTitle no longer strips any trailing N; the full source title is always the family base, so duplicating Incident 2025 now yields Incident 2025 1 (as suggested) rather than Incident 1, and the duplicate lineage is unambiguous. A title-only heuristic can't distinguish a user's trailing digits from a duplicate counter, so I removed the collapse step entirely.

Comment thread programs/server/play.html Outdated
The duplicate gestures (middle-click, Ctrl/Shift/Cmd+click) could fire
while the tab title span was still in inline `contentEditable` edit mode.
Because `duplicateTab` copies `src.title`, which is only committed on the
editor's `blur`, the copy was built from the stale, not-yet-committed
title: renaming `Query A` to `Foo` and duplicating before blurring created
`Query A 1` instead of `Foo 1`.

Guard all three handlers on `titleEl.isContentEditable`:
- `click`: while editing, return early so the click places the cursor or
  extends the selection instead of duplicating.
- `auxclick`/`mousedown`: while editing, do not intercept the middle click,
  so it pastes the X11 PRIMARY selection into the title as usual.

This also stops a plain click from re-triggering `startTitleEdit` (which
re-selects the whole title) while already editing.

AI Review: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=109357&sha=793109b5&name_0=PR

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread programs/server/play.html
… tab

Address review: the inline title editor writes back to `tab.title` only on
`blur`, but the duplicate path (`duplicateTab`) rerenders the tab bar and
destroys the editable span. For a middle click, the `mousedown` handler also
calls `preventDefault`, which suppresses the automatic `blur`. So renaming one
tab and then middle-clicking a different tab's title to duplicate it silently
reverted the first tab's rename to its old title.

Fix: expose the active editor's commit function via `activeTitleCommit` and
flush any pending rename at the start of `duplicateTab`, before the tab bar is
rerendered. The editable tab is always the active tab, so this works regardless
of which tab is being duplicated. The `blur` listener's `done` guard prevents a
double commit.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread programs/server/play.html Outdated
…on duplicate

Address review: the previous fix only flushed a pending inline title rename
(`activeTitleCommit`) inside `duplicateTab`, but any tab-bar rerender destroys
the focused `contentEditable` title span before its `blur` commits. The
concrete case flagged by review: start a slow query in the active tab, rename
it (leave the editor open), and when the response finishes `writeHistoryEntry`
calls `renderTabBar`, rebuilding the bar and silently reverting the rename.

Centralize the flush at the start of `renderTabBar` - the single rerender
chokepoint every path goes through - so a commit-or-lose can no longer be
missed. It is safe against re-entry because `commit` clears `activeTitleCommit`
before it calls back into `renderTabBar`. The explicit flush in `duplicateTab`
stays, since it must run before `duplicateTitle(src.title)` reads the title.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread programs/server/play.html
@pamarcos

pamarcos commented Jul 7, 2026

Copy link
Copy Markdown
Member

Feedback from GPT-5.5:
The source behavior looks OK now, but the PR description still documents the old duplicate-title rule (Query A 2 -> Query A 3). The current code intentionally preserves the full source title (Query A 2 -> Query A 2 1) to avoid corrupting user-authored numeric suffixes. Please update the PR description/manual test text to match the shipped behavior.

@alexey-milovidov

Copy link
Copy Markdown
Member Author

Thanks — updated the PR description accordingly. It now documents the shipped behavior: the full source title is always the base, so Query A 2 becomes Query A 2 1 (no suffix stripping), which is what avoids corrupting a user-authored numeric suffix.

@pamarcos pamarcos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

Image

Comment thread programs/server/play.html Outdated
alexey-milovidov and others added 2 commits July 13, 2026 04:09
Address review (clickhouse-gh AI review, PR #109357): the disarm path for
`suppress_middleclick_paste` keyed off `e.key === 'v' || e.key === 'V'`, which
is layout-dependent. On a non-Latin keyboard layout (e.g. Russian), a genuine
`Ctrl+V` / `Cmd+V` paste reports a non-`v` `e.key` (the Cyrillic character), so
the guard was never disarmed and the subsequent real `paste` event was
swallowed - breaking the PR's own guarantee that normal keyboard paste is
unaffected after duplicating a tab.

Detect the physical `V` key via `e.code === 'KeyV'` instead of the printed
character. This is layout-independent and matches how the OS binds the paste
shortcut on non-Latin layouts. The `Shift+Insert` branch already used the
layout-independent named key `'Insert'` and is unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread programs/server/play.html Outdated
…plicating a tab

On macOS, Ctrl+Click is the secondary (context-menu) click, not a new-tab
gesture. The tab-title click handler treated any Ctrl-modified click as a
duplicate gesture, so a macOS Ctrl+Click on a tab title duplicated the tab
instead of leaving the gesture to the browser. Gate the duplicate branch the
same way as `set_query_or_run`: `e.shiftKey || (isMac ? e.metaKey : e.ctrlKey)`,
and ignore plain macOS Ctrl+Click entirely.
@alexey-milovidov
alexey-milovidov added this pull request to the merge queue Jul 17, 2026
Merged via the queue into master with commit 1d3aa4e Jul 17, 2026
179 checks passed
@alexey-milovidov
alexey-milovidov deleted the web-ui-duplicate-tab branch July 17, 2026 15:09
@robot-ch-test-poll2 robot-ch-test-poll2 added the pr-synced-to-cloud The PR is synced to the cloud repo label Jul 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-improvement Pull request with some product improvements pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants