577847 - "See also" field should accept any URL

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: General, enhancement, P2)

Description

[Thomas D. (:thomas8)]

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

Comment 1

[Max Kanat-Alexander]

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.

Comment 2

[Thomas D. (:thomas8)]

^^

Comment 3

[Wayne Mery (:wsmwk)]

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

Comment 4

[Max Kanat-Alexander]

Validation is about data consistency and implementing all of the features listed in bug 231429.

Comment 5

[Biju]

I feel the validation of See Also is very stupid, see following URL I have added to see also
http://snipurl.com/11wpwj/show_bug.cgi?id=1234

Comment 6

[Frédéric Buclin]

(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.

Comment 8

[Andre Klapper]

(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: code.google.com
* bug 617802: appspot
* bug 661533: reviewboard

Comment 9

[Marcin Cieślak (:saper)]

Here's a patch to allow linking to "bugs.php.net":

http://tools.wikimedia.pl/cgi-bin/fossil/ci/cf05f0d7bc?sbs=0

Comment 10

[Thomas D. (:thomas8)]

(In reply to Marcin CIESLAK from comment #9)
> Here's a patch to allow linking to "bugs.php.net":
> http://tools.wikimedia.pl/cgi-bin/fossil/ci/cf05f0d7bc?sbs=0

Marcin, thanks for providing that patch. Could you attach it here and ensure the correct patch headers (see similar patches here)?

Comment 11

[Thomas D. (:thomas8)]

(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.

Comments?

Comment 12

[Marcin Cieślak (:saper)]

Attachment: Patch to allow "bugs.php.net" in the "See also" field

Here it is. Now :)

Comment 13

[Thomas D. (:thomas8)]

(In reply to Marcin CIESLAK from comment #12)
> Created attachment 605331 [details] [diff] [review]
> Patch to allow "bugs.php.net" 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.

Comment 14

[Marcin Cieślak (:saper)]

Attachment: Patch for Bugzilla r8155

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:

http://bugs.php.net/bug.cgi?id=%(id)d

or something like that. We would cover most cases with this I am sure...

Comment 15

[Max Kanat-Alexander]

(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.

Comment 16

[Frédéric Buclin]

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. ;)

Comment 17

[Dylan Hardison [:dylan] (he/him)]

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.

Comment 19

[Kohei Yoshino [:kohei]]

From my bug field analysis:

• Make the URL field accept any multiple URLs.
• Migrate the current URLs from the See Also field to the URL field, while leaving bug IDs as is. This also makes the UI simpler.
• Implement auto bug reference like GitHub issues and deprecate the See Also field.

Comment 20

[Dylan Hardison [:dylan] (he/him)]

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.

Comment 21

[Kohei Yoshino [:kohei]]

The tentative plan is fine. But I don’t want to see the mixture of links and Bug IDs in the same field, hence the proposal.

• See Also: internal bug IDs only
• URL field: any external links

Will file a separate bug for that, which won’t block the Bugzilla 6 release.

The following fields would all be bug IDs fields:

• Depends on
• Blocks
• Regressions (to be added soon)
• Regressed by (to be added soon)
• See Also
• Duplicates (currently read-only)

Comment 22

[Dylan Hardison [:dylan] (he/him)]

(In reply to Kohei Yoshino [:kohei] (Bugzilla UX) (FxSiteCompat) from comment #21)

The tentative plan is fine. But I don’t want to see the mixture of links and Bug IDs, hence the proposal.

• See Also: internal bug IDs only
• URL field: any external links

Will file a separate bug for that, which won’t block the Bugzilla 6 release.

The following fields would all be bug ID fields:

• Depends on
• Blocks
• Regressions (to be added soon)
• Regressed by (to be added soon)
• See Also
• Duplicates (currently read-only)

So that would be a bug_relations which would obsolete dependencies, duplicates, and regressions. Correct?

Comment 24

[Kohei Yoshino [:kohei]]

Well yeah, we can merge them like that.

Comment 37

[:glob ✱]

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.

Comment 38

[David Lawrence [:dkl]]

Merged
https://github.com/mozilla-bteam/bmo/commit/37a4da611b603e1e8d498d121f81bf408c8f8d8a

Comment 39

[`{third: "Beedell", first: "Roke"}`{.JSON5}]

Does this continue to block "http://bugzilla.mozilla.org/show_bug.cgi?id=1775300" despite "http://bugzilla.mozilla.org/show_bug.cgi?id=577847#c38"?

Comment 40

[:glob ✱]

(In reply to BEEDELLROKEJULIANLOCKHART from comment #39)

Does this continue to block "http://bugzilla.mozilla.org/show_bug.cgi?id=1775300" despite "http://bugzilla.mozilla.org/show_bug.cgi?id=577847#c38"?

Yes, having test coverage here is still desirable.