Skip to content

Web UI: add user name to the Documentation link URL#109419

Merged
alexey-milovidov merged 5 commits into
masterfrom
web-ui-docs-link-user
Jul 19, 2026
Merged

Web UI: add user name to the Documentation link URL#109419
alexey-milovidov merged 5 commits into
masterfrom
web-ui-docs-link-user

Conversation

@alexey-milovidov

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

Copy link
Copy Markdown
Member

The Documentation link in the Web UI (play.html) now carries the Play username over to the /docs page as a user query parameter, so the docs page connects as the same user instead of falling back to the server default user.

The password is intentionally omitted to avoid leaking it in the URL. The link stays in sync as the username changes, matching the existing behavior of the Web Terminal link.

Changelog category (leave one):

  • Improvement

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

Web UI: the Documentation link now carries the current user name in the URL.

🤖 Generated with Claude Code

Version info

  • Merged into: 26.7.1.1195 (included in 26.7 and later)

The Documentation link in the Web UI (`play.html`) now carries the
Play username over to the `/docs` page as a `user` query parameter, so
the docs page connects as the same user instead of falling back to the
server default user. The password is intentionally omitted to avoid
leaking it in the URL. The link stays in sync as the username changes,
matching the existing behavior of the Web Terminal link.

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

clickhouse-gh Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Workflow [PR], commit [c7e3914]

Summary:


AI Review

Summary

This PR updates play.html so the Documentation link points at the configured server and forwards the current Play user in ?user=. The earlier stale-href issues appear fixed, but the new propagation still misses one supported way Play carries the current user, so the docs page can still reconnect as the server default user instead of the active Play user. Final verdict: changes requested.

Findings

⚠️ Majors

  • [programs/server/play.html:3871] play.html already treats the connection identity as "explicit user field first, otherwise the username embedded in url_elem.value" (effectiveConnectionUser), but this link only forwards the visible field. With a connection like http://alice@host:8123/ and an empty user field, /docs still opens without ?user=alice; docs.html then initializes its own server URL from location.origin and only restores $user from the query string, so later documentation queries run as the server default user. Suggested fix: forward effectiveConnectionUser(url_elem.value, user_elem.value) (or equivalent field-first fallback to URL userinfo) when building the docs link.
Final Verdict

⚠️ Needs changes before merge: the docs link still drops the effective user when Play authenticates through URL userinfo, so the PR does not yet meet its "open docs as the same user" contract.

@clickhouse-gh clickhouse-gh Bot added the pr-improvement Pull request with some product improvements label Jul 4, 2026
Comment thread programs/server/play.html
Address review feedback: `#user` is a browser-autofillable credential
field, so the browser can fill a saved login without firing an `input`
event. In that case `updateDocsURL` never reran, `docs_icon.href` stayed
stale, and opening Documentation fell back to the server default user
even though the visible field showed the saved one.

Refresh the `href` on `pointerdown` as well, right before navigation,
mirroring how the Web Terminal link (`updateTerminalHref`) already avoids
the same stale-`href` problem.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread programs/server/play.html
The previous fix refreshed `docs_icon.href` only on `pointerdown`, which
does not fire when the Documentation link is activated from the keyboard
(tab to the link, then Enter). In that case, if the browser had autofilled
`#user` after the initial `updateDocsURL` without firing `input`, the href
stayed stale and `/docs` opened as the server's default user.

Add a `click` listener in addition to `pointerdown`. `click` fires for both
mouse and keyboard activation and runs before the browser's default
navigation, so the href is refreshed on every navigation path
(`pointerdown` still covers middle-button `auxclick`, which produces no
`click`).

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:56
@pamarcos pamarcos self-assigned this Jul 6, 2026
Comment thread programs/server/play.html
try {
docs_icon.href = new URL('/docs', url_elem.value).href;
const url = new URL('/docs', url_elem.value);
if (user_elem.value) url.searchParams.set('user', user_elem.value);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

play.html already treats the connection user as "explicit user field first, otherwise the username embedded in url_elem.value" (see effectiveConnectionUser). This update only forwards user_elem.value, so a connection like http://alice@host:8123/ with an empty user field still opens /docs without ?user=alice. docs.html then seeds $url from location.origin and only restores $user from the query string, so its later queries run as the server default user instead of alice.

Please forward the effective connection user here (field first, otherwise URL userinfo) rather than only the visible field value.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Isn't a big deal.

@alexey-milovidov
alexey-milovidov merged commit c1ec4e8 into master Jul 19, 2026
179 of 180 checks passed
@alexey-milovidov
alexey-milovidov deleted the web-ui-docs-link-user branch July 19, 2026 17:03
@robot-ch-test-poll robot-ch-test-poll added the pr-synced-to-cloud The PR is synced to the cloud repo label Jul 19, 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