fix: comment url swaps title and answer id - #1603
Merged
LinkinStars merged 1 commit intoSep 8, 2026
Merged
Conversation
CommentURL called AnswerURL(permalink, siteUrl, questionID, answerID, title), but AnswerURL takes (permalink, siteUrl, questionID, title, answerID), so the last two arguments were reversed for every comment left on an answer. The answer route is /questions/:id/:title/:answerid, so the answer id landed in the title slot and the title landed in the answer id slot. Under the numeric permalink settings the title was run through uid.DeShortID and came out as an unrelated number. Under the short id settings uid.EnShortID returned it unchanged, so the raw title, spaces and all, was written into the path. The broken link reaches the new comment notification email built in internal/service/export/email_service.go and the CommentUrl handed to notification plugins in internal/service/notification_common/notification.go. A comment on a question takes the other branch and was already correct. Pass title and answerID in the order AnswerURL declares them, and add pkg/display/url_test.go covering both branches across all four permalink settings.
LinkinStars
self-requested a review
September 8, 2026 03:42
LinkinStars
approved these changes
Sep 8, 2026
LinkinStars
left a comment
Member
There was a problem hiding this comment.
Thank you very much for your contribution. This is indeed a bug.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed Changes
A comment left on an answer links to the wrong URL.
CommentURLcalls its siblingAnswerURLwith the last two arguments reversed:The answer route is
/questions/:id/:title/:answerid(internal/router/template_router.go:71, andquestions/:qid/:slugPermalink/:aidinui/src/router/routes.ts:116), so the answer id ends up in the title slot and the title ends up in the answer id slot. What the title then becomes depends on the permalink setting: the numeric ones send it throughuid.DeShortID, which parses it as a short id and returns an unrelated number, and the short id ones send it throughuid.EnShortID, which returns non-numeric input unchanged, so the raw title with its spaces is written straight into the path.For question
10010000000000001, titleHow to install Answer, answer10020000000000002, comment10030000000000003:This is what readers click in the new comment notification email built in
internal/service/export/email_service.go:314, and it is theCommentUrlhanded to notification plugins ininternal/service/notification_common/notification.go:399. Both call sites already pass the arguments in the orderCommentURLdeclares, so the swap is entirely insideCommentURL. A comment on a question takes the other branch, which was already correct.titleandanswerIDtoAnswerURLin the order it declares them, so a comment on an answer is the answer URL plus the comment querypkg/display/url_test.gocovering both branches across all four permalink settingsTesting
Against
devat 947a48b, the new test fails on all four permalink settings for a comment on an answer, and the four question cases pass, which is the branch that was already correct. With the one line changed:No existing test referenced
CommentURL, so nothing else needed updating. The test uses an ASCII title, whichhtmltext.UrlTitleslugifies identically regardless of thetransliterateNonLatinsetting, locale or timezone.