Closed Bug 422691 Opened 17 years ago Closed 17 years ago

Attachment gets added twice after hitting "Back" and "Refresh"

Categories

(Bugzilla :: Attachments & Requests, defect)

3.1.3
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Bugzilla 3.2

People

(Reporter: wescottnimbus7, Assigned: dkl)

References

Details

Attachments

(1 file, 2 obsolete files)

User-Agent: Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 5.1; .NET CLR 1.1.4322; .NET CLR 2.0.50727) Build Identifier: 1.Submitted a bug report. 2. Went back into the bug and clicked on the "Add Attachment" button. 3. Added the attachment and gave it a description. Then I added some optional comments on the last field below. 4. clicked the Submit button. 5. Then I clicked on the "Details" link next to the attachment. 6. Then I clicked the Browser's "Back" button. (I receive an error at this point). 7. Then I click on the Browser's "Refresh" button. Results: The attachment was added again. Reproducible: Always Steps to Reproduce: 1.Submitted a bug report. 2. Went back into the bug and clicked on the "Add Attachment" button. 3. Added the attachment and gave it a description. Then I added some optional comments on the last field below. 4. clicked the Submit button. 5. Then I clicked on the "Details" link next to the attachment. 6. Then I clicked the Browser's "Back" button. (I receive an error at this point). 7. Then I click on the Browser's "Refresh" button. Results: The attachment was added again. Actual Results: The attachment gets added twice Expected Results: When I hit the back button it should return me to the bug report without another attachment being added. Any Refresh action should not cause the attachment to be re-added to the bug report.
This is because adding an attachment immediately brings you to show_bug.cgi. And if you click "Details" or view the attachment and then want to come back into the bug, attachment.cgi is called again to resubmit the attachment, which is a problem. Maybe do we need an intermediate page which immediately redirects the user to show_bug.cgi to prevent this.
Status: UNCONFIRMED → NEW
Component: Creating/Changing Bugs → Attachments & Requests
Depends on: 398428
Ever confirmed: true
Flags: blocking3.2+
OS: Windows XP → All
Hardware: PC → All
Target Milestone: --- → Bugzilla 3.2
Version: unspecified → 3.1.3
Created a patch to throw an intermediate message screen when someone hits refresh/reload after previous submitting an attachment. The screen will allow them to go to the page for entering a new attachment, to the previously submitted attachment, or back to the bug itself. I basically took parts of the same code that protects against duplicate bug form submissions and reworked it for attachments. Please take a look and review. Thanks Dave
Assignee: create-and-change → dkl
Status: NEW → ASSIGNED
Attachment #325637 - Flags: review?
Attachment #325637 - Flags: review? → review?(mkanat)
Comment on attachment 325637 [details] [diff] [review] Patch to cancel duplicate attachments when reloading form (v1) Attachment stuff is LpSolit's.
Attachment #325637 - Flags: review?(mkanat) → review?(LpSolit)
Simply clicking the Back button doesn't resubmit the attachment (I thought it did), so that's not a big enough issue to be considered as a blocker.
Flags: blocking3.2+ → blocking3.2-
Comment on attachment 325637 [details] [diff] [review] Patch to cancel duplicate attachments when reloading form (v1) This looks good to me. Just a few things to fix. >Index: attachment.cgi >+ # Detect if the user already used the same form to submit a bug s/bug/attachment/. >+ unless ($creator_id >+ && ($creator_id == $user->id) >+ && ($old_attach_id =~ "^createattachment:")) { Nit: put { on its own line as the test is several lines long. >Index: template/en/default/attachment/cancel-create-dupe.html.tmpl >+ # bugid: integer. ID of the bug previously used to create a bug. This must be updated. We are talking about attachments now. Also, you forgot to describe attachid. >+ # allow_override: boolean int. Is 1 if the user may submit the bug again. This parameter is no longer relevant for attachments. >+ You already used the form to file attachment >+ <a href="[% urlbase FILTER html %]attachment.cgi?id=[% attachid FILTER url_quote %]&action=edit">[% attachid FILTER url_quote %]</a>. Nit: enclose "attachment" in the link (as we do for bugs). This makes the link longer and easier to click. >+ If you would like to submit a new attachment, click here to >+ <a href="[% urlbase FILTER html %]attachment.cgi?bugid=[% bugid FILTER url_quote %]&action=enter">add a new attachment</a>. >+<p> >+<p> >+ Return to <a href="[% urlbase FILTER html %]show_bug.cgi?id=[% bugid FILTER url_quote %]">[% terms.bug %]&nbsp;[% bugid FILTER html %]</a>. Nit: couldn't we mix both sentences in a single one, without repeating "add a new attachment"? For instance, you can either <a>add a new attachment</a> or <a>go back to bug XXX</a>.
Attachment #325637 - Flags: review?(LpSolit) → review-
Thanks for the review. Here is an updated patch with your suggested changes. Please review. Thanks Dave
Attachment #325637 - Attachment is obsolete: true
Attachment #326948 - Flags: review?(LpSolit)
Comment on attachment 326948 [details] [diff] [review] Patch to cancel duplicate attachments when reloading form (v2) >Index: template/en/default/attachment/cancel-create-dupe.html.tmpl >+ # Contributor(s): Olav Vitters <olav@bkor.dhs.org> Nit: you should add your name to the list. >+ create a new attachment</a> or <a href="[% urlbase FILTER html %]show_bug.cgi?id=[% bugid FILTER url_quote %]"> >+ go back to [% terms.bug %]&nbps;[% bugid FILTER html %]</a>. Two things: 1) Remove &nbps;, first because there is a typo (should be &nbsp;), and secondly because it's useless. Write [%+ bugid FILTER html %] to force a whitespace before the bug ID. 2) Instead of 1), you could write [% "go back to $terms.bug $bugid" FILTER bug_link($bugid) FILTER none %] instead of the whole URL. My preference is to implement 2). Could you attach a new patch with 2) implemented?
Attachment #326948 - Flags: review?(LpSolit) → review+
Holding approval till the patch is updated.
Flags: approval?
New patch addressing final suggestions by LpSolit. I updated the back to bug link to use bug_link(bugid) instead. Eventually I suppose we should also have a attach_link(attachid) similar utility function but that is a different bug. Also added my name to the contributor list. Please approve Dave
Attachment #326948 - Attachment is obsolete: true
Attachment #327155 - Flags: review?(LpSolit)
Comment on attachment 327155 [details] [diff] [review] Patch to cancel duplicate attachments when reloading form (v3) r=LpSolit
Attachment #327155 - Flags: review?(LpSolit) → review+
Flags: approval?
Flags: approval3.2+
Flags: approval+
3.2: Checking in attachment.cgi; /cvsroot/mozilla/webtools/bugzilla/attachment.cgi,v <-- attachment.cgi new revision: 1.144.2.1; previous revision: 1.144 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.35.2.1; previous revision: 1.35 done RCS file: /cvsroot/mozilla/webtools/bugzilla/template/en/default/attachment/Attic/cancel-create-dupe.html.tmpl,v done Checking in template/en/default/attachment/cancel-create-dupe.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/attachment/Attic/cancel-create-dupe.html.tmpl,v <-- cancel-create-dupe.html.tmpl new revision: 1.1.2.1; previous revision: 1.1 done tip: /cvsroot/mozilla/webtools/bugzilla/attachment.cgi,v <-- attachment.cgi new revision: 1.145; previous revision: 1.144 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.36; previous revision: 1.35 done Checking in template/en/default/attachment/cancel-create-dupe.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/attachment/cancel-create-dupe.html.tmpl,v <-- cancel-create-dupe.html.tmpl new revision: 1.2; previous revision: 1.1 done
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: