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)
Tracking
()
RESOLVED
FIXED
Bugzilla 3.2
People
(Reporter: wescottnimbus7, Assigned: dkl)
References
Details
Attachments
(1 file, 2 obsolete files)
4.98 KB,
patch
|
LpSolit
:
review+
|
Details | Diff | Splinter Review |
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.
![]() |
||
Comment 1•17 years ago
|
||
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
Assignee | ||
Comment 2•17 years ago
|
||
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 | ||
Updated•17 years ago
|
Attachment #325637 -
Flags: review? → review?(mkanat)
Comment 3•17 years ago
|
||
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)
![]() |
||
Comment 4•17 years ago
|
||
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 5•17 years ago
|
||
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 %] [% 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-
Assignee | ||
Comment 6•17 years ago
|
||
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 7•17 years ago
|
||
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 ), 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+
Assignee | ||
Comment 9•17 years ago
|
||
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 10•17 years ago
|
||
Comment on attachment 327155 [details] [diff] [review]
Patch to cancel duplicate attachments when reloading form (v3)
r=LpSolit
Attachment #327155 -
Flags: review?(LpSolit) → review+
![]() |
||
Updated•17 years ago
|
Flags: approval?
Flags: approval3.2+
Flags: approval+
Assignee | ||
Comment 11•17 years ago
|
||
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.
Description
•