Merge ~andrey-fedoseev/launchpad:bug-attachment-url into launchpad:master

[Andrey Fedoseev]

Add `url` to `BugAttachment`

`libraryfile` field of `BugAttachment` is now optional, an attachment can either contain a file, or have a URL

Add `displayed_url` property to `BugAttachment` that contains either a proxied attachment file URL, or the URL associated with the attachment.

Improve the `BugAttachment` links:

- Use the formatter API to create the links
- Use different icons depending on the link type: external URL, a patch, a downloadable file
- Add rel=nofollow to external URLs

[Andrey Fedoseev (andrey-fedoseev)]

Some screenshots:

https://people.canonical.com/~afedoseev/LP-912/Screenshot_20221004_195626.png
https://people.canonical.com/~afedoseev/LP-912/Screenshot_20221004_195659.png
https://people.canonical.com/~afedoseev/LP-912/Screenshot_20221004_195724.png

[Andrey Fedoseev (andrey-fedoseev)]

More screenshots:

Before:
https://people.canonical.com/~afedoseev/LP-912/before.png

After:
https://people.canonical.com/~afedoseev/LP-912/after.png

[Colin Watson (cjwatson)]

I like the use of the formatter API and the new `displayed_url` property.

review: Approve

diff --git a/lib/canonical/launchpad/icing/css/components/sidebar_portlets.css b/lib/canonical/launchpad/icing/css/components/sidebar_portlets.css
index e4f4ff3..37487d2 100644
--- a/lib/canonical/launchpad/icing/css/components/sidebar_portlets.css
+++ b/lib/canonical/launchpad/icing/css/components/sidebar_portlets.css
@@ -132,3 +132,9 @@
     padding-right: 0.25em;
     }
 
+/******************************************************************************
+ Attachments portlet
+*/
+.side .portlet .download-attachment {
+    word-break: break-word;
+    }
diff --git a/lib/lp/app/browser/configure.zcml b/lib/lp/app/browser/configure.zcml
index 88985a9..cc40021 100644
--- a/lib/lp/app/browser/configure.zcml
+++ b/lib/lp/app/browser/configure.zcml
@@ -932,6 +932,12 @@
       name="fmt"
       />
   <adapter
+      for="lp.bugs.interfaces.bugattachment.IBugAttachment"
+      provides="zope.traversing.interfaces.IPathAdapter"
+      factory="lp.app.browser.tales.BugAttachmentFormatterAPI"
+      name="fmt"
+      />
+  <adapter
       for="lp.services.webapp.interfaces.ILink"
       provides="zope.traversing.interfaces.IPathAdapter"
       factory="lp.app.browser.tales.LinkFormatterAPI"
diff --git a/lib/lp/app/browser/tales.py b/lib/lp/app/browser/tales.py
index a413347..89a29bc 100644
--- a/lib/lp/app/browser/tales.py
+++ b/lib/lp/app/browser/tales.py
@@ -44,6 +44,7 @@ from lp.app.interfaces.launchpad import (
 from lp.blueprints.interfaces.specification import ISpecification
 from lp.blueprints.interfaces.sprint import ISprint
 from lp.bugs.interfaces.bug import IBug
+from lp.bugs.interfaces.bugattachment import IBugAttachment
 from lp.buildmaster.enums import BuildStatus
 from lp.code.enums import RevisionStatusResult
 from lp.code.interfaces.branch import IBranch
@@ -726,7 +727,6 @@ class ObjectImageDisplayAPI:
     def __init__(self, context):
         self._context = context
 
-    # def default_icon_resource(self, context):
     def sprite_css(self):
         """Return the CSS class for the sprite"""
         # XXX: mars 2008-08-22 bug=260468
@@ -754,6 +754,13 @@ class ObjectImageDisplayAPI:
             sprite_string = "meeting"
         elif IBug.providedBy(context):
             sprite_string = "bug"
+        elif IBugAttachment.providedBy(context):
+            if context.url:
+                sprite_string = "external-link"
+            elif context.is_patch:
+                sprite_string = "haspatch-icon"
+            else:
+                sprite_string = "download-icon"
         elif IPPA.providedBy(context):
             if context.enabled:
                 sprite_string = "ppa-icon"
@@ -1866,6 +1873,36 @@ class BugTaskFormatterAPI(CustomizableFormatter):
         return BugFormatterAPI(self._context.bug)._make_link_summary()
 
 
+class BugAttachmentFormatterAPI(CustomizableFormatter):
+
+    final_traversable_names = dict(
+        **CustomizableFormatter.final_traversable_names,
+        attachment_link="attachment_link",
+    )
+
+    def attachment_link(self):
+        sprite = self.sprite_css()
+        if sprite is None:
+            css = ""
+        else:
+            css = ' class="' + sprite + '"'
+
+        if self._context.url:
+            rel = ' rel="nofollow"'
+        else:
+            rel = ""
+
+        title = structured(self._context.title).escapedtext
+        url = structured(self._context.displayed_url).escapedtext
+
+        return """<a href="{url}"{css}{rel}>{title}</a>""".format(
+            title=title,
+            url=url,
+            css=css,
+            rel=rel,
+        )
+
+
 class CodeImportFormatterAPI(CustomizableFormatter):
     """Adapter providing fmt support for CodeImport objects"""
 
diff --git a/lib/lp/bugs/adapters/bugchange.py b/lib/lp/bugs/adapters/bugchange.py
index c7babdc..e97d6e8 100644
--- a/lib/lp/bugs/adapters/bugchange.py
+++ b/lib/lp/bugs/adapters/bugchange.py
@@ -63,7 +63,6 @@ from lp.bugs.interfaces.bugtask import (
     IBugTask,
 )
 from lp.registry.interfaces.product import IProduct
-from lp.services.librarian.browser import ProxiedLibraryFileAlias
 from lp.services.webapp.publisher import canonical_url
 
 # These are used lp.bugs.model.bugactivity.BugActivity.attribute to normalize
@@ -698,11 +697,6 @@ class BugTagsChange(AttributeChange):
         return {"text": "\n".join(messages)}
 
 
-def download_url_of_bugattachment(attachment):
-    """Return the URL of the ProxiedLibraryFileAlias for the attachment."""
-    return ProxiedLibraryFileAlias(attachment.libraryfile, attachment).http_url
-
-
 class BugAttachmentChange(AttributeChange):
     """Used to represent a change to an `IBug`'s attachments."""
 
@@ -712,13 +706,13 @@ class BugAttachmentChange(AttributeChange):
             old_value = None
             new_value = "%s %s" % (
                 self.new_value.title,
-                download_url_of_bugattachment(self.new_value),
+                self.new_value.displayed_url,
             )
         else:
             what_changed = ATTACHMENT_REMOVED
             old_value = "%s %s" % (
                 self.old_value.title,
-                download_url_of_bugattachment(self.old_value),
+                self.old_value.displayed_url,
             )
             new_value = None
 
@@ -737,7 +731,7 @@ class BugAttachmentChange(AttributeChange):
             message = '** %s added: "%s"\n   %s' % (
                 attachment_str,
                 self.new_value.title,
-                download_url_of_bugattachment(self.new_value),
+                self.new_value.displayed_url,
             )
         else:
             if self.old_value.is_patch:
@@ -747,7 +741,7 @@ class BugAttachmentChange(AttributeChange):
             message = '** %s removed: "%s"\n   %s' % (
                 attachment_str,
                 self.old_value.title,
-                download_url_of_bugattachment(self.old_value),
+                self.old_value.displayed_url,
             )
 
         return {"text": message}
diff --git a/lib/lp/bugs/browser/bug.py b/lib/lp/bugs/browser/bug.py
index 3a675a8..717f39c 100644
--- a/lib/lp/bugs/browser/bug.py
+++ b/lib/lp/bugs/browser/bug.py
@@ -85,7 +85,6 @@ from lp.registry.interfaces.person import IPersonSet
 from lp.services.compat import message_as_bytes
 from lp.services.features import getFeatureFlag
 from lp.services.fields import DuplicateBug
-from lp.services.librarian.browser import ProxiedLibraryFileAlias
 from lp.services.mail.mailwrapper import MailWrapper
 from lp.services.propertycache import cachedproperty
 from lp.services.searchbuilder import any, not_equals
@@ -534,17 +533,11 @@ class BugViewMixin:
             "other": [],
         }
         for attachment in self.context.attachments_unpopulated:
-            info = {
-                "attachment": attachment,
-                "file": ProxiedLibraryFileAlias(
-                    attachment.libraryfile, attachment
-                ),
-            }
             if attachment.type == BugAttachmentType.PATCH:
                 key = attachment.type
             else:
                 key = "other"
-            result[key].append(info)
+            result[key].append(attachment)
         return result
 
     @property
@@ -636,12 +629,6 @@ class BugView(LaunchpadView, BugViewMixin):
 
         return dupes
 
-    def proxiedUrlForLibraryFile(self, attachment):
-        """Return the proxied download URL for a Librarian file."""
-        return ProxiedLibraryFileAlias(
-            attachment.libraryfile, attachment
-        ).http_url
-
 
 class BugActivity(BugView):
 
@@ -1304,13 +1291,16 @@ class BugTextView(LaunchpadView):
 
     def attachment_text(self, attachment):
         """Return a text representation of a bug attachment."""
-        mime_type = normalize_mime_type.sub(
-            " ", attachment.libraryfile.mimetype
-        )
-        download_url = ProxiedLibraryFileAlias(
-            attachment.libraryfile, attachment
-        ).http_url
-        return "%s %s" % (download_url, mime_type)
+        if attachment.url:
+            if attachment.title != attachment.url:
+                return "{}: {}".format(attachment.title, attachment.url)
+            return attachment.url
+        elif attachment.libraryfile:
+            mime_type = normalize_mime_type.sub(
+                " ", attachment.libraryfile.mimetype
+            )
+            return "{} {}".format(attachment.displayed_url, mime_type)
+        raise AssertionError()
 
     def comment_text(self):
         """Return a text representation of bug comments."""
diff --git a/lib/lp/bugs/browser/bugattachment.py b/lib/lp/bugs/browser/bugattachment.py
index 7d45e45..ce004b8 100644
--- a/lib/lp/bugs/browser/bugattachment.py
+++ b/lib/lp/bugs/browser/bugattachment.py
@@ -25,10 +25,7 @@ from lp.bugs.interfaces.bugattachment import (
     IBugAttachmentIsPatchConfirmationForm,
     IBugAttachmentSet,
 )
-from lp.services.librarian.browser import (
-    FileNavigationMixin,
-    ProxiedLibraryFileAlias,
-)
+from lp.services.librarian.browser import FileNavigationMixin
 from lp.services.librarian.interfaces import ILibraryFileAliasWithParent
 from lp.services.webapp import GetitemNavigation, Navigation, canonical_url
 from lp.services.webapp.authorization import check_permission
@@ -115,15 +112,22 @@ class BugAttachmentEditView(LaunchpadFormView, BugAttachmentContentCheck):
         self.next_url = self.cancel_url = canonical_url(
             ICanonicalUrlData(context).inside
         )
+        if not context.libraryfile:
+            # Remove `contenttype` from the form if there's no file attached.
+            self.field_names = [
+                name for name in self.field_names if name != "contenttype"
+            ]
 
     @property
     def initial_values(self):
         attachment = self.context
-        return dict(
+        values = dict(
             title=attachment.title,
             patch=attachment.type == BugAttachmentType.PATCH,
-            contenttype=attachment.libraryfile.mimetype,
         )
+        if attachment.libraryfile:
+            values["contenttype"] = attachment.libraryfile.mimetype
+        return values
 
     def canEditAttachment(self, action):
         return check_permission("launchpad.Edit", self.context)
@@ -135,30 +139,38 @@ class BugAttachmentEditView(LaunchpadFormView, BugAttachmentContentCheck):
         else:
             new_type = BugAttachmentType.UNSPECIFIED
         if new_type != self.context.type:
-            filename = self.context.libraryfile.filename
-            file_content = self.context.libraryfile.read()
-            # We expect that users set data['patch'] to True only for
-            # real patch data, indicated by guessed_content_type ==
-            # 'text/x-diff'. If there are inconsistencies, we don't
-            # set the value automatically. Instead, we lead the user to
-            # another form where we ask them if they are sure about their
-            # choice of the patch flag.
-            new_type_consistent_with_guessed_type = (
-                self.attachmentTypeConsistentWithContentType(
-                    new_type == BugAttachmentType.PATCH, filename, file_content
+            if self.context.libraryfile:
+                filename = self.context.libraryfile.filename
+                file_content = self.context.libraryfile.read()
+                # We expect that users set data['patch'] to True only for
+                # real patch data, indicated by guessed_content_type ==
+                # 'text/x-diff'. If there are inconsistencies, we don't
+                # set the value automatically. Instead, we lead the user to
+                # another form where we ask them if they are sure about their
+                # choice of the patch flag.
+                new_type_consistent_with_guessed_type = (
+                    self.attachmentTypeConsistentWithContentType(
+                        new_type == BugAttachmentType.PATCH,
+                        filename,
+                        file_content,
+                    )
                 )
-            )
-            if new_type_consistent_with_guessed_type:
-                self.context.type = new_type
+                if new_type_consistent_with_guessed_type:
+                    self.context.type = new_type
+                else:
+                    self.next_url = self.nextUrlForInconsistentPatchFlags(
+                        self.context
+                    )
             else:
-                self.next_url = self.nextUrlForInconsistentPatchFlags(
-                    self.context
-                )
+                self.context.type = new_type
 
         if data["title"] != self.context.title:
             self.context.title = data["title"]
 
-        if self.context.libraryfile.mimetype != data["contenttype"]:
+        if (
+            self.context.libraryfile
+            and self.context.libraryfile.mimetype != data["contenttype"]
+        ):
             lfa_with_parent = getMultiAdapter(
                 (self.context.libraryfile, self.context),
                 ILibraryFileAliasWithParent,
@@ -167,14 +179,11 @@ class BugAttachmentEditView(LaunchpadFormView, BugAttachmentContentCheck):
 
     @action("Delete Attachment", name="delete", condition=canEditAttachment)
     def delete_action(self, action, data):
-        libraryfile_url = ProxiedLibraryFileAlias(
-            self.context.libraryfile, self.context
-        ).http_url
         self.request.response.addInfoNotification(
             structured(
                 'Attachment "<a href="%(url)s">%(name)s</a>" has been '
                 "deleted.",
-                url=libraryfile_url,
+                url=self.context.displayed_url,
                 name=self.context.title,
             )
         )
diff --git a/lib/lp/bugs/browser/bugcomment.py b/lib/lp/bugs/browser/bugcomment.py
index 6a32ff1..0f653f6 100644
--- a/lib/lp/bugs/browser/bugcomment.py
+++ b/lib/lp/bugs/browser/bugcomment.py
@@ -29,7 +29,6 @@ from lp.bugs.interfaces.bugmessage import IBugComment
 from lp.services.comments.browser.comment import download_body
 from lp.services.comments.browser.messagecomment import MessageComment
 from lp.services.config import config
-from lp.services.librarian.browser import ProxiedLibraryFileAlias
 from lp.services.messages.interfaces.message import IMessage
 from lp.services.propertycache import cachedproperty, get_property_cache
 from lp.services.webapp import (
@@ -378,12 +377,6 @@ class BugCommentBoxViewMixin:
         else:
             return False
 
-    def proxiedUrlOfLibraryFileAlias(self, attachment):
-        """Return the proxied URL for the Librarian file of the attachment."""
-        return ProxiedLibraryFileAlias(
-            attachment.libraryfile, attachment
-        ).http_url
-
 
 class BugCommentBoxView(LaunchpadView, BugCommentBoxViewMixin):
     """Render a comment box with reply field collapsed."""
diff --git a/lib/lp/bugs/browser/bugmessage.py b/lib/lp/bugs/browser/bugmessage.py
index b0267a1..30cff9e 100644
--- a/lib/lp/bugs/browser/bugmessage.py
+++ b/lib/lp/bugs/browser/bugmessage.py
@@ -10,6 +10,7 @@ __all__ = [
 from io import BytesIO
 
 from zope.component import getUtility
+from zope.formlib.textwidgets import TextWidget
 from zope.formlib.widget import CustomWidgetFactory
 from zope.formlib.widgets import TextAreaWidget
 
@@ -29,6 +30,9 @@ class BugMessageAddFormView(LaunchpadFormView, BugAttachmentContentCheck):
     custom_widget_comment = CustomWidgetFactory(
         TextAreaWidget, cssClass="comment-text"
     )
+    custom_widget_attachment_url = CustomWidgetFactory(
+        TextWidget, displayWidth=44, displayMaxWidth=250
+    )
 
     page_title = "Add a comment or attachment"
 
@@ -49,14 +53,19 @@ class BugMessageAddFormView(LaunchpadFormView, BugAttachmentContentCheck):
 
     def validate(self, data):
 
-        # Ensure either a comment or filecontent was provide, but only
-        # if no errors have already been noted.
+        # Ensure either a comment or filecontent or a URL was provided,
+        # but only if no errors have already been noted.
         if len(self.errors) == 0:
             comment = data.get("comment") or ""
             filecontent = data.get("filecontent", None)
-            if not comment.strip() and not filecontent:
+            attachment_url = data.get("attachment_url") or ""
+            if (
+                not comment.strip()
+                and not filecontent
+                and not attachment_url.strip()
+            ):
                 self.addError(
-                    "Either a comment or attachment " "must be provided."
+                    "Either a comment or attachment must be provided."
                 )
 
     @action("Post Comment", name="save")
@@ -73,8 +82,10 @@ class BugMessageAddFormView(LaunchpadFormView, BugAttachmentContentCheck):
         # Write proper FileUpload field and widget instead of this hack.
         file_ = self.request.form.get(self.widgets["filecontent"].name)
 
+        attachment_url = data.get("attachment_url")
+
         message = None
-        if data["comment"] or file_:
+        if data["comment"] or file_ or attachment_url:
             bugwatch_id = data.get("bugwatch_id")
             if bugwatch_id is not None:
                 bugwatch = getUtility(IBugWatchSet).get(bugwatch_id)
@@ -87,7 +98,7 @@ class BugMessageAddFormView(LaunchpadFormView, BugAttachmentContentCheck):
                 bugwatch=bugwatch,
             )
 
-            # A blank comment with only a subect line is always added
+            # A blank comment with only a subject line is always added
             # when the user attaches a file, so show the add comment
             # feedback message only when the user actually added a
             # comment.
@@ -97,6 +108,9 @@ class BugMessageAddFormView(LaunchpadFormView, BugAttachmentContentCheck):
                 )
 
         self.next_url = canonical_url(self.context)
+
+        attachment_description = data.get("attachment_description")
+
         if file_:
 
             # Slashes in filenames cause problems, convert them to dashes
@@ -104,11 +118,8 @@ class BugMessageAddFormView(LaunchpadFormView, BugAttachmentContentCheck):
             filename = file_.filename.replace("/", "-")
 
             # if no description was given use the converted filename
-            file_description = None
-            if "attachment_description" in data:
-                file_description = data["attachment_description"]
-            if not file_description:
-                file_description = filename
+            if not attachment_description:
+                attachment_description = filename
 
             # Process the attachment.
             # If the patch flag is not consistent with the result of
@@ -132,7 +143,8 @@ class BugMessageAddFormView(LaunchpadFormView, BugAttachmentContentCheck):
                 owner=self.user,
                 data=BytesIO(data["filecontent"]),
                 filename=filename,
-                description=file_description,
+                url=None,
+                description=attachment_description,
                 comment=message,
                 is_patch=is_patch,
             )
@@ -146,6 +158,21 @@ class BugMessageAddFormView(LaunchpadFormView, BugAttachmentContentCheck):
                 "Attachment %s added to bug." % filename
             )
 
+        elif attachment_url:
+            is_patch = data["patch"]
+            bug.addAttachment(
+                owner=self.user,
+                data=None,
+                filename=None,
+                url=attachment_url,
+                description=attachment_description,
+                comment=message,
+                is_patch=is_patch,
+            )
+            self.request.response.addNotification(
+                "Attachment %s added to bug." % attachment_url
+            )
+
     def shouldShowEmailMeWidget(self):
         """Should the subscribe checkbox be shown?"""
         return not self.context.bug.isSubscribed(self.user)
diff --git a/lib/lp/bugs/browser/bugtarget.py b/lib/lp/bugs/browser/bugtarget.py
index e7b3dc6..43e084f 100644
--- a/lib/lp/bugs/browser/bugtarget.py
+++ b/lib/lp/bugs/browser/bugtarget.py
@@ -117,7 +117,6 @@ from lp.registry.vocabularies import ValidPersonOrTeamVocabulary
 from lp.services.config import config
 from lp.services.features import getFeatureFlag
 from lp.services.job.interfaces.job import JobStatus
-from lp.services.librarian.browser import ProxiedLibraryFileAlias
 from lp.services.propertycache import cachedproperty
 from lp.services.webapp import LaunchpadView, canonical_url, urlappend
 from lp.services.webapp.authorization import check_permission
@@ -673,6 +672,7 @@ class FileBugViewBase(LaunchpadFormView):
                     owner=self.user,
                     data=BytesIO(data["filecontent"]),
                     filename=filename,
+                    url=None,
                     description=file_description,
                     comment=attachment_comment,
                     is_patch=data["patch"],
@@ -686,6 +686,7 @@ class FileBugViewBase(LaunchpadFormView):
                 bug.linkAttachment(
                     owner=self.user,
                     file_alias=attachment["file_alias"],
+                    url=None,
                     description=attachment["description"],
                     comment=attachment_comment,
                     send_notifications=False,
@@ -1472,10 +1473,6 @@ class BugsPatchesView(LaunchpadView):
         now = datetime.now(timezone("UTC"))
         return now - patch.message.datecreated
 
-    def proxiedUrlForLibraryFile(self, patch):
-        """Return the proxied download URL for a Librarian file."""
-        return ProxiedLibraryFileAlias(patch.libraryfile, patch).http_url
-
 
 class TargetSubscriptionView(LaunchpadView):
     """A view to show all a person's structural subscriptions to a target."""
diff --git a/lib/lp/bugs/browser/tests/bug-views.rst b/lib/lp/bugs/browser/tests/bug-views.rst
index 74a9fd7..008659a 100644
--- a/lib/lp/bugs/browser/tests/bug-views.rst
+++ b/lib/lp/bugs/browser/tests/bug-views.rst
@@ -334,22 +334,22 @@ librarian file of the attachment.
     ... )
     >>> view = BugView(bug_seven, request)
     >>> for attachment in view.regular_attachments:
-    ...     print(attachment["attachment"].title)
+    ...     print(attachment.title)
     ...
     attachment 1
     attachment 2
     >>> for patch in view.patches:
-    ...     print(patch["attachment"].title)
+    ...     print(patch.title)
     ...
     patch 1
     patch 2
     >>> for attachment in view.regular_attachments:
-    ...     print(attachment["file"].http_url)
+    ...     print(attachment.displayed_url)
     ...
     http://bugs.launchpad.test/firefox/+bug/5/+attachment/.../+files/a1
     http://bugs.launchpad.test/firefox/+bug/5/+attachment/.../+files/a2
     >>> for patch in view.patches:
-    ...     print(patch["file"].http_url)
+    ...     print(patch.displayed_url)
     ...
     http://bugs.launchpad.test/firefox/+bug/5/+attachment/.../+files/p1
     http://bugs.launchpad.test/firefox/+bug/5/+attachment/.../+files/p2
diff --git a/lib/lp/bugs/browser/tests/test_bug_views.py b/lib/lp/bugs/browser/tests/test_bug_views.py
index 2264af4..787b257 100644
--- a/lib/lp/bugs/browser/tests/test_bug_views.py
+++ b/lib/lp/bugs/browser/tests/test_bug_views.py
@@ -779,6 +779,25 @@ class TestBugMessageAddFormView(TestCaseWithFactory):
         )
         self.assertEqual(0, len(view.errors))
 
+    def test_add_attachment_with_url(self):
+        bug = self.factory.makeBug()
+        form = {
+            "field.comment": " ",
+            "field.actions.save": "Post Comment",
+            "field.attachment_url": "https://launchpad.net",
+            "field.attachment_description": "description",
+            "field.patch.used": "",
+        }
+        login_person(self.factory.makePerson())
+        view = create_initialized_view(
+            bug.default_bugtask, "+addcomment", form=form
+        )
+        self.assertEqual([], view.errors)
+        attachments = list(bug.attachments)
+        self.assertEqual(1, len(attachments))
+        self.assertEqual("description", attachments[0].title)
+        self.assertEqual("https://launchpad.net", attachments[0].url)
+
 
 class TestBugMarkAsDuplicateView(TestCaseWithFactory):
     """Tests for marking a bug as a duplicate."""
diff --git a/lib/lp/bugs/browser/tests/test_bugview.py b/lib/lp/bugs/browser/tests/test_bugview.py
index 04d2e84..1124544 100644
--- a/lib/lp/bugs/browser/tests/test_bugview.py
+++ b/lib/lp/bugs/browser/tests/test_bugview.py
@@ -35,10 +35,7 @@ class TestBugView(TestCaseWithFactory):
         removeSecurityProxy(attachment.libraryfile).content = None
         self.assertEqual(
             ["regular attachment"],
-            [
-                attachment["attachment"].title
-                for attachment in self.view.regular_attachments
-            ],
+            [attachment.title for attachment in self.view.regular_attachments],
         )
 
     def test_patches_dont_include_invalid_records(self):
@@ -55,10 +52,7 @@ class TestBugView(TestCaseWithFactory):
         removeSecurityProxy(patch.libraryfile).content = None
         self.assertEqual(
             ["patch"],
-            [
-                attachment["attachment"].title
-                for attachment in self.view.patches
-            ],
+            [attachment.title for attachment in self.view.patches],
         )
 
 
diff --git a/lib/lp/bugs/doc/bug-export.rst b/lib/lp/bugs/doc/bug-export.rst
index 3f466dd..afe9b09 100644
--- a/lib/lp/bugs/doc/bug-export.rst
+++ b/lib/lp/bugs/doc/bug-export.rst
@@ -142,13 +142,23 @@ the file when we later serialise the bug:
     >>> bug4 = getUtility(IBugSet).get(4)
     >>> sampleperson = getUtility(IPersonSet).getByEmail("[e-mail]")
     >>> bug4.addAttachment(
-    ...     sampleperson,
-    ...     io.BytesIO(b"Hello World"),
-    ...     "Added attachment",
-    ...     "hello.txt",
+    ...     owner=sampleperson,
+    ...     data=io.BytesIO(b"Hello World"),
+    ...     comment="Added attachment",
+    ...     filename="hello.txt",
+    ...     url=None,
     ...     description='"Hello World" attachment',
     ... )
     <lp.bugs.model.bugattachment.BugAttachment ...>
+    >>> bug4.addAttachment(
+    ...     owner=sampleperson,
+    ...     data=None,
+    ...     comment="Added attachment with URL",
+    ...     filename=None,
+    ...     url="https://launchpad.net/",
+    ...     description=None,
+    ... )
+    <lp.bugs.model.bugattachment.BugAttachment ...>
 
     >>> transaction.commit()
 
@@ -166,13 +176,21 @@ attachment contents encoded using base-64:
     <text>Added attachment</text>
     <attachment href="http://bugs.launchpad.test/bugs/4/.../+files/hello.txt">
     <type>UNSPECIFIED</type>
-    <filename>hello.txt</filename>
     <title>"Hello World" attachment</title>
+    <filename>hello.txt</filename>
     <mimetype>text/plain</mimetype>
     <contents>SGVsbG8gV29ybGQ=
     </contents>
     </attachment>
     </comment>
+    <comment>
+    <sender name="name12">Sample Person</sender>
+    <date>...</date>
+    <text>Added attachment with URL</text>
+    <attachment href="https://launchpad.net/">
+    <type>UNSPECIFIED</type>
+    <title>https://launchpad.net/</title>
+    </attachment>
     ...
 
 
diff --git a/lib/lp/bugs/doc/bug.rst b/lib/lp/bugs/doc/bug.rst
index e28eff2..848028f 100644
--- a/lib/lp/bugs/doc/bug.rst
+++ b/lib/lp/bugs/doc/bug.rst
@@ -742,6 +742,7 @@ Adding an attachment.
     >>> attachment = attachmentset.create(
     ...     bug=firefox_bug,
     ...     filealias=filealias,
+    ...     url=None,
     ...     title="Some info.",
     ...     message=message,
     ... )
diff --git a/lib/lp/bugs/doc/bugattachments.rst b/lib/lp/bugs/doc/bugattachments.rst
index d2f99c8..58e16f1 100644
--- a/lib/lp/bugs/doc/bugattachments.rst
+++ b/lib/lp/bugs/doc/bugattachments.rst
@@ -53,6 +53,7 @@ ObjectCreatedEvent in order to trigger email notifications:
     ...     owner=foobar,
     ...     data=data,
     ...     filename="foo.bar",
+    ...     url=None,
     ...     description="this fixes the bug",
     ...     comment=message,
     ...     is_patch=False,
@@ -77,6 +78,7 @@ passed in is often a file-like object, but can be bytes too.
     ...     owner=foobar,
     ...     data=data,
     ...     filename="foo.baz",
+    ...     url=None,
     ...     description="this fixes the bug",
     ...     comment="a string comment",
     ...     is_patch=False,
@@ -92,6 +94,7 @@ If no description is given, the title is set to the filename.
     >>> screenshot = bug_four.addAttachment(
     ...     owner=foobar,
     ...     data=data,
+    ...     url=None,
     ...     filename="screenshot.jpg",
     ...     comment="a string comment",
     ...     is_patch=False,
@@ -110,6 +113,7 @@ The content type is guessed based on the information provided.
     ...     owner=foobar,
     ...     data=data,
     ...     filename="something.debdiff",
+    ...     url=None,
     ...     comment="something debdiffish",
     ...     is_patch=False,
     ... )
@@ -503,6 +507,7 @@ It's also possible to delete attachments.
     ...     owner=foobar,
     ...     data=data,
     ...     filename="foo.baz",
+    ...     url=None,
     ...     description="Attachment to be deleted",
     ...     comment="a string comment",
     ...     is_patch=False,
@@ -552,6 +557,7 @@ property returning True.
     ...     owner=foobar,
     ...     data=BytesIO(filecontent.getvalue()),
     ...     filename="foo.baz",
+    ...     url=None,
     ...     description="A non-patch attachment",
     ...     comment="a string comment",
     ...     is_patch=False,
@@ -564,6 +570,7 @@ property returning True.
     ...     owner=foobar,
     ...     data=BytesIO(filecontent.getvalue()),
     ...     filename="foo.baz",
+    ...     url=None,
     ...     description="A patch attachment",
     ...     comment="a string comment",
     ...     is_patch=True,
@@ -601,7 +608,10 @@ LibraryFileAlias.restricted and Bug.private. See also the section
 
     >>> bug = factory.makeBug()
     >>> bug.linkAttachment(
-    ...     owner=bug.owner, file_alias=file_alias, comment="Some attachment"
+    ...     owner=bug.owner,
+    ...     file_alias=file_alias,
+    ...     url=None,
+    ...     comment="Some attachment",
     ... )
     <lp.bugs.model.bugattachment.BugAttachment ...>
 
@@ -631,6 +641,7 @@ meaningful description.
     >>> bug.linkAttachment(
     ...     owner=bug.owner,
     ...     file_alias=file_alias,
+    ...     url=None,
     ...     comment="Some attachment",
     ...     is_patch=True,
     ...     description="An attachment of some sort",
@@ -688,6 +699,7 @@ its Librarian file is set.
     ...     owner=private_bug_owner,
     ...     data=b"secret",
     ...     filename="baz.txt",
+    ...     url=None,
     ...     comment="Some attachment",
     ... )
     >>> private_attachment.libraryfile.restricted
diff --git a/lib/lp/bugs/doc/bugcomment.rst b/lib/lp/bugs/doc/bugcomment.rst
index 14a8b71..5157e55 100644
--- a/lib/lp/bugs/doc/bugcomment.rst
+++ b/lib/lp/bugs/doc/bugcomment.rst
@@ -77,6 +77,7 @@ in activity_and_comments...
     ...     data=BytesIO(b"whatever"),
     ...     comment=bug_11.initial_message,
     ...     filename="test.txt",
+    ...     url=None,
     ...     is_patch=False,
     ...     content_type="text/plain",
     ...     description="sample data",
@@ -235,6 +236,7 @@ attachments to a bug to see this in action:
     ...     data=file_,
     ...     description="Ho",
     ...     filename="munchy",
+    ...     url=None,
     ...     comment="Hello there",
     ... )
     >>> m6 = bug_three.newMessage(
@@ -436,6 +438,7 @@ not included in BugComment.patches.
     ...     data=BytesIO(b"whatever"),
     ...     comment=bug.initial_message,
     ...     filename="file1",
+    ...     url=None,
     ...     is_patch=False,
     ...     content_type="text/plain",
     ...     description="sample data 1",
@@ -445,6 +448,7 @@ not included in BugComment.patches.
     ...     data=BytesIO(b"whatever"),
     ...     comment=bug.initial_message,
     ...     filename="file2",
+    ...     url=None,
     ...     is_patch=False,
     ...     content_type="text/plain",
     ...     description="sample data 2",
@@ -454,6 +458,7 @@ not included in BugComment.patches.
     ...     data=BytesIO(b"whatever"),
     ...     comment=bug.initial_message,
     ...     filename="patch1",
+    ...     url=None,
     ...     is_patch=True,
     ...     content_type="text/plain",
     ...     description="patch 1",
@@ -463,6 +468,7 @@ not included in BugComment.patches.
     ...     data=BytesIO(b"whatever"),
     ...     comment=bug.initial_message,
     ...     filename="patch2",
+    ...     url=None,
     ...     is_patch=True,
     ...     content_type="text/plain",
     ...     description="patch 2",
diff --git a/lib/lp/bugs/interfaces/bug.py b/lib/lp/bugs/interfaces/bug.py
index 9697660..e02f2ff 100644
--- a/lib/lp/bugs/interfaces/bug.py
+++ b/lib/lp/bugs/interfaces/bug.py
@@ -43,6 +43,7 @@ from lazr.restful.interface import copy_field
 from zope.component import getUtility
 from zope.interface import Attribute, Interface
 from zope.schema import (
+    URI,
     Bool,
     Bytes,
     Choice,
@@ -877,11 +878,12 @@ class IBugAppend(Interface):
 
     @call_with(owner=REQUEST_USER, from_api=True)
     @operation_parameters(
-        data=Bytes(constraint=attachment_size_constraint),
+        data=Bytes(constraint=attachment_size_constraint, required=False),
         comment=Text(),
-        filename=TextLine(),
+        filename=TextLine(required=False),
+        url=URI(required=False),
         is_patch=Bool(),
-        content_type=TextLine(),
+        content_type=TextLine(required=False),
         description=Text(),
     )
     @export_factory_operation(IBugAttachment, [])
@@ -891,6 +893,7 @@ class IBugAppend(Interface):
         data,
         comment,
         filename,
+        url,
         is_patch=False,
         content_type=None,
         description=None,
@@ -903,6 +906,7 @@ class IBugAppend(Interface):
         :description: A brief description of the attachment.
         :comment: An IMessage or string.
         :filename: A string.
+        :url: External URL of the attachment
         :is_patch: A boolean.
         """
 
@@ -956,12 +960,13 @@ class IBugAppend(Interface):
         """
 
     def linkAttachment(
-        owner, file_alias, comment, is_patch=False, description=None
+        owner, file_alias, url, comment, is_patch=False, description=None
     ):
         """Link an `ILibraryFileAlias` to this bug.
 
         :owner: An IPerson.
         :file_alias: The `ILibraryFileAlias` to link to this bug.
+        :url: External URL of the attachment.
         :description: A brief description of the attachment.
         :comment: An IMessage or string.
         :is_patch: A boolean.
diff --git a/lib/lp/bugs/interfaces/bugattachment.py b/lib/lp/bugs/interfaces/bugattachment.py
index 0ded472..4edb5de 100644
--- a/lib/lp/bugs/interfaces/bugattachment.py
+++ b/lib/lp/bugs/interfaces/bugattachment.py
@@ -22,7 +22,7 @@ from lazr.restful.declarations import (
 )
 from lazr.restful.fields import Reference
 from zope.interface import Interface
-from zope.schema import Bool, Bytes, Choice, Int, TextLine
+from zope.schema import URI, Bool, Bytes, Choice, Int, TextLine
 
 from lp import _
 from lp.bugs.interfaces.hasbug import IHasBug
@@ -90,10 +90,13 @@ class IBugAttachmentView(IHasBug):
     )
     libraryfile = exported(
         Bytes(
-            title=_("The attachment content."), required=True, readonly=True
+            title=_("The attachment content."), required=False, readonly=True
         ),
         exported_as="data",
     )
+    url = exported(
+        URI(title=_("Attachment URL"), required=False, readonly=True)
+    )
     _message_id = Int(title=_("Message ID"))
     message = exported(
         Reference(
@@ -109,6 +112,12 @@ class IBugAttachmentView(IHasBug):
         description=_("Is this attachment a patch?"),
         readonly=True,
     )
+    displayed_url = URI(
+        title=_(
+            "Download URL of the files or the external URL of the attachment"
+        ),
+        readonly=True,
+    )
 
     def getFileByName(filename):
         """Return the `ILibraryFileAlias` for the given file name.
@@ -176,6 +185,7 @@ class IBugAttachmentSet(Interface):
     def create(
         bug,
         filealias,
+        url,
         title,
         message,
         type=IBugAttachment["type"].default,
@@ -184,7 +194,9 @@ class IBugAttachmentSet(Interface):
         """Create a new attachment and return it.
 
         :param bug: The `IBug` to which the new attachment belongs.
-        :param filealias: The `IFilealias` containing the data.
+        :param filealias: The `ILibraryFileAlias` containing
+            the data (optional).
+        :param url: External URL of the attachment (optional).
         :param message: The `IMessage` to which this attachment belongs.
         :param type: The type of attachment. See `BugAttachmentType`.
         :param send_notifications: If True, a notification is sent to
diff --git a/lib/lp/bugs/interfaces/bugmessage.py b/lib/lp/bugs/interfaces/bugmessage.py
index 7768b07..22c8d39 100644
--- a/lib/lp/bugs/interfaces/bugmessage.py
+++ b/lib/lp/bugs/interfaces/bugmessage.py
@@ -11,7 +11,7 @@ __all__ = [
 ]
 
 from zope.interface import Attribute, Interface
-from zope.schema import Bool, Bytes, Int, Object, Text, TextLine
+from zope.schema import URI, Bool, Bytes, Int, Object, Text, TextLine
 
 from lp.app.validators.attachment import attachment_size_constraint
 from lp.bugs.interfaces.bug import IBug
@@ -103,6 +103,7 @@ class IBugMessageAddForm(Interface):
         required=False,
         constraint=attachment_size_constraint,
     )
+    attachment_url = URI(title="URL", required=False)
     patch = Bool(
         title="This attachment contains a solution (patch) for this bug",
         required=False,
diff --git a/lib/lp/bugs/mail/handler.py b/lib/lp/bugs/mail/handler.py
index 975af40..775b570 100644
--- a/lib/lp/bugs/mail/handler.py
+++ b/lib/lp/bugs/mail/handler.py
@@ -410,6 +410,7 @@ class MaloneHandler:
             getUtility(IBugAttachmentSet).create(
                 bug=bug,
                 filealias=blob,
+                url=None,
                 attach_type=attach_type,
                 title=blob.filename,
                 message=message,
diff --git a/lib/lp/bugs/model/bug.py b/lib/lp/bugs/model/bug.py
index 5b1465c..88f793d 100644
--- a/lib/lp/bugs/model/bug.py
+++ b/lib/lp/bugs/model/bug.py
@@ -1608,6 +1608,7 @@ class Bug(SQLBase, InformationTypeMixin):
         data,
         comment,
         filename,
+        url,
         is_patch=False,
         content_type=None,
         description=None,
@@ -1619,35 +1620,45 @@ class Bug(SQLBase, InformationTypeMixin):
         # wrongly encoded.
         if from_api:
             data = get_raw_form_value_from_current_request(data, "data")
-        if isinstance(data, bytes):
-            filecontent = data
-        else:
-            filecontent = data.read()
 
-        if is_patch:
-            content_type = "text/plain"
-        else:
-            if content_type is None:
-                content_type, encoding = guess_content_type(
-                    name=filename, body=filecontent
-                )
+        if data:
+            if isinstance(data, bytes):
+                filecontent = data
+            else:
+                filecontent = data.read()
 
-        filealias = getUtility(ILibraryFileAliasSet).create(
-            name=filename,
-            size=len(filecontent),
-            file=BytesIO(filecontent),
-            contentType=content_type,
-            restricted=self.private,
-        )
+            if is_patch:
+                content_type = "text/plain"
+            else:
+                if content_type is None:
+                    content_type, encoding = guess_content_type(
+                        name=filename, body=filecontent
+                    )
+
+            file_alias = getUtility(ILibraryFileAliasSet).create(
+                name=filename,
+                size=len(filecontent),
+                file=BytesIO(filecontent),
+                contentType=content_type,
+                restricted=self.private,
+            )
+        else:
+            file_alias = None
 
         return self.linkAttachment(
-            owner, filealias, comment, is_patch, description
+            owner=owner,
+            file_alias=file_alias,
+            url=url,
+            comment=comment,
+            is_patch=is_patch,
+            description=description,
         )
 
     def linkAttachment(
         self,
         owner,
         file_alias,
+        url,
         comment,
         is_patch=False,
         description=None,
@@ -1670,11 +1681,6 @@ class Bug(SQLBase, InformationTypeMixin):
         else:
             attach_type = BugAttachmentType.UNSPECIFIED
 
-        if description:
-            title = description
-        else:
-            title = file_alias.filename
-
         if IMessage.providedBy(comment):
             message = comment
         else:
@@ -1685,8 +1691,9 @@ class Bug(SQLBase, InformationTypeMixin):
         return getUtility(IBugAttachmentSet).create(
             bug=self,
             filealias=file_alias,
+            url=url,
             attach_type=attach_type,
-            title=title,
+            title=description,
             message=message,
             send_notifications=send_notifications,
         )
@@ -2206,9 +2213,10 @@ class Bug(SQLBase, InformationTypeMixin):
         # XXX: This should be a bulk update. RBC 20100827
         # bug=https://bugs.launchpad.net/storm/+bug/625071
         for attachment in self.attachments_unpopulated:
-            attachment.libraryfile.restricted = (
-                information_type in PRIVATE_INFORMATION_TYPES
-            )
+            if attachment.libraryfile:
+                attachment.libraryfile.restricted = (
+                    information_type in PRIVATE_INFORMATION_TYPES
+                )
 
         self.information_type = information_type
         self._reconcileAccess()
@@ -2558,16 +2566,34 @@ class Bug(SQLBase, InformationTypeMixin):
 
     def _attachments_query(self):
         """Helper for the attachments* properties."""
-        # bug attachments with no LibraryFileContent have been deleted - the
-        # garbo_daily run will remove the LibraryFileAlias asynchronously.
-        # See bug 542274 for more details.
+        # get bug attachments that have either a URL,
+        # or associated LibraryFileContent
+        # bug attachments with LibraryFileAlias and no LibraryFileContent have
+        # been deleted - the garbo_daily run will remove the LibraryFileAlias
+        # asynchronously. See bug 542274 for more details.
         store = Store.of(self)
-        return store.find(
-            (BugAttachment, LibraryFileAlias, LibraryFileContent),
-            BugAttachment.bug == self,
-            BugAttachment.libraryfile == LibraryFileAlias.id,
-            LibraryFileContent.id == LibraryFileAlias.contentID,
-        ).order_by(BugAttachment.id)
+        return (
+            store.using(
+                BugAttachment,
+                LeftJoin(
+                    LibraryFileAlias,
+                    BugAttachment.libraryfile == LibraryFileAlias.id,
+                ),
+                LeftJoin(
+                    LibraryFileContent,
+                    LibraryFileContent.id == LibraryFileAlias.contentID,
+                ),
+            )
+            .find(
+                (BugAttachment, LibraryFileAlias, LibraryFileContent),
+                BugAttachment.bug == self,
+                Or(
+                    BugAttachment.url != None,
+                    LibraryFileAlias.contentID != None,
+                ),
+            )
+            .order_by(BugAttachment.id)
+        )
 
     @property
     def attachments(self):
diff --git a/lib/lp/bugs/model/bugattachment.py b/lib/lp/bugs/model/bugattachment.py
index 180c353..064482a 100644
--- a/lib/lp/bugs/model/bugattachment.py
+++ b/lib/lp/bugs/model/bugattachment.py
@@ -17,6 +17,7 @@ from lp.bugs.interfaces.bugattachment import (
 from lp.services.database.enumcol import DBEnum
 from lp.services.database.interfaces import IStore
 from lp.services.database.stormbase import StormBase
+from lp.services.librarian.browser import ProxiedLibraryFileAlias
 from lp.services.propertycache import cachedproperty
 
 
@@ -35,9 +36,10 @@ class BugAttachment(StormBase):
         allow_none=False,
         default=IBugAttachment["type"].default,
     )
-    title = Unicode(allow_none=False)
-    libraryfile_id = Int(name="libraryfile", allow_none=False)
+    _title = Unicode(name="title", allow_none=True)
+    libraryfile_id = Int(name="libraryfile", allow_none=True)
     libraryfile = Reference(libraryfile_id, "LibraryFileAlias.id")
+    url = Unicode(allow_none=True)
     _message_id = Int(name="message", allow_none=False)
     _message = Reference(_message_id, "Message.id")
 
@@ -46,6 +48,7 @@ class BugAttachment(StormBase):
         bug,
         title,
         libraryfile,
+        url,
         message,
         type=IBugAttachment["type"].default,
     ):
@@ -53,9 +56,22 @@ class BugAttachment(StormBase):
         self.bug = bug
         self.title = title
         self.libraryfile = libraryfile
+        self.url = url
         self._message = message
         self.type = type
 
+    @property
+    def title(self) -> str:
+        if self._title:
+            return self._title
+        if self.libraryfile:
+            return self.libraryfile.filename
+        return self.url
+
+    @title.setter
+    def title(self, title) -> None:
+        self._title = title
+
     @cachedproperty
     def message(self):
         """This is a cachedproperty to allow message to be an IIndexedMessage.
@@ -81,15 +97,23 @@ class BugAttachment(StormBase):
         # Delete the reference to the LibraryFileContent record right now,
         # in order to avoid problems with not deleted files as described
         # in bug 387188.
-        self.libraryfile.content = None
+        if self.libraryfile:
+            self.libraryfile.content = None
         Store.of(self).remove(self)
 
     def getFileByName(self, filename):
         """See IBugAttachment."""
-        if filename == self.libraryfile.filename:
+        if self.libraryfile and filename == self.libraryfile.filename:
             return self.libraryfile
         raise NotFoundError(filename)
 
+    @property
+    def displayed_url(self):
+        return (
+            self.url
+            or ProxiedLibraryFileAlias(self.libraryfile, self).http_url
+        )
+
 
 @implementer(IBugAttachmentSet)
 class BugAttachmentSet:
@@ -110,18 +134,26 @@ class BugAttachmentSet:
         self,
         bug,
         filealias,
+        url,
         title,
         message,
         attach_type=None,
         send_notifications=False,
     ):
         """See `IBugAttachmentSet`."""
+        if not filealias and not url:
+            raise ValueError("Either filealias or url must be provided")
+
+        if filealias and url:
+            raise ValueError("Only one of filealias or url may be provided")
+
         if attach_type is None:
             # XXX kiko 2005-08-03 bug=1659: this should use DEFAULT.
             attach_type = IBugAttachment["type"].default
         attachment = BugAttachment(
             bug=bug,
             libraryfile=filealias,
+            url=url,
             type=attach_type,
             title=title,
             message=message,
diff --git a/lib/lp/bugs/model/tests/test_bugtasksearch.py b/lib/lp/bugs/model/tests/test_bugtasksearch.py
index db52d71..05793e0 100644
--- a/lib/lp/bugs/model/tests/test_bugtasksearch.py
+++ b/lib/lp/bugs/model/tests/test_bugtasksearch.py
@@ -268,6 +268,7 @@ class OnceTests:
                 data=b"filedata",
                 comment="a comment",
                 filename="file1.txt",
+                url=None,
                 is_patch=False,
             )
             self.bugtasks[1].bug.addAttachment(
@@ -275,6 +276,7 @@ class OnceTests:
                 data=b"filedata",
                 comment="a comment",
                 filename="file1.txt",
+                url=None,
                 is_patch=True,
             )
         # We can search for bugs with non-patch attachments...
diff --git a/lib/lp/bugs/scripts/bugexport.py b/lib/lp/bugs/scripts/bugexport.py
index 3d3327a..c6c0c43 100644
--- a/lib/lp/bugs/scripts/bugexport.py
+++ b/lib/lp/bugs/scripts/bugexport.py
@@ -16,7 +16,6 @@ from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.bugs.browser.bugtask import get_comments_for_bugtask
 from lp.bugs.interfaces.bugtask import IBugTaskSet
 from lp.bugs.interfaces.bugtasksearch import BugTaskSearchParams
-from lp.services.librarian.browser import ProxiedLibraryFileAlias
 
 BUGS_XMLNS = "https://launchpad.net/xmlns/2006/bugs"
 
@@ -85,27 +84,30 @@ def serialise_bugtask(bugtask):
             attachment_node = ET.SubElement(
                 comment_node,
                 "attachment",
-                href=ProxiedLibraryFileAlias(
-                    attachment.libraryfile, attachment
-                ).http_url,
+                href=attachment.displayed_url,
             )
             attachment_node.text = attachment_node.tail = "\n"
             addnode(attachment_node, "type", attachment.type.name)
-            addnode(
-                attachment_node, "filename", attachment.libraryfile.filename
-            )
             addnode(attachment_node, "title", attachment.title)
-            addnode(
-                attachment_node, "mimetype", attachment.libraryfile.mimetype
-            )
-            # Attach the attachment file contents, base 64 encoded.
-            addnode(
-                attachment_node,
-                "contents",
-                base64.encodebytes(attachment.libraryfile.read()).decode(
-                    "ASCII"
-                ),
-            )
+            if attachment.libraryfile:
+                addnode(
+                    attachment_node,
+                    "filename",
+                    attachment.libraryfile.filename,
+                )
+                addnode(
+                    attachment_node,
+                    "mimetype",
+                    attachment.libraryfile.mimetype,
+                )
+                # Attach the attachment file contents, base 64 encoded.
+                addnode(
+                    attachment_node,
+                    "contents",
+                    base64.encodebytes(attachment.libraryfile.read()).decode(
+                        "ASCII"
+                    ),
+                )
 
     return bug_node
 
diff --git a/lib/lp/bugs/scripts/bugimport.py b/lib/lp/bugs/scripts/bugimport.py
index 323c9b1..38306fb 100644
--- a/lib/lp/bugs/scripts/bugimport.py
+++ b/lib/lp/bugs/scripts/bugimport.py
@@ -445,6 +445,7 @@ class BugImporter:
             getUtility(IBugAttachmentSet).create(
                 bug=bug,
                 filealias=filealias,
+                url=None,
                 attach_type=attach_type,
                 title=title,
                 message=message,
diff --git a/lib/lp/bugs/scripts/tests/test_bugnotification.py b/lib/lp/bugs/scripts/tests/test_bugnotification.py
index f9aa65d..80fee9c 100644
--- a/lib/lp/bugs/scripts/tests/test_bugnotification.py
+++ b/lib/lp/bugs/scripts/tests/test_bugnotification.py
@@ -1026,7 +1026,7 @@ class TestEmailNotificationsAttachments(
             # another five minutes.  Therefore, we send out separate
             # change notifications.
             return self.bug.addAttachment(
-                self.person, b"content", "a comment", "stuff.txt"
+                self.person, b"content", "a comment", "stuff.txt", url=None
             )
 
     old = cachedproperty("old")(_attachment)
diff --git a/lib/lp/bugs/stories/bugattachments/xx-attachments-to-bug-report.rst b/lib/lp/bugs/stories/bugattachments/xx-attachments-to-bug-report.rst
index 090ca83..9df6720 100644
--- a/lib/lp/bugs/stories/bugattachments/xx-attachments-to-bug-report.rst
+++ b/lib/lp/bugs/stories/bugattachments/xx-attachments-to-bug-report.rst
@@ -35,6 +35,7 @@ If we add an attachment to the bug report...
     ...     data=BytesIO(b"whatever"),
     ...     comment=bug_11.initial_message,
     ...     filename="test.txt",
+    ...     url=None,
     ...     is_patch=False,
     ...     content_type="text/plain",
     ...     description="sample data",
@@ -74,6 +75,7 @@ Patch attachments are shown before non-patch attachments
     ...     data=BytesIO(b"patchy patch patch"),
     ...     comment=bug_11.initial_message,
     ...     filename="patch.txt",
+    ...     url=None,
     ...     is_patch=True,
     ...     content_type="text/plain",
     ...     description="a patch",
diff --git a/lib/lp/bugs/stories/bugs/xx-bug-text-pages.rst b/lib/lp/bugs/stories/bugs/xx-bug-text-pages.rst
index 93e9cc6..bfa0b6e 100644
--- a/lib/lp/bugs/stories/bugs/xx-bug-text-pages.rst
+++ b/lib/lp/bugs/stories/bugs/xx-bug-text-pages.rst
@@ -23,6 +23,7 @@ We'll start by adding some attachments to the bug:
     ...     content,
     ...     "comment for file a",
     ...     "file_a.txt",
+    ...     url=None,
     ...     content_type="text/html",
     ... )
     >>> content = BytesIO(b"do we need to")
@@ -31,6 +32,7 @@ We'll start by adding some attachments to the bug:
     ...     content,
     ...     "comment for file with space",
     ...     "file with space.txt",
+    ...     url=None,
     ...     content_type='text/plain;\n  name="file with space.txt"',
     ... )
     >>> content = BytesIO(b"Yes we can!")
@@ -39,6 +41,7 @@ We'll start by adding some attachments to the bug:
     ...     content,
     ...     "comment for patch",
     ...     "bug-patch.diff",
+    ...     url=None,
     ...     is_patch=True,
     ...     content_type="text/plain",
     ...     description="a patch",
diff --git a/lib/lp/bugs/stories/bugtask-searches/xx-listing-basics.rst b/lib/lp/bugs/stories/bugtask-searches/xx-listing-basics.rst
index 12f5e92..bed6dba 100644
--- a/lib/lp/bugs/stories/bugtask-searches/xx-listing-basics.rst
+++ b/lib/lp/bugs/stories/bugtask-searches/xx-listing-basics.rst
@@ -311,6 +311,7 @@ Patches also appear as badges in bug listings.
     ...     owner=foobar,
     ...     data=BytesIO(b"file data"),
     ...     filename="foo.bar",
+    ...     url=None,
     ...     description="this fixes the bug",
     ...     comment=message,
     ...     is_patch=True,
diff --git a/lib/lp/bugs/stories/patches-view/patches-view.rst b/lib/lp/bugs/stories/patches-view/patches-view.rst
index 8a5d5a7..786a75d 100644
--- a/lib/lp/bugs/stories/patches-view/patches-view.rst
+++ b/lib/lp/bugs/stories/patches-view/patches-view.rst
@@ -101,7 +101,7 @@ patches view.
     Bug                                 Importance   Status    Patch Age
     Bug #...: bug_a title               Undecided     New     ...second...
     From: Patchy Person
-    Link: patch_a.diff description of patch a
+    Link: description of patch a
 
 The page title and other wording in the page reflects the contents.
 
@@ -205,13 +205,13 @@ attachments, and various statuses...
     Bug                                 Importance   Status    Patch Age
     Bug #...: bug_c title               Wishlist  Fix Committed ...second...
     From: Patchy Person
-    Link: patch_f.diff description of patch f
+    Link: description of patch f
     Bug #...: bug_b title               Critical    Confirmed ...second...
     From: Patchy Person
-    Link: patch_c.diff description of patch c
+    Link: description of patch c
     Bug #...: bug_a title               Undecided     New     ...second...
     From: Patchy Person
-    Link: patch_a.diff description of patch a
+    Link: description of patch a
 
 The default sort order is by patch age. We can sort patches by
 importance and status.
@@ -224,13 +224,13 @@ importance and status.
     Bug                                 Importance   Status    Patch Age
     Bug #...: bug_b title               Critical    Confirmed ...second...
     From: Patchy Person
-    Link: patch_c.diff description of patch c
+    Link: description of patch c
     Bug #...: bug_c title               Wishlist  Fix Committed ...second...
     From: Patchy Person
-    Link: patch_f.diff description of patch f
+    Link: description of patch f
     Bug #...: bug_a title               Undecided     New     ...second...
     From: Patchy Person
-    Link: patch_a.diff description of patch a
+    Link: description of patch a
 
     >>> anon_browser.getControl(name="orderby").value = ["status"]
     >>> anon_browser.getControl("sort").click()
@@ -240,13 +240,13 @@ importance and status.
     Bug                                 Importance   Status    Patch Age
     Bug #...: bug_a title               Undecided     New     ...second...
     From: Patchy Person
-    Link: patch_a.diff description of patch a
+    Link: description of patch a
     Bug #...: bug_b title               Critical    Confirmed ...second...
     From: Patchy Person
-    Link: patch_c.diff description of patch c
+    Link: description of patch c
     Bug #...: bug_c title               Wishlist  Fix Committed ...second...
     From: Patchy Person
-    Link: patch_f.diff description of patch f
+    Link: description of patch f
 
 But we can't sort by things that aren't validated by the view.
 
@@ -327,12 +327,10 @@ Bugs in a product series show up in the patches view for that series.
     Bug                       Importance     Status      Patch Age
     Bug #...: bug_c title     Wishlist  Fix Committed   ...second...
     From: Patchy Person
-    Link: patch_f.diff
-    description of patch f
+    Link: description of patch f
     Bug #...: bug_a title     Undecided      New        ...second...
     From: Patchy Person
-    Link: patch_a.diff
-    description of patch a
+    Link: description of patch a
 
 The page title and other wording in the page reflects the contents.
 
diff --git a/lib/lp/bugs/stories/webservice/xx-bug.rst b/lib/lp/bugs/stories/webservice/xx-bug.rst
index 80c2f24..8266254 100644
--- a/lib/lp/bugs/stories/webservice/xx-bug.rst
+++ b/lib/lp/bugs/stories/webservice/xx-bug.rst
@@ -1531,6 +1531,7 @@ An attachment can be added to the bug:
     ...     "addAttachment",
     ...     data=io.BytesIO(b"12345"),
     ...     filename="numbers.txt",
+    ...     url=None,
     ...     content_type="foo/bar",
     ...     comment="The numbers you asked for.",
     ... )
@@ -1558,6 +1559,7 @@ Now, bug 1 has one attachment:
     self_link: 'http://.../bugs/1/+attachment/...'
     title: 'numbers.txt'
     type: 'Unspecified'
+    url: None
     web_link: 'http://bugs.../bugs/1/+attachment/...'
     ---
 
@@ -1572,6 +1574,7 @@ The attachment can be fetched directly:
     self_link: 'http://.../bugs/1/+attachment/...'
     title: 'numbers.txt'
     type: 'Unspecified'
+    url: None
     web_link: 'http://bugs.../bugs/1/+attachment/...'
 
 Fetching the data actually yields a redirect to the Librarian, which
diff --git a/lib/lp/bugs/templates/bug-attachment-macros.pt b/lib/lp/bugs/templates/bug-attachment-macros.pt
index a06b835..d91d0c3 100644
--- a/lib/lp/bugs/templates/bug-attachment-macros.pt
+++ b/lib/lp/bugs/templates/bug-attachment-macros.pt
@@ -15,6 +15,12 @@
       <metal:widget metal:use-macro="context/@@launchpad_form/widget_row" />
     </tal:filecontent>
 
+    <tal:url
+        tal:define="widget nocall:view/widgets/attachment_url|nothing"
+        tal:condition="widget">
+      <metal:widget metal:use-macro="context/@@launchpad_form/widget_row" />
+    </tal:url>
+
     <tal:patch
         tal:define="widget nocall:view/widgets/patch|nothing"
         tal:condition="widget">
diff --git a/lib/lp/bugs/templates/bug-portlet-attachments.pt b/lib/lp/bugs/templates/bug-portlet-attachments.pt
index 5a79a9f..399ceff 100644
--- a/lib/lp/bugs/templates/bug-portlet-attachments.pt
+++ b/lib/lp/bugs/templates/bug-portlet-attachments.pt
@@ -8,14 +8,10 @@
     <ul>
       <li class="download-attachment"
           tal:repeat="attachment view/patches">
-        <a tal:attributes="href attachment/file/http_url"
-           tal:content="attachment/attachment/title"
-           class="sprite haspatch-icon">
-          Attachment Title
-        </a>
-        <small>
-          (<a tal:attributes="href attachment/attachment/fmt:url">edit</a>)
-        </small>
+        <a tal:replace="structure attachment/fmt:attachment_link" />
+        <a class="sprite edit action-icon"
+           title="Change patch details"
+           tal:attributes="href attachment/fmt:url">Edit</a>
       </li>
     </ul>
     <ul>
@@ -31,14 +27,10 @@
     <ul>
       <li class="download-attachment"
           tal:repeat="attachment view/regular_attachments">
-        <a tal:attributes="href attachment/file/http_url"
-           tal:content="attachment/attachment/title"
-           class="sprite download-icon">
-          Attachment Title
-        </a>
-        <small>
-          (<a tal:attributes="href attachment/attachment/fmt:url">edit</a>)
-        </small>
+        <a tal:replace="structure attachment/fmt:attachment_link" />
+        <a class="sprite edit action-icon"
+           title="Change attachment details"
+           tal:attributes="href attachment/fmt:url">Edit</a>
       </li>
     </ul>
     <ul>
diff --git a/lib/lp/bugs/templates/bugcomment-box.pt b/lib/lp/bugs/templates/bugcomment-box.pt
index b25c482..58a9458 100644
--- a/lib/lp/bugs/templates/bugcomment-box.pt
+++ b/lib/lp/bugs/templates/bugcomment-box.pt
@@ -113,24 +113,25 @@
     <ul tal:condition="comment/bugattachments" style="margin-bottom: 1em">
       <li tal:repeat="attachment comment/bugattachments"
             class="download-attachment">
-        <a tal:attributes="href python: view.proxiedUrlOfLibraryFileAlias(attachment)"
-            tal:content="attachment/title"
-            class="sprite download-icon">foo.txt</a>
+        <a tal:replace="structure attachment/fmt:attachment_link" />
         <a tal:attributes="href attachment/fmt:url" class="sprite edit action-icon">Edit</a>
+        <tal:block condition="attachment/libraryfile">
         (<span
            tal:replace="attachment/libraryfile/content/filesize/fmt:bytes" />,
         <span tal:replace="attachment/libraryfile/mimetype" />)
+        </tal:block>
       </li>
     </ul>
 
     <ul tal:condition="comment/patches" style="margin-bottom: 1em">
       <li tal:repeat="attachment comment/patches" class="download-attachment">
-        <a tal:attributes="href python: view.proxiedUrlOfLibraryFileAlias(attachment)"
-           tal:content="attachment/title" class="sprite haspatch-icon">foo.txt</a>
+        <a tal:replace="structure attachment/fmt:attachment_link" />
         <a tal:attributes="href attachment/fmt:url" class="sprite edit action-icon">Edit</a>
+        <tal:block condition="attachment/libraryfile">
         (<span
            tal:replace="attachment/libraryfile/content/filesize/fmt:bytes" />,
         <span tal:replace="attachment/libraryfile/mimetype" />)
+        </tal:block>
       </li>
     </ul>
 
diff --git a/lib/lp/bugs/templates/bugtarget-patches.pt b/lib/lp/bugs/templates/bugtarget-patches.pt
index fca68c7..b4a0f8d 100644
--- a/lib/lp/bugs/templates/bugtarget-patches.pt
+++ b/lib/lp/bugs/templates/bugtarget-patches.pt
@@ -74,7 +74,7 @@
                  tal:define="patch patch_task/bug/latest_patch;
                              age python:view.patchAge(patch)"
                  tal:attributes="id string:patch-cell-${repeat/patch_task/index}">
-                   <a tal:attributes="href python:view.proxiedUrlForLibraryFile(patch)"
+                   <a tal:attributes="href patch/displayed_url; rel python:patch.url and 'nofollow' or None"
                       tal:content="age/fmt:approximateduration"></a>
                <div class="popupTitle"
                     tal:attributes="id string:patch-popup-${repeat/patch_task/index};">
@@ -82,9 +82,8 @@
                     <strong>From:</strong>
                     <a tal:replace="structure submitter/fmt:link"></a><br/>
                     <strong>Link:</strong>
-                    <a tal:attributes="href python:view.proxiedUrlForLibraryFile(patch)"
-                       tal:content="patch/libraryfile/filename"></a></p>
-                 <p tal:content="string:${patch/title}"></p>
+                    <a tal:replace="structure patch/fmt:attachment_link" />
+                 </p>
                </div>
                <script type="text/javascript" tal:content="string:
                  LPJS.use('base', 'node', 'event', function(Y) {
diff --git a/lib/lp/bugs/tests/bugs-emailinterface.rst b/lib/lp/bugs/tests/bugs-emailinterface.rst
index 8e765d1..61b1ef7 100644
--- a/lib/lp/bugs/tests/bugs-emailinterface.rst
+++ b/lib/lp/bugs/tests/bugs-emailinterface.rst
@@ -657,7 +657,7 @@ otherwise permission to complete the operation will be denied.)
 We will also add an attachment to the bug.
 
     >>> bug_attachment = bug_four.addAttachment(
-    ...     bug_four.owner, b"Attachment", "No comment", "test.txt"
+    ...     bug_four.owner, b"Attachment", "No comment", "test.txt", url=None
     ... )
 
     >>> submit_commands(bug_four, "private yes")
diff --git a/lib/lp/scripts/tests/test_garbo.py b/lib/lp/scripts/tests/test_garbo.py
index b33159e..dbdedf5 100644
--- a/lib/lp/scripts/tests/test_garbo.py
+++ b/lib/lp/scripts/tests/test_garbo.py
@@ -1209,13 +1209,15 @@ class TestGarbo(FakeAdapterMixin, TestCaseWithFactory):
         switch_dbuser("testadmin")
         bug = self.factory.makeBug()
         attachment = self.factory.makeBugAttachment(bug=bug)
+        # Attachments with URLs are never deleted.
+        self.factory.makeBugAttachment(bug=bug, url="https://launchpad.net")
         transaction.commit()
 
         # Bug attachments that have a LibraryFileContent record are
         # not deleted.
         self.assertIsNot(attachment.libraryfile.content, None)
         self.runDaily()
-        self.assertEqual(bug.attachments.count(), 1)
+        self.assertEqual(bug.attachments.count(), 2)
 
         # But once we delete the LfC record, the attachment is deleted
         # in the next daily garbo run.
@@ -1224,7 +1226,7 @@ class TestGarbo(FakeAdapterMixin, TestCaseWithFactory):
         transaction.commit()
         self.runDaily()
         switch_dbuser("testadmin")
-        self.assertEqual(bug.attachments.count(), 0)
+        self.assertEqual(bug.attachments.count(), 1)
 
     def test_TimeLimitedTokenPruner(self):
         # Ensure there are no tokens
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index e25375f..226dc96 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -2565,6 +2565,7 @@ class LaunchpadObjectFactory(ObjectFactory):
         filename=None,
         content_type=None,
         description=None,
+        url=None,
         is_patch=_DEFAULT,
     ):
         """Create and return a new bug attachment.
@@ -2581,23 +2582,31 @@ class LaunchpadObjectFactory(ObjectFactory):
             string will be used.
         :param content_type: The MIME-type of this file.
         :param description: The description of the attachment.
+        :param url: External URL of the attachment (a string or None)
         :param is_patch: If true, this attachment is a patch.
         :return: An `IBugAttachment`.
         """
+        if url:
+            if data or filename:
+                raise ValueError(
+                    "Either `url` or `data` / `filename` can be provided."
+                )
+        else:
+            if data is None:
+                data = self.getUniqueBytes()
+            if filename is None:
+                filename = self.getUniqueString()
+
         if bug is None:
             bug = self.makeBug()
         elif isinstance(bug, (int, str)):
             bug = getUtility(IBugSet).getByNameOrID(str(bug))
         if owner is None:
             owner = self.makePerson()
-        if data is None:
-            data = self.getUniqueBytes()
         if description is None:
             description = self.getUniqueString()
         if comment is None:
             comment = self.getUniqueString()
-        if filename is None:
-            filename = self.getUniqueString()
         # If the default value of is_patch when creating a new
         # BugAttachment should ever change, we don't want to interfere
         # with that.  So, we only override it if our caller explicitly
@@ -2610,6 +2619,7 @@ class LaunchpadObjectFactory(ObjectFactory):
             data,
             comment,
             filename,
+            url,
             content_type=content_type,
             description=description,
             **other_params,