119703 - Create an attachment by pasting it into a text field

Status: RESOLVED FIXED

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

Description

[Doug Turner (:dougt)]

It would be nice if I could just paste a patch into a text field instead of
having to create a temp file.  Any chance of doing something like this??

Comment 1

[Bradley Baetz (:bbaetz)]

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

Comment 2

[Matthew Tuck [:CodeMachine]]

Anything's possible.

Why do you have to create temp files?  Wouldn't the patch usually start as a
file?

On the rare occasion I have had to create a temp file (through telnet), they
never seem to work anyway.

Comment 3

[Doug Turner (:dougt)]

Well, here is what I do....

a) modify code.
b) cvs diff -u > xx
c) click on Create a New Attachment
d) find the file in the File Picker

I was just thinking that c/d could be replaced with a copy and paste.

Comment 4

[Dave Miller [:justdave]]

Reassigning all of my "future" targetted bugs to indicate that I'm not presently
working on them, and someone else could feel free to work on them.

Comment 5

[Dave Miller [:justdave]]

Reassigning all of my "future" targetted bugs to indicate that I'm not presently
working on them, and someone else could feel free to work on them. (sorry for
the spam if you got this twice, it didn't take right the first time)

Comment 7

[Nicholas Sushkin]

Instead of going through multiple steps starting with pasting clipboard into a new temporary file, I should be able to paste clipboard contents into a text/plain attachment directly through a form.

Comment 8

[Denis Roy]

This functionality has been requested at Eclipse.  See http://bugs.eclipse.org/304229

Comment 9

[Paul Fullbright]

To follow up on the Eclipse request, another major use case is for users to attach a stack trace in a simple way.  As it is now, users often paste large multithreaded stack traces directly into a bug description, which impairs the communication involved.

Comment 11

[Frédéric Buclin]

Should be pretty trivial to do now that I refactored Attachment.pm.

Comment 12

[Frédéric Buclin]

Attachment: patch, v1

Was indeed pretty easy to implement. My patch from bug 490930 must land first (it already has r+ and a+) due to conflicts.

In a separate bug, I plan to let attachment.cgi extract URLs from plain/text files, and display them above the attachment itself, so that if you paste a URL, you still have an easy way to click on it (to reproduce the old behavior).

Note that when you paste text directly, the MIME type is automatically set to text/plain. I may relax that in a separate bug. Same for the is_patch attribute.

mkanat, could you look at pieces which are outside my ownership, especially in WS and Install/?

Comment 13

[Max Kanat-Alexander]

Comment on attachment 456452 [details] [diff] [review]
patch, v1

  Hmm, I don't fully understand why you're removing isurl as part of this patch? I'm perfectly happy to see that feature go away, but it doesn't seem like this new patch actually fully duplicates that functionality?

  In any case:

>=== modified file 'Bugzilla/Field.pm'
>--- Bugzilla/Field.pm	2010-07-05 23:40:12 +0000
>+++ Bugzilla/Field.pm	2010-07-08 14:46:46 +0000
>@@ -238,7 +238,6 @@
>      buglist => 1},
>     {name => 'content',               desc => 'Content'},
>     {name => 'attach_data.thedata',   desc => 'Attachment data'},
>-    {name => 'attachments.isurl',     desc => 'Attachment is a URL'},

  You also need to add code somewhere to delete the field from fielddefs. This field never could have been updated, right? So it will never be in bugs_activity.

>=== modified file 'Bugzilla/WebService/Bug.pm'
> C<boolean> True if the attachment is obsolete, False otherwise.
> 
>-=item C<is_url>
>-
>-C<boolean> True if the attachment is a URL instead of actual data,

  You'll also need to update History to note that this field went away in Bugzilla 4.2.

  Also, I just added add_attachment to WebServices (it's pending checkin right now) so that will have to be updated as well. It will also need a History section, because it is going in to 4.0. (And stuff will also have to be removed from Errors and WS_ERROR_CODES or whatever it's called.)

>=== modified file 'template/en/default/attachment/createformcontents.html.tmpl'
>-  <th><label for="attachurl">AttachURL</label>:</th>
>+  <th><label for="attach_text">&nbsp;</label></th>

  There's no reason to have an empty <label>.

>+    <em>Or paste the text to be added as an attachment (instead of attaching a file).</em><br>
>+    <textarea id="attach_text" name="attach_text" cols="60" rows="10"
>+              onkeyup="URLFieldHandler()" onblur="URLFieldHandler()"></textarea>

  I think that it would be a really good idea to hide this field by default, and only show it if somebody clicks on a link. (And then you could hide the data field when the attach_text field was shown.)

Comment 14

[Guy Pyrzak]

I think this is a great idea and probably one I'd use a lot personally. Some smarts might be if the only text pasted in matches the URL Regex then it assumes it is a URL? or does that have security implications, or maybe it's just dumb period...

I do agree with Mkanat that it would be nice to hide the big text box behind a hyperlink of sorts but not 100% necessary. Using TUI might be a good idea here as well. I find that the fewer form widgets in view the less complicated/scary the page appears. Especially when those fields are redundant and don't work in tandem.

Comment 15

[Frédéric Buclin]

(In reply to comment #13)
>   Hmm, I don't fully understand why you're removing isurl as part of this
> patch? I'm perfectly happy to see that feature go away, but it doesn't seem
> like this new patch actually fully duplicates that functionality?

As pyrzak said, I'm going to add a regexp (in a separate bug), and if it matches, then Bugzilla will linkify the URL, restoring the behavior of isurl.

Comment 16

[Max Kanat-Alexander]

  Okay. That sounds fine. :-) Or actually, why not just run the text through quoteUrls?

Comment 17

[Frédéric Buclin]

Because we display the file as is (coming from the alternate attachment server), not an altered one, in Details.

Comment 18

[Frédéric Buclin]

Attachment: patch, v2

I fixed everything you commented on.

Comment 19

[Max Kanat-Alexander]

(In reply to comment #17)
> Because we display the file as is (coming from the alternate attachment
> server), not an altered one, in Details.

  Ah, okay. Well, maybe there could be a &quoteUrls=1 option or something, but that would be another bug for sure.

Comment 20

[Max Kanat-Alexander]

Comment on attachment 457682 [details] [diff] [review]
patch, v2

This looks great, and is really cool! :-) On checkin, I'd like it if you could change the "text_field" class to something more specific, like "attach_text", because TUI classes are global across all of Bugzilla.

Also, it might be nice to take the "or paste text as attachment" text and make it non-italic, so that it's more noticeable. But that's something we can fix later or pyrzak can make some decision about if he wants to.

Comment 21

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified attachment.cgi
modified bugzilla.dtd
modified post_bug.cgi
modified Bugzilla/Attachment.pm
modified Bugzilla/Field.pm
modified Bugzilla/Config/Attachment.pm
modified Bugzilla/DB/Schema.pm
modified Bugzilla/Install/DB.pm
modified Bugzilla/WebService/Bug.pm
modified Bugzilla/WebService/Constants.pm
modified docs/en/xml/using.xml
modified js/attachment.js
modified template/en/default/admin/params/attachment.html.tmpl
modified template/en/default/attachment/create.html.tmpl
modified template/en/default/attachment/createformcontents.html.tmpl
modified template/en/default/attachment/edit.html.tmpl
modified template/en/default/attachment/list.html.tmpl
modified template/en/default/bug/show.xml.tmpl
modified template/en/default/bug/create/create.html.tmpl
modified template/en/default/global/code-error.html.tmpl
modified template/en/default/global/field-descs.none.tmpl
modified template/en/default/global/user-error.html.tmpl
modified xt/lib/Bugzilla/Test/Search/Constants.pm
Committed revision 7383.

Comment 24

[David Lawrence [:dkl]]

jhammel. I am going to work on backporting this to bmo/4.0 so we can have it sooner than 4.2.

Dave

Comment 25

[Max Kanat-Alexander]

(In reply to comment #24)
> jhammel. I am going to work on backporting this to bmo/4.0 so we can have it
> sooner than 4.2.

  For what it's worth, I wouldn't recommend backporting much of anything from 4.2 at this stage--it has never even had a development release and seems to be rather unstable.

Comment 26

[Frédéric Buclin]

(In reply to comment #25)
>   For what it's worth, I wouldn't recommend backporting much of anything from
> 4.2 at this stage--it has never even had a development release and seems to be
> rather unstable.

I agree with Max here. This is a major change to the code which probably depends on some other changes. A good way to break everything. I wouldn't recommend to backport this.

Comment 27

[David Lawrence [:dkl]]

So even if I only pull out the part that displays a textarea for pasting text and using that instead of any uploaded data or url, that will be dependent on various other changes and not be worth the trouble?

Comment 28

[David Lawrence [:dkl]]

Will look into this in more detail once 4.0 is in production.

Comment 29

[Max Kanat-Alexander]

(In reply to comment #27)
> So even if I only pull out the part that displays a textarea for pasting text
> and using that instead of any uploaded data or url, that will be dependent on
> various other changes and not be worth the trouble?

  Right. There is no "only the part that displays a textarea for pasting text", there's a lot of prerequisite work and a huge patch here that should not be backported.

Comment 31

[Frédéric Buclin]

Added to relnotes in bug 713346.

Comment 32

[Ed Morley [:emorley]]

Frederic, for the rest of bugs on b.m.o regressions are usually marked as blocking the originating bug, not vice versa. Is this not the case with the Bugzilla product?

Comment 33

[Frédéric Buclin]

(In reply to Ed Morley (Away until 13th March) [:edmorley UTC+0] from comment #32)
> Frederic, for the rest of bugs on b.m.o regressions are usually marked as
> blocking the originating bug, not vice versa. Is this not the case with the
> Bugzilla product?

Correct. For us, a regression depends on the original implementation.

Comment 36

[Frédéric Buclin]

Sam: please stop playing in this bug!! This is not the right place for it.

Comment 38

[maggus.staab]

I experience this issue also on https://bugzilla.mozilla.org

would be great if I could "paste from clipboard" my screenshot I just made with my favorite screenshot tool, without the need to save it as a file in between.

as this ticket is marked as "fixed" I am wondering how this feature works on https://bugzilla.mozilla.org