Closed Bug 5179 Opened 25 years ago Closed 18 years ago

Need to be able to put attachment on new bug.

Categories

(Bugzilla :: Creating/Changing Bugs, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
Bugzilla 3.0

People

(Reporter: shaver, Assigned: Wurblzap)

References

(Blocks 1 open bug)

Details

(Keywords: selenium, Whiteboard: [content:attachments] enter_bug)

Attachments

(1 file, 2 obsolete files)

I'd like to be able to upload a stack trace or test file when I file a new bug, not just later.
Reassigning to dmose@mozilla.org, who now has front-line responsibility for all Bonsai and Bugzilla bugs.
[CC:ing self, so that the bug writing guidelines can be promptly updated if this alteration is made.]
Reassigning back to me. That stuff about me no longer being the front-line person responsible for Bugzilla and Bonsai turned out to be short-lived. Please pardon our confusion, and I'm very sorry about the spam.
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
The problem here is (mostly) one of UI. I don't want to cram the full "specify an attachment" form into the main "submit a bug" UI. Submitting a bug should be as painless and easy as we can make it, so that very casual bugzilla users will not be scared away from the idea of submitting bugs. So, if I don't cram all this stuff there, then the best I can do is put a checkbox that says "I have a file I'd like to attach", and then get the details later. This works out to almost the same as the current behavior, where you can create a bug, then click on the link to bring up the bug, then click on the "create attachment" link. The checkbox is slightly better, in that it would have one less click, and two less page loads. And is more obvious. It turns out that implementing the checkbox is a bit hard, but I've done the easy thing for now. Which is to put a link up immediately after creating a bug that says "attach a file to this new bug". This works out to the same number of clicks as the checkbox, but one more page load. And is a bit less obvious. Good enough?
Status: RESOLVED → REOPENED
Personally I think it's still not wonderful, in that it separates out a single conceptual operation ("here is a patch, and here is my long comment on what it is") into two actions, and results in everybody getting two pieces of Bugzilla email rather than one. A simple improvement would be to add a text box to the 'add attachment' page that allowed you to alter the long description field simultaneously with adding the attachment. This improves the UI for adding things to already-open bugs, although the problem remains for creating a bug report with an attachment. This should be easy to implement: just change createattachment.cgi to: (1) put a suitable text box in the HTML it outputs (2) use the contents of that text box in the call to AppendComment() where the short description is currently used. I might submit a patch if I feel sufficiently bored :-)
Resolution: FIXED → ---
Status: REOPENED → ASSIGNED
On a related note - it would be nice to be able to add several attachments in a single commit. I believe the best solution (from the UI perspective) would be to add a "Preview" button along with the "Commit" button. The "Preview" button would allow a person to review the changes, cancel them, modify them (by pressing the "Back" button on the browser) and commit them as well as make some additional changes (such as attaching a file) before commiting.
It would probably be best to file the multiple attach issue as a separate bug report.
QA Contact: matty
See the super dooper new bug #31144 for a proposal which would allow this and more.
tara@tequilarista.org is the new owner of Bugzilla and Bonsai. (For details, see my posting in netscape.public.mozilla.webtools, news://news.mozilla.org/38F5D90D.F40E8C1A%40geocast.com .)
Assignee: terry → tara
Status: ASSIGNED → NEW
*** Bug 43161 has been marked as a duplicate of this bug. ***
One of the users on our internal bugzilla system was wanting to add a comment and an attachment at the same time. At present when submitting an attachment, you need to do two bug updates, one to give the comments describing what the attachment is, and another to actually add the attachment. ISTM that it would be useful either to have an "Additional Comments" field on the createattachment.cgi, or to be able to add attachment fields onto the standard output from bug_form.pl. The latter could work in a similar manner to the "And" and "Or" buttons on the query page.
2.18 watch!
Target Milestone: --- → Bugzilla 2.18
It's also been noted that the current method sends out two emails when you add a patch and comment on it. If it was all done as the same action it could generate a single email.
Severity: normal → enhancement
-> Bugzilla product, Creating Bugs component, reassigning.
Assignee: tara → myk
Component: Bugzilla → Creating/Changing Bugs
Product: Webtools → Bugzilla
Version: other → unspecified
Whiteboard: [content:attachments] enter_bug
*** Bug 127274 has been marked as a duplicate of this bug. ***
*** Bug 140863 has been marked as a duplicate of this bug. ***
*** Bug 142519 has been marked as a duplicate of this bug. ***
*** Bug 157205 has been marked as a duplicate of this bug. ***
The add attachment stuff has been updated to allow comments to be added at the same time an attachment is made, is still doesn't allow attachments to be made to new bugs, but it does at least cover what was mentioned in comment #5 (and also #11).
I see a problem with the implementation of this feature. I've modified my local bugzilla version so that i can add an attachment on new bugs. You are running into problems with that because the form must be posted with enctype="multipart/form-data" and it seems like the Bugzilla CGI.pl cant extract the FORM header correctly if posted as enctype="multipart/form-data" and any field contains some special characters like „a string“. I detail some Form Field values are not extracted if enctype="multipart/form- data" and any textfield contains special characters. I am not absolutly sure about that but i didnt do much modifications to the original enter_bug.cgi and post_bug.cgi and i am dont getting that undefined Form Field Value errors if enctype != "multipart/form-data". If anybody has an idea how one could prevent that error. His thoughts would be highly appreciated by me. Maybe anyone could reproduce that problem? Maybe that error wont happen if the standard CGI Module would be used within Bugzilla?
Bugzilla does use the standard CGI module, as of about a month or two ago (it was prior to 2.17.1 being posted I believe).
comment #20 has been dealt with by using Perl's CGI module to handle the form data...
*** Bug 202423 has been marked as a duplicate of this bug. ***
OS: Linux → All
Hardware: PC → All
*** Bug 232645 has been marked as a duplicate of this bug. ***
Enhancements which don't currently have patches on them which are targetted at 2.18 are being retargetted to 2.20 because we're about to freeze for 2.18. Consideration will be taken for moving items back to 2.18 on a case-by-case basis (but is unlikely for enhancements)
Target Milestone: Bugzilla 2.18 → Bugzilla 2.20
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.
Target Milestone: Bugzilla 2.20 → Bugzilla 2.22
In the modern world it might be possible to pull this off without cluttering the UI by putting the fields in the form and hide them via css. Have the checkbox for "I'd like to include an attached file" which unhides the relevant fields when checked.
*** Bug 279901 has been marked as a duplicate of this bug. ***
Blocks: 315860
Target Milestone: Bugzilla 2.22 → ---
Attached patch Patch (obsolete) (deleted) — Splinter Review
This works basically. Perhaps somebody can take a look at it? There is a known deficiency: attachment creation failures during bug creation aren't handled very well (read: Throw*Error hides successful bug creation, which is not good). They should be caught silently, so that we can display a message along the lines of "bug creation succeeded, but attachment creation failed. Please go to the bug and try attaching again".
Assignee: myk → wurblzap
Status: NEW → ASSIGNED
Attachment #218418 - Flags: review?
Oh yeah, and flags don't work either.
Comment on attachment 218418 [details] [diff] [review] Patch Let's put it in my radar.
Attachment #218418 - Flags: review?(LpSolit)
I like your patch, Marc (I haven't tested it yet, nor reviewed it in great details though). Just two minor questions: 1) why keeping some validation routines in attachment.cgi? Looks like you only moved into Attachment.pm those you need from post_bug.cgi, but maybe it would appear more logical to have them all in the same module (Attachment.pm). 2) I'm not a big fan using CGI data from Attachment.pm. I would have prefer collecting all required data in attachment.cgi and passing a hash to Attachment.pm instead. Did you think about this alternative? Anyway, both are not critical and could be fixed in separate bugs, if desired. I will try to review this patch tomorrow evening, when I'm back home.
Target Milestone: --- → Bugzilla 2.24
Attached patch Patch 1.2 (obsolete) (deleted) — Splinter Review
- Better error handling - Friendlier to non-CSS browsers - Friendlier to non-JS browsers (In reply to comment #32) > 1) why keeping some validation routines in attachment.cgi? Looks like you only > moved into Attachment.pm those you need from post_bug.cgi, but maybe it would > appear more logical to have them all in the same module (Attachment.pm). Maybe... Let's decide when we know how the routines need to be modified. If we're clear about the structure they need to take, then perhaps we can move the yet-missing ones along, too. > 2) I'm not a big fan using CGI data from Attachment.pm. I would have prefer > collecting all required data in attachment.cgi and passing a hash to > Attachment.pm instead. Did you think about this alternative? I agree this would be cleaner. Two things come to mind, though. It would require code duplication accross enter_bug.cgi and attachment.cgi. And I don't know whether it's a good idea to put the uploaded file into a hash. Maybe we can find solutions for these.
Attachment #218418 - Attachment is obsolete: true
Attachment #221991 - Flags: review?(LpSolit)
Attachment #218418 - Flags: review?(LpSolit)
Attachment #218418 - Flags: review?
You know what? I don't like this attachment_errorhandler() function and the way these validate_foo() work. IMO, you should still use ThrowUserError() in them. Maybe something like: Bugzilla::Attachment->validate_foo({no_error => 1}); and in validate_foo(): $vars->{'no_error'} ? return 0 : ThrowUserError(); One reason is that &$errorhandler("missing_content_type") is really designed for ThrowUserError(). Another reason is that 012throwables.t will be unable to find these error tags and will fail. And I strongly disagree to fix 012throwables.t in any way to match your changes.
Ok, your idea is more straightforward and not as muddled as mine. I'll upload a new patch.
(In reply to comment #35) > Ok, your idea is more straightforward and not as muddled as mine. I'll upload a > new patch. > Oh, and even easier: you could use the THROW_ERROR constant, see Constants.pm: Bugzilla::Attachment->validate_foo(THROW_ERROR); Bugzilla::Attachment->validate_foo(); and validate_foo() would get this constant in: $throw_error = shift;
Comment on attachment 221991 [details] [diff] [review] Patch 1.2 Per my previous comments. I will wait for an updated patch.
Attachment #221991 - Flags: review?(LpSolit) → review-
Attached patch Patch 1.3 (deleted) — Splinter Review
Addressed comments.
Attachment #221991 - Attachment is obsolete: true
Attachment #225282 - Flags: review?(LpSolit)
Comment on attachment 225282 [details] [diff] [review] Patch 1.3 >Index: Bugzilla/Attachment.pm >+=item C<insert_attachment_for_bug($bug_id)> >+ >+sub insert_attachment_for_bug { >+ my ($class, $throw_error, $bug_id, $user, $timestamp, $hr_vars) = @_; You must write: C<insert_attachment_for_bug($throw_error, $bug_id, $user, $timestamp, $hr_vars)> as you have to enumerate all arguments. >+ else { >+ $filename = _validate_filename($throw_error) || return 0; >+ # need to validate content type before data as >+ # we now check the content type for image/bmp in validateData() Nit: s/validateData()/_validate_data/ >Index: template/en/default/attachment/createformcontents.html.tmpl >+ [% IF flag_types.size > 0 %] >+ [% PROCESS "flag/list.html.tmpl" bug_id=bugid attach_id=attachid %]<br> >+ [% END %] Write [% IF flag_types && flag_types.size > 0 %], else you are filling your apache error log: enter_bug.cgi: Argument "" isn't numeric in numeric gt (>) at data/template/template/en/default/attachment/createformcontents.html.tmpl line 40. That's because you cannot use flags in enter_bug.cgi and so flag_types is undefined. You can easily fix all these comments on checkin. Your patch is working fine; I saw no regressions. r=LpSolit
Attachment #225282 - Flags: review?(LpSolit) → review+
Flags: approval?
We might want to come up with a way to make the "messages" type messages show up a little louder when they indicate a "partial success, partial failure" condition (there's other places besides this bug where that's an issue, too, like failed review matches). But that's another bug, and this looks awesome otherwise. :)
Flags: approval? → approval+
Committing this patch now because it conflicts with bug 283926. Checking in attachment.cgi; /cvsroot/mozilla/webtools/bugzilla/attachment.cgi,v <-- attachment.cgi new revision: 1.111; previous revision: 1.110 done Checking in post_bug.cgi; /cvsroot/mozilla/webtools/bugzilla/post_bug.cgi,v <-- post_bug.cgi new revision: 1.147; previous revision: 1.146 done Checking in Bugzilla/Attachment.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Attachment.pm,v <-- Attachment.pm new revision: 1.33; previous revision: 1.32 done RCS file: /cvsroot/mozilla/webtools/bugzilla/js/attachment.js,v done Checking in js/attachment.js; /cvsroot/mozilla/webtools/bugzilla/js/attachment.js,v <-- attachment.js initial revision: 1.1 done RCS file: /cvsroot/mozilla/webtools/bugzilla/skins/standard/create_attachment.css,v done Checking in skins/standard/create_attachment.css; /cvsroot/mozilla/webtools/bugzilla/skins/standard/create_attachment.css,v <-- create_attachment.css initial revision: 1.1 done Checking in template/en/default/attachment/create.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/attachment/create.html.tmpl,v <-- create.html.tmpl new revision: 1.27; previous revision: 1.26 done RCS file: /cvsroot/mozilla/webtools/bugzilla/template/en/default/attachment/createformcontents.html.tmpl,v done Checking in template/en/default/attachment/createformcontents.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/attachment/createformcontents.html.tmpl,v <-- createformcontents.html.tmpl initial revision: 1.1 done Checking in template/en/default/bug/create/create.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/create/create.html.tmpl,v <-- create.html.tmpl new revision: 1.60; previous revision: 1.59 done Checking in template/en/default/global/messages.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/messages.html.tmpl,v <-- messages.html.tmpl new revision: 1.35; previous revision: 1.34 done
Status: ASSIGNED → RESOLVED
Closed: 25 years ago18 years ago
Keywords: relnote
QA Contact: mattyt-bugzilla → default-qa
Resolution: --- → FIXED
Blocks: 341932
Blocks: 341934
Blocks: 302684
Added to relnotes as part of bug 349423.
Keywords: relnote
Flags: testcase?
Flags: documentation?
Documentation updated as part of the doc patch from bug 174039.
Flags: documentation? → documentation+
Blocks: 367077
Flags: testcase? → testcase+
Keywords: selenium
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: