149504 - Permit a reference to a URL to be treated as an attachment

Status: RESOLVED FIXED

(Bugzilla :: Attachments & Requests, enhancement, P3)

Description

[Joel Peshkin]

To permit URL's to be handled as attachments, permit a URL to be specified in
place of a filename and create a redirect page and attach it instead.

Comment 1

[Joel Peshkin]

Attachment: Patch

Adds a new box to the attachment entry form for a URL.	If a URL is specified,
it is placed in a redirect page and the redirect page is attached as a
text/html page in place of an uploaded file.

This comes out nicely in the edit attachment IFRAME.

Comment 2

[Jouni Heikniemi]

Comment on attachment 86556 [details] [diff] [review]
Patch 

Some code comments, followed by philosophy.

>+<META HTTP-EQUIV=\"REFRESH\" CONTENT=\"0; URL=$::FORM{'url'}\">
>+<H1>I think you are looking for <a href=\"$::FORM{'url'}\">$::FORM{'url'}</a></H1>

Is there a reason why we couldn't just write out a HTTP 302 
statuscode and a location header?

If there is and we have to rely on meta redirects, write
the HTML in lowercase, templatize the message shown
and take care of the proper escaping of the URLs to avoid
cross-site scripting vulnerabilities.

I'm not sure I like the MIME type always being set to text/html - 
while it is of course correct for the html link document, it 
sucks big time when the user cannot really know what sort of 
material to expect. OTOH, how would Bugzilla then know the real
media type... Hmm...

I'm starting to believe that a proper implementation involves
a backend change.

>+        <em>Attach a URL instead</em><br>

Umh... This is not very clear.

How about "You can also use a URL reference as the attachment.
Note that this creates a link - the material is not copied 
into Bugzilla!" or something like that?


But before we go further with this, could you describe a bit
more precisely what do you intend to do with this - what sort
of situations need this feature? Also note bug 106778, which
is similar, but based on the concept of downloading the stuff
from the given URL and making that the attachment. This would
be different, of course.

Comment 3

[Joel Peshkin]

WRT comment 2.

I'll have to look up the statuscode 302 item if it potentially cleaner than the
redirect.  The chunk of HTML used for the redirect was lifted from
checksetup.pl.  My hope was to keep this good and simple by changing only the
attach process and using something that automatically works in all the reterival
sections, even when the reference URL points to something other than a text/html
thing.  By redirecting, the text/html redirect message points to something that
then gets to specify its own mime-type, so anything that can ever be opened in a
browser works properly. 

The reason for this is the same reason that bug 11901 is more than a cosmetic
change.   A combination of several open-source tools is close to being able to
become an auditable document review/signoff system.  

1) I have a system (being GPLd), that is being GPL's that sits on top of CVS and
provides web-based indexed document retreival and authorization checks.  That
system creates URLs to particular versions of documents kept in CVS.  These
documents are in multiple repositories (usually on the same continent as their
author) and often exceed 16MB. 

2) We have started to use Bugzilla for the review/approval cycle, currently
using comments (with a bug 11901 patch to not break the URLs) as both the
mechanism to point back to the document and the mechanism to express the
approval state of th document.  Naturally, this is the wrong place for the state
information.

3) By putting the pointer to the document itself in a redirect and including
that as the attachment, all the retreival mechanisms work well (Even word docs
opening in an IFRAME) and we do not violate the golden rule of such systems
where there is really only one "official" copy of an approved document.

4) Just adding this portion, lets us keep a record of who approved and when. 
Once the new group model is available, then it becomes possible to direct the
workflow a bit more by designating who is permitted to add particular approval
statuses to an attachment.  (On this last item, I have been considering the
question of how to sufficiently generalize the decision - something very
flexible that permits a general check resembling "Are any of the roles this user
has permitted to change this field of a bug in this state to this new value?" --
naturally doing the check both as the user is being offered options and as
changes are being processed)

 

Comment 4

[Andreas Franke (gone)]

This is a nice hack. Amazing what you can do with a few lines of code in the
right place. :-)

In the long run, it seems to me that it's better to handle this in the backend
instead of adding redirect pages as "pseudo" attachments. This is similar to the
case of "pseudo" comments of the form "*** This bug has been marked as a
duplicate of xxxxxx ***", which I think is The Wrong Thing (TM), too. 
(Of course, we've been living with them for a while, so we could live with the
"pseudo" attachments for some time (years?), too.)

In the "FeatureZilla" - bug 75172 comment #1 I have described a generalization
of the current "URL" field so that each bug can have an arbitrary (but finite)
number of associated links, where each link has a type (which is conceptually a
selection from an enum). 

Assuming this gets implemented at some point, it may be worth thinking about
whether this could be unified with the attachment tracker. From a user's point
of view, "real" attachments and urls would behave very similar: in the
attachment case, the user clicks on some linked text which the attachment
submitter has provided at upload time, and in the url case this could even be
the same if we allow each url entry to have a short text, but of course the
document would be fetched upon clicking on the link rather than retrieved from
the database. All we need to distinguish these two cases would be a flag whether
the entry is a "url" (i.e. dynamic, lazy-loading data) or a "real attachment"
(i.e. static, fixed data). Database-wise you would still want to store the
actual URL value in a different column (or even in a different table) than the
"real attachment" data, if only because attachments tend to get huge, whereas
urls are usually small; also when you run queries, you don't want to mix the
urls with attachment content. But for the user (i.e. UI-wise), this could be
just a very small difference. (Database-wise you could even use the attachment
content field to contain a cached copy of the url in some cases if someone
thinks that this makes sense at all; obviously you don't want this for most
scenarios.)

This merge of generalized URLs and real attachments would give us "statuses" for
url entries for free. (I don't know of applications yet, but I suspect this is
only because the generalized url entries are not implemented yet.)

On the other hand, this merge would allow attachments to be grouped together by
assigning types to them, so e.g. you could have a "screenshot" type, a "patch"
type, a "testcase" type, etc.

What do you think? Does this make sense?

Comment 5

[Joel Peshkin]

WRT comment 4

What I am eventually moving towards is essentially a combination of
"FeatureZilla" with document managment (where a critical issue is document
revision control).  Attchment review statuses help this out quite a bit.  Once
the new group system is in place, it may even be possible to implement a workflow.

To generalize the workflow, my options on editing the state of something could
be limited on the basis of it's current state, it's current catagorization, and
who I am.  Of course, this will be a distinct bug.

For this specific item, I will eventually clean this patch up by templatizing it
in accordance with Jouni's suggestion (unless someone else gets around to it first).

Comment 6

[Alex Vincent [:WeirdAl]]

This would be very, very helpful to our Documentation effort for mozilla.org --
we're thinking of using moz.zope.org as a base for writing draft documents, and
being able to r=/sr= URL's like this would be very beneficial.

Comment 7

[Joel Peshkin]

Attachment: patch updated for 2.17.1

Comment 8

[Joel Peshkin]

Well, this is up-to-date and I think it is safe to say it is useful.
I'm open to suggestion on how this should be implemented.

Comment 9

[Vlad Dascalu]

Bug 106778 is a possible duplicate for this.

Comment 10

[Christian Reis]

*** Bug 106778 has been marked as a duplicate of this bug. ***

Comment 11

[Dave Miller [:justdave]]

Bugzilla 2.20 feature set is now frozen as of 15 Sept 2004.  Anything flagged
enhancement that hasn't already landed is being pushed out.  If this bug is
otherwise ready to land, we'll handle it on a case-by-case basis, please set the
blocking2.20 flag to '?' if you think it qualifies.

Comment 12

[Joel Peshkin]

Attachment: Patch for 2.21

This version adds a new column, isurl, identifying URL attachments.  The attach
page still needs some javascript magic to diable the File, patch, and
content-type dialogs when a URL has been provided.

Comment 13

[Erik Stambaugh]

Attachment: now featuring javascript

This version adds some simple javascript cues (greying out the file fields when
the url field is populated)

Comment 14

[Joel Peshkin]

Attachment: Cleaned up

Comment 15

[Joel Peshkin]

Comment on attachment 197802 [details] [diff] [review]
Cleaned up

Turns out I broke the edit functionality.  New patch coming.

Comment 16

[Joel Peshkin]

Attachment: v3 - with editing fixed

Comment 17

[Vlad Dascalu]

Comment on attachment 198048 [details] [diff] [review]
v3 - with editing fixed

This is non-commitable since it would cause HTML validation regressions for
situations where & is not & and it's not passed via FILTER html, like here:

-	   <a href="attachment.cgi?id=[% attachment.id %]">[%
attachment.description FILTER html FILTER obsolete(attachment.isobsolete)
%]</a>
+	   <a href="attachment.cgi?id=[% attachment.id %][% "&action=edit" IF
attachment.isurl %]">[% attachment.description FILTER html FILTER
obsolete(attachment.isobsolete) %]</a>
	 </td>

and here:

	  | <a href="attachment.cgi?id=[% attachid %]&action=diff">Diff</a>

Comment 18

[Vlad Dascalu]

Hmm, the second instance seems to exist already... So we don't validate for HTML
4 Transitional...

Comment 19

[Joel Peshkin]

Attachment: v4 - using &

Actually, it doesn't even fail html validation the other way, but here it is.

Comment 20

[Joel Peshkin]

Attachment: v5 - unrotted

Comment 21

[Joel Peshkin]

Attachment: patch with the missing files

Comment 22

[Frédéric Buclin]

Comment on attachment 199292 [details] [diff] [review]
patch with the missing files

links in the attachment table has a "&action=edit" part which should only be
available from the "Edit" link, not from the description of the attachment
itself.

Moreover, the "Created an attachment" link in comments contains "&action=view",
as expected, but clicking on it downloads the attachment instead of redirecting
the user to the given URL.

I also suggested on IRC to use a iframe with a toggle button to either display
the URL or the page it points to. But this part could be fixed in another bug.

Comment 23

[Joel Peshkin]

Attachment: version next ... view == edit for URLs

This actually simplifies things.  If an attachment is a URL, attempts to view
it take the user to the same page as edit.  That makes them see the URL before
clicking on it.

As lpsolit and I discussed in IRC, a future enhancment may let the user elect
to have view actually redirect to the target URL and may let the user elect to
see the target in an IFRAME when editing.  But... that is a future enhancment
that needs its security implications considered carefully.

Comment 24

[Frédéric Buclin]

Comment on attachment 199387 [details] [diff] [review]
version next ... view == edit for URLs

bitrotten. Moreover, allow_attach_url should go into the new
Bugzilla/Config/Attachment.pm module.

Comment 25

[Joel Peshkin]

Attachment: Revised to unrot

Comment 26

[Frédéric Buclin]

Comment on attachment 199483 [details] [diff] [review]
Revised to unrot

>Index: template/en/default/admin/params/admin.html.tmpl

> [% param_descs = {
>+  allow_attach_url => "If this option is on, it will be possible to " _
>+                      "specify a URL when creating an attachment and " _
>+                      "treat the URL itself as if it were an attachment.",

This param should be in admin/params/attachment.html.tmpl as it is
attachment-related. Also, its corresponding entry in
Bugzilla/Config/Attachment.pm is missing, meaning that this param doesn't work
at all. Maybe you forgot to include this file in your patch?

I didn't investigate further; so either wait till I have tested it a bit (with
my comment above fixed) or resubmit a new patch which I will review this
week-end.

Comment 27

[Joel Peshkin]

Attachment: patch with the diff done correctly this time

Comment 28

[Frédéric Buclin]

Comment on attachment 199572 [details] [diff] [review]
patch with the diff done correctly this time

>Index: attachment.cgi

>+    SendSQL("SELECT mimetype, filename, isurl, thedata FROM attachments " .
>             "INNER JOIN attach_data ON id = attach_id " .
>             "WHERE attach_id = $attach_id");
>+    my ($contenttype, $filename, $isurl, $thedata) = FetchSQLData();
>+    return edit() if $isurl;

I see no reason why URLs should be treated differently. This will bring
confusion to the user. When action=view, display its content as we do actually.
As it stores the URL itself, this URL will be displayed, as expected. Change
its content-type to text/plain though, instead of application/url.


>+    my $attachurl = $cgi->param('attachurl') || '';
>+    my $data;
>+    my $filename;
>+    my $contenttype;
>+    my $isurl;
>+    if ($attachurl =~ /\S+/) {
>+        $filename = '';
>+        validateIsPatch();
>+        validateDescription();
>+        $data = $attachurl;
>+        $contenttype = SqlQuote('application/url');
>+        $isurl = 1;
>+    } else {
>+        $filename = validateFilename();
>+        validateIsPatch();
>+        validateDescription();
>+        # need to validate content type before data as
>+        # we now check the content type for image/bmp in validateData()
>+        validateContentType() unless $cgi->param('ispatch');
>+        $data = validateData();
>+        $contenttype = SqlQuote($cgi->param('contenttype'));
>+        $isurl = 0;
>+    }

Some optimization is possible here, for instance validateIsPatch() and
validateDescription() which are common to both cases. Moreover, 'attachurl'
should be validated before being added to the DB (i.e., is this a valid URL?).
One more thing: when JavaScript is disabled on my browser, I am free to turn on
the 'ispatch' bit, which is then stored in the DB. You should either completely
ignore this bit and force ispatch=0, or you accept that some URLs should be
able to be marked as patches and you have to let this checkbox accessible.


>   my $sth = $dbh->prepare("INSERT INTO attachments
>       (bug_id, creation_ts, filename, description,
>+       mimetype, ispatch, isurl, isprivate, submitter_id) 
>       VALUES ($bugid, $sql_timestamp, $sql_filename,
>               $description, $contenttype, " . $cgi->param('ispatch') . ",
>+              $isurl, $isprivate, $userid)");

Following this insert, you have an option to treat your attachment as a 'big
file'. If this bit is turned on when attaching a URL, Bugzilla crashes:

"Can't use an undefined value as a symbol reference at
/var/www/html/cvsbugzilla/attachment.cgi line 1015."


>+  if ($isurl && $datasize < 2048) {
>+      SendSQL("SELECT thedata FROM attach_data WHERE id = $attach_id");
>+      ($thedata) = FetchSQLData();
>+  }

Could you add a comment about this magic number "2048"?



>Index: checksetup.pl

>+# 2005-09-28 [e-mail] Bug 149504
>+$dbh->bz_add_column('attachments', 'isurl',
>+                    {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'FALSE'});

It should be DEFAULT => '0', see previous calls to bz_add_column().



>Index: template/en/default/attachment/edit.html.tmpl

>+        [% UNLESS isurl %]
>+          <strong>Actions:</strong>
>+          <a href="attachment.cgi?id=[% attachid %]">View</a>
>+        [% END %]
>         [% IF ispatch && patchviewerinstalled %]
>          | <a href="attachment.cgi?id=[% attachid %]&action=diff">Diff</a>
>         [% END %]

The 'diff' link should not be available when displaying a URL (remember I can
disable JavaScript to turn the 'ispatch' bit on).


>@@ -268,6 +276,18 @@
>             //-->
>           </script>
>         </td>
>+      [% ELSIF isurl AND thedata.match("^http") %]

This check should be done before inserting the URL into the DB. We should also
accept ftp:// btw.

Comment 29

[Joel Peshkin]

Attachment: patch incorporating lpsolit's suggestions

Comment 30

[Joel Peshkin]

Attachment: version++ include lpsolits comments from IRC

Comment 31

[Frédéric Buclin]

Comment on attachment 200050 [details] [diff] [review]
version++ include lpsolits comments from IRC

>Index: template/en/default/attachment/create.html.tmpl

>+[% IF Param("allow_attach_url") %]
>+    <script type="text/javascript">
>+
>+        function URLFieldHandler() {
>+            var field_attachurl = document.getElementById("attachurl");
>+            var greyfields = new Array("data", "ispatch", "autodetect",
>+                                       "list", "manual",
>+                                       "contenttypeselection",
>+                                       "contenttypeentry");
>+            if (field_attachurl.value.match(/^\s*$/)) {
>+                for (var i = 0; i < greyfields.length; i++) {
>+                    thisfield = document.getElementById(greyfields[i]);
>+                    if (thisfield) {
>+                        thisfield.removeAttribute("disabled");
>+                    }
>+                }
>+            } else {
>+                for (var i = 0; i < greyfields.length; i++) {
>+                    thisfield = document.getElementById(greyfields[i]);
>+                    if (thisfield) {
>+                        thisfield.setAttribute("disabled", "disabled");
>+                    }
>+                }
>+            }
>+        }
>+
>+        function DataFieldHandler() {
>+            var field_data = document.getElementById("data");
>+            var greyfields = new Array("attachurl");
>+            if (field_data.value.match(/^\s*$/)) {
>+                for (var i = 0; i < greyfields.length; i++) {
>+                    thisfield = document.getElementById(greyfields[i]);
>+                    if (thisfield) {
>+                        thisfield.removeAttribute("disabled");
>+                    }
>+                }
>+            } else {
>+                for (var i = 0; i < greyfields.length; i++) {
>+                    thisfield = document.getElementById(greyfields[i]);
>+                    if (thisfield) {
>+                        thisfield.setAttribute("disabled", "disabled");
>+                    }
>+                }
>+            }
>+        }
>+
>+    </script>
>+[% END %]

Nit: a large part of this code is common for both the file field and the URL
field. I think you could refactor it a bit (in another bug).

On checkin: please also disable the "big file" checkbox when entering a URL.


>+    [% IF Param("allow_attach_url") %]
>+    <tr>
>+      <th><label for="attachurl">AttachURL:</label></th>
>+      <td>
>+        <em>URL to be attached instead.</em><br>
>+        <input type="text" id="attachurl" name="attachurl" size="60" maxlength="500"
>+               onkeyup="URLFieldHandler()" onblur="URLFieldHandler()">
>+      </td>
>+    </tr>
>+    [% END %]

On checkin: 500 characters is by far too low for a standard query from
Bugzilla. ;) Please increase this value to, say, 2000 characters.



>Index: template/en/default/attachment/edit.html.tmpl

>+      [% ELSIF isurl %]
>+        <td width="75%">
>+          <a href="[% thedata FILTER html %]">
>+            [% IF datasize < 120 %]
>+              [% thedata FILTER html %]
>+            [% ELSE %]
>+              [% thedata FILTER truncate(80) FILTER html %]
>+              &nbsp;...
>+              [% thedata.match(".*(.{20})$").0 FILTER html %]
>+            [% END %]
>+          </a>
>+        </td>
>       [% ELSE %]

Nit: the display of the URL could be improve a bit. I thought about using
[% thedata FILTER html FILTER replace('&amp;', '&amp;&#x200B;') %] or <wbr> to
display the full URL on several lines, in order to fit your window. But
&#x200B; is not understood neither by Internet Explorer nor by Konqueror (I
didn't test with Opera), and <wbr> works with both Firefox and Internet
Explorer but is not in the HTML 4.01 Spec. :( If you have a better idea, open a
bug. :)


Also, for another bug: sanitycheck.cgi should do this check:

UPDATE attachments SET mimetype = 'text/plain', ispatch = 0, filename = ''
WHERE isurl = 1;


It seems to work fine now, so r=LpSolit

Comment 32

[Dave Miller [:justdave]]

I'm glad we can turn this off.

We never got any hooks in for custom attachment handling yet have we?  This is
something that would make sense to offload to a plugin if we could find a way to
make it modular.

Comment 33

[Alex Vincent [:WeirdAl]]

(In reply to comment #31)
> >+        function URLFieldHandler() {
> >+                for (var i = 0; i < greyfields.length; i++) {
...
> >+                }
> >+            } else {
> >+                for (var i = 0; i < greyfields.length; i++) {
...
> >+                }

JavaScript strict warning for redeclaring i.  Ditto for DataFieldHandler.  Could
you please fix this before you check in?

Comment 34

[Joel Peshkin]

Checking in attachment.cgi;
/cvsroot/mozilla/webtools/bugzilla/attachment.cgi,v  <--  attachment.cgi
new revision: 1.100; previous revision: 1.99
done
Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.444; previous revision: 1.443
done
Checking in Bugzilla/Attachment.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Attachment.pm,v  <--  Attachment.pm
new revision: 1.27; previous revision: 1.26
done
Checking in Bugzilla/Config/Attachment.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Config/Attachment.pm,v  <--  Attachm
ent.pm
new revision: 1.2; previous revision: 1.1
done
Checking in Bugzilla/DB/Schema.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Schema.pm,v  <--  Schema.pm
new revision: 1.40; previous revision: 1.39
done
Checking in template/en/default/admin/params/attachment.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/params/attachment.h
tml.tmpl,v  <--  attachment.html.tmpl
new revision: 1.2; previous revision: 1.1
done
Checking in template/en/default/attachment/create.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/attachment/create.html.tm
pl,v  <--  create.html.tmpl
new revision: 1.21; previous revision: 1.20
done
Checking in template/en/default/attachment/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/attachment/edit.html.tmpl
,v  <--  edit.html.tmpl
new revision: 1.28; previous revision: 1.27
done
Checking in template/en/default/attachment/list.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/attachment/list.html.tmpl
,v  <--  list.html.tmpl
new revision: 1.25; previous revision: 1.24
done

Comment 35

[Boris Zbarsky [:bzbarsky]]

So after this fix attachments can effectively go away (if the URI is what was
"attached"), right?  And can also change silently with no notification in the bug?

Comment 36

[Alex Vincent [:WeirdAl]]

My main reason for liking this idea is that attachments are currently
unchangeable.  But I do see bz's point about losing an attachment's history.

I'm willing to work on a Bugzilla extension to store Bugzilla attachments in a
CVS or SVN repository.  This would allow us to track an attachment's history,
replacing review+/- flags when an attachment is updated, and still keep the
history of the patch.

Thoughts?  If I am to do this, I'd best do it in a new bug.

Comment 37

[Joel Peshkin]

(In reply to comment #36)
> My main reason for liking this idea is that attachments are currently
> unchangeable.  But I do see bz's point about losing an attachment's history.
> 
> I'm willing to work on a Bugzilla extension to store Bugzilla attachments in a
> CVS or SVN repository.  This would allow us to track an attachment's history,
> replacing review+/- flags when an attachment is updated, and still keep the
> history of the patch.
> 
> Thoughts?  If I am to do this, I'd best do it in a new bug.

Absolutely.  But open a new bug (perhaps a new project that just happens to have
a nice integration module for Bugzilla)

The reason this exists is so it can point to a document that is
version-controlled using a (stopgap) front-end to CVS.   I would love to see
someone create a proper document system, probaly SVN-based.  Ideally, make it do
WebDAV well.

Comment 38

[Christopher Cordahi]

Is clicking on a URL attachment supposed to display the attachment or the URL?

This patch seems to be included in 2.22rc1 but only the URL (non-clickable) is displayed when clicked not the contents.  Clicking the [edit] link displays a page where there is a clickable version of the URL which when clicked displays the desired contents.  This seems to be long, complicated and non-intuitive.

Comment 39

[Frédéric Buclin]

(In reply to comment #35)
> So after this fix attachments can effectively go away (if the URI is what was
> "attached"), right?  And can also change silently with no notification in the bug?
> 

Right!


(In reply to comment #38)
> Is clicking on a URL attachment supposed to display the attachment or the URL?

The URL. It's is so by design, see comment 23. See also the request made in bug 345504. Add further comments there.

Comment 40

[Dave Miller [:justdave]]

don't see anything in the docs for this.

Comment 41

[victory <[e-mail]>]

Attachment: docs patch for tip

Comment 42

[Frédéric Buclin]

Comment on attachment 248059 [details] [diff] [review]
docs patch for tip

>-          You can attach files (e.g. testcases or patches) to bugs. If there
>-          are any attachments, they are listed in this section.  Attachments are
>-          normally stored in the Bugzilla database, unless they are marked as
>-          Big Files, which are stored directly on disk and (unlike attachments
>-          kept in the database) may be deleted at some future time.
>+          You can attach files (e.g. testcases or patches) to bugs. If
>+          there are any attachments, they are listed in this section.
>+          Attachments are normally stored in the Bugzilla database,
>+          unless they are marked as Big Files, which are stored directly
>+          on disk and (unlike attachments kept in the database) may be
>+          deleted at some future time, nor they are the URL as

Please leave these lines alone. You did nothing more than wrapping these lines differently, which breaks history without any good reason.

Moreover, the comment in parens is wrong. You can delete attachments since Bugzilla 3.0, see bug 44595. We can fix this comment separately, though.


>+          attachments, in this case only string of the URL of the
>+          attachments to refer are stored.

This sentence isn't clear at all, at least for me. I would prefer a separate sentence instead of making existing ones longer and longer.


>-      If you have a really large attachment, something that does not need to
>-      be recorded forever (as most attachments are), you can mark your
>-      attachment as a <quote>Big File</quote>, assuming the administrator of the
>-      installation has enabled this feature.  Big Files are stored directly on
>-      disk instead of in the database, and can be deleted when it is no longer
>-      needed.  The maximum size of a <quote>Big File</quote> is normally larger
>-      than the maximum size of a regular attachment.
>+      If you have a really large attachment, something that does not
>+      need to be recorded forever (as most attachments are), you can
>+      mark your attachment as a <quote>Big File</quote> or
>+      <quote>AttachURL</quote>, assuming the administrator of the
>+      installation has enabled this feature.

Same comment here: do not break history without any good reason. Wrapping is not a valid one.

Moreover, "marking an attachment as AttachURL" is meaningless.


>+      Attachments which are marked as AttachURL are stored as strings
>+      of the URL of the attachment.

Same comment here. This sentence is unclear.

Comment 43

[victory <[e-mail]>]

To native English speakers:
i don't update docs patch for this bug.
feel free to take this docs.

Comment 44

[Frédéric Buclin]

Attachment: doc patch, v1

The first hunk is for 3.0 only. In 2.22, it must be omitted.

Comment 45

[Colin Ogilvie [:cso]]

Comment on attachment 254741 [details] [diff] [review]
doc patch, v1

>Index: docs/xml/using.xml
>+      uploading the attachment itself. This is useful e.g. if you want to point

For example, this is useful if you want to point

r=me with this nit fixed.

Comment 46

[Frédéric Buclin]

OK, nit fixed on checkin.

tip:

Checking in docs/xml/using.xml;
/cvsroot/mozilla/webtools/bugzilla/docs/xml/using.xml,v  <--  using.xml
new revision: 1.62; previous revision: 1.61
done

2.22.2:

Checking in docs/xml/using.xml;
/cvsroot/mozilla/webtools/bugzilla/docs/xml/using.xml,v  <--  using.xml
new revision: 1.37.2.17; previous revision: 1.37.2.16
done