Closed Bug 577847 Opened 12 years ago Closed 2 months ago

"See also" field should accept any URL


( :: General, enhancement, P2)






(Reporter: thomas8, Assigned: dkl)




(2 obsolete files)

We already have requests for 5 different bug tracking systems to be accepted as valid URLs for "See also" field, 2 of which have been implemented so far.

bug 532350 (fixed) Can't add Debian bug URLs to a bug using "See Also"
bug 533121 (fixed) Support Google Code for see_also urls
bug 543667 Support trac URLs in the "See Also" field
bug 558784 support JIRA links in see also
bug 571740 Cannot add getsatisfaction links/URLs to "See also" field

This isn't scalable, and I doubt very much that the benefits of validation outweigh the disadvantages of non-scalability and input restrictions imposed on the user.

Looking at the current BMO installation (as a typical/standard bugzilla installation), we definitely have a usability problem here:
1) expected content for URL field isn't clearly defined
2) traditionally, it seems to be URLs that have a very direct and explicit relation to the bug, like test cases etc, as opposed to more remotely related content.
3) Even if we wanted to accept less directly related URLs in URL field, we currently have bug 577842 (cannot add multiple URLs to URL field, i.e. multiple URLs are not parsed correctly).
4) Consequently, both semantically and technically, the only suitable field for "related URLs" seems to be "See also" field (albeit somewhat neglecting its definition where we say it's only for bugs). However, we don't accept any URL there, just the chosen few of important bug systems (this bug).

IMO the easiest solution would be if "See also" accepts any URL (this bug).
- Using URL fields doesn't feel right (even if it could take multiple entries)
- Implementing separate "Related URLs" field is adding unnecessary clutter
- Field header "See also" is semantically much more open than the current narrow definition that we are trying to enforce by validation
This might be possible, but we'd have to see how we do it after we add better automation.

However, I'm generally against it from a validation standpoint.
Priority: -- → P5
I don't know what validation is about. Is requiring the target to be in another bugzilla database in order to exploit using the target db? as in extended queries, or leveraging of other bugzilla capabilities
Validation is about data consistency and implementing all of the features listed in bug 231429.
I feel the validation of See Also is very stupid, see following URL I have added to see also
(In reply to Biju from comment #5)
> I feel the validation of See Also is very stupid

If you don't like how the See Also field works, don't use it.
Duplicate of this bug: 703919
(In reply to Thomas D. from comment #0)
> We already have requests for 5 different bug tracking systems to be accepted
> as valid URLs for "See also" field, 2 of which have been implemented so far.

Listing the currently existing further requests:
* bug 704999: github
* bug 562292:
* bug 617802: appspot
* bug 661533: reviewboard
Here's a patch to allow linking to "":
(In reply to Marcin CIESLAK from comment #9)
> Here's a patch to allow linking to "":

Marcin, thanks for providing that patch. Could you attach it here and ensure the correct patch headers (see similar patches here)?
(In reply to Max Kanat-Alexander from comment #4)
> Validation is about data consistency and implementing all of the features
> listed in bug 231429.

Max, I fully appreciate that the ultimate goal of the see also field (bug 231429) is to interact with target bug databases for easy input (shorthand) and intelligent output (e.g. truncate URLs, and provide information about the status of external bugs within bugzilla). Obviously, this will require validation for such URLs for which we want to offer such services.

Notwithstanding, two questions remain:

1) Which field type in a typical Bugzilla installation (like BMO) allows entering a list of *multiple freeform URLs* to link to *any type of* relevant information for a bug (which is not necessarily found in bug databases only)?
If there is none, that looks like quite a big shortcoming of Bugzilla.

2) Especially given our shortcomings in 1), why are we enforcing validation of *all* URLs in See-also? Maybe the following could work:

Proposal for adding flavors "strict" vs. "free" to See-Also Field

Implement a pref, property or whatever to toggle between "strict" and "free" mode of See-Also field:

a) in "strict" mode, only URLs from known databases will be accepted (as we do now); so there's strict validation and we do input/output magic for *all* allowed URLs (while excluding free URLs)

b) in "free" mode, both URLs from known databases and freeform URLs (any URLs) will be accepted. We can still provide input/output magic for *known* URLs, while *free* URLs will just be accepted alongside.

Codewise, this isn't a problem: For b), we just iterate the comma-separated list of input URLs (or their respective shorthand), and do all the magic for *known* URL types, while leaving other URLs untouched and just accept them as they are.

(In reply to Marcin CIESLAK from comment #12)
> Created attachment 605331 [details] [diff] [review]
> Patch to allow "" in the "See also" field
> Here it is. Now :)

Marcin, I have moved your patch into new Bug 735196 so that it can be reviewed there so that we don't lose the focus of this bug.

Your patch just proves the point of this bug that we can keep adding patches for each and every bug tracker out there and we'll probably not finish them all. As opposed to that non-scalable approach (which certainly has its own advantages, see my comment 11), I'm advocating here and elsewhere that we need a scalable approach, an approach that supports *multiple freeform* (any) URLs in a single bugzilla field. I've made a compromise proposal in comment 11 how the best of both worlds (strict validation vs. support any URL, including input/output magic always for known URLs) could be combined into the existing See Also field, e.g. using a "strict" vs. "free" switch.
Attached patch Patch for Bugzilla r8155 (obsolete) — Splinter Review
Patch to add PHP support into core. Not sure what kind of distinction we have between "core" bug trackers and those in extension. I think there should be one place only. But I'm not involved in Bugzilla development at all so feel free to do whatever you need.

You have a backport to 4.0.x already for free :)

Re: substance

Although I got hit by this - the bugtracker I and other people
needed for Wikimedia installation wasn't there - I believe
this is a RightThing to provide code that understands other
bugtracking systems.

I think in the future it should be possible to extract core bug metadata out of foreign bugtrackers and this *WILL* need interpretation of different output format (unless we have a standard most folk will implement).

For a starter, it would be great to display something like

 "PHP bug #####" 

instead of full URL.

Next stages - some intelligent fetching of foreign bug information and
updating some local information.

For example, displaying:

  <strike>PHP bug #####</strike>

when the bug got closed up/down/sidestream.

This is for me a rationale to maintain some code for every bugtracker type.

As it stands, however, it might have been much nicer to be able to just
have a configuration screen that keeps lists like:

or something like that. We would cover most cases with this I am sure...
(In reply to Thomas D. from comment #11)
So, I've been thinking about this, and perhaps one way forward would be to allow any URL, but for those we recognize, validate and normalize them. That way we can add cross-bugtracker functionality with those in the future.

The problem with that approach is when somebody enters a URL in a format they *expect* us to recognize, but we do not. (For example, if somebody has hacked their Bugzilla to support /bug/12345.) Then, when we implement cross-bugtracker support, Bugzilla behaves in a way that users don't expect.
I have the exact same thought as mkanat in the previous comment. For now, requiring validation is a good thing to force contributors/users to ask for specific support. If we allowed free URLs from the start, nobody would ask/contribute support for additional bug trackers, and people would then be disappointed why such or such bugs in the See Also field don't display metadata for some given bug trackers.

Also, now that we have the MoreBugUrl extension, each admin is free to add a validator which accepts all URLs. So this can now be done on a per-installation basis, without waiting for the core code to implement this (which we probably won't do in the near future).

Marcin, to get a chance to see your code reviewed and committed into the source code, please request review, else your patch won't appear in reviewers' radar. Thomas knows the review process, he could have tell you this, or set the review flag himself. ;)
A fall through that accepts any url should be done before the next release. We can mark such "last resort" entries with special CSS or something, and even suggest the MoreBugUrl extension. But this is a much better default behavior.
Priority: P5 → P1
Target Milestone: --- → Bugzilla 6.0
Duplicate of this bug: 1526090
Assignee: create-and-change → nobody
Component: Creating/Changing Bugs → General
Product: Bugzilla →
QA Contact: default-qa
Target Milestone: Bugzilla 6.0 → ---
Version: unspecified → Production
Attachment #605380 - Attachment is obsolete: true
Attachment #605331 - Attachment is obsolete: true

For the moment I'm just adding a catch-all to bug_see_also.
Conveniently that class is just called Bugzilla::BugUrl. We can remove the URL field and migrate those as entries to bug_see_also as part of this though, so it should match comment 19.

Blocks: bugzilla-6
Duplicate of this bug: 1526992
Duplicate of this bug: 1486398
Duplicate of this bug: 1459365
Duplicate of this bug: 1429208
Duplicate of this bug: 1371326
Duplicate of this bug: 1338450
Duplicate of this bug: 1244457
Duplicate of this bug: 1344367
Duplicate of this bug: 1238057
Duplicate of this bug: 1229990
Duplicate of this bug: 1202983
Priority: P1 → P2
See Also: → 1752107
Duplicate of this bug: 1353283

Holding back on this simple change due to major rearchitecting isn't ideal.

We're going to allow any http:// or https:// URL in See Also fields.

Redesign work, if desired, can happen in other bugs.

Assignee: nobody → dkl
No longer blocks: bz-seealso, bugzilla-6
See Also: 1752107
Blocks: 1775300
Closed: 2 months ago
Resolution: --- → FIXED

(In reply to BEEDELLROKEJULIANLOCKHART from comment #39)

Does this continue to block "" despite ""?

Yes, having test coverage here is still desirable.

Flags: needinfo?(dkl)
You need to log in before you can comment on or make changes to this bug.