Bug 22155 - REGRESSION (r38261): "Created an attachment" links for non-patches link to "Review Patch" page, but should not
Summary: REGRESSION (r38261): "Created an attachment" links for non-patches link to "R...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Website (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL: https://bugs.webkit.org/show_bug.cgi?...
Keywords:
Depends on:
Blocks:
 
Reported: 2008-11-10 07:52 PST by Adam Roben (:aroben)
Modified: 2008-11-10 08:22 PST (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Roben (:aroben) 2008-11-10 07:52:00 PST
To reproduce:

1. Go to https://bugs.webkit.org/show_bug.cgi?id=3235#c2

Note that the link in that comment now reads:

> Created an attachment (id=5331) [review]

The text "[review]" links to the "Review Patch" page for this attachment. But the attachment is a test case, not a patch, so we should not be linking to "Review Patch" for it at all. We could probably just omit the "[review]" text entirely in this case.
Comment 1 Adam Roben (:aroben) 2008-11-10 07:52:33 PST
Well, Bugzilla's "smart" formatting screwed up the quoted part of my previous comment, but you get the point.
Comment 2 Darin Adler 2008-11-10 08:09:44 PST
This won't be easy to fix. The level of code dealing with the attachment has only the attachment number and has no way to figure out if the attachment is a patch or not. It's formatting a description and not doing database access.
Comment 3 Adam Roben (:aroben) 2008-11-10 08:21:03 PST
(In reply to comment #2)
> This won't be easy to fix. The level of code dealing with the attachment has
> only the attachment number and has no way to figure out if the attachment is a
> patch or not. It's formatting a description and not doing database access.

It looks to me like GetAttachmentLink does in fact do database access:

https://trac.webkit.org/browser/trunk/BugsSite/globals.pl?rev=38261#L866
Comment 4 Darin Adler 2008-11-10 08:22:38 PST
OK, then maybe it will be easy!