Doubleclick creates 2 tickets

[anonymous]

When you doublelick on a submit button, you get 2 tickets.

Is this a bug or, maybe, a feature?

[Remy Blank]

Certainly not a feature, but there's a logical explanation. When you double-click on the submit button, your browser starts a POST request, aborts it, then starts another one. The server doesn't necessarily see the abort until too late (typically, when it tries to send the reply back), so it processes both POST requests completely, and hence creates two tickets.

Maybe we could use some JavaScript trickery to work around the issue, like disabling the submit button after the first click.

[Christian Boos]

I'd imagine the browsers should try to avoid such problems with submit buttons in the first place… but OK, we could help with some javascript.

[Carsten Klein <carsten.klein@…>]

Considering that with each form, a hidden token is being set back to the server, I wonder why the token was not expired by the first POST hitting the server and why the second time the same token hits the server, that token is still being accepted?

[Remy Blank]

Replying to Carsten Klein <carsten.klein@…>:

Considering that with each form, a hidden token is being set back to the server, I wonder why the token was not expired by the first POST hitting the server and why the second time the same token hits the server, that token is still being accepted?

The token, which is used to avoid CSRF, isn't stored server-side, so there's nothing to expire. It's stored as a cookie, and in every form, and both are compared on submit.

[contato@…]

I'm implementing Trac 0.12.3 at the company I work. I'm new to the system and yet, I'm teaching the developers and analysts to use it. One of the analysts double-clicks this freaking submit button every single time, creating duplicated tickets.

So, I've managed to create a small hack to solve this while there is no official "bug-fix". Keep in mind that my version is 0.12.3.

From the Trac source-code copy the file trac/ticket/templates/ticket.html (​http://trac.edgewall.org/browser/trunk/trac/ticket/templates/ticket.html) to your trac_env/templates

Now, open the ticket.html that you just copied to the trac_env/templates

Go to line 376 ​http://trac.edgewall.org/browser/trunk/trac/ticket/templates/ticket.html#L376

And, inside the input (submit) tag, write this

onclick="javascript: this.disabled='disabled'"

So, your line will look like this

<input type="submit" name="submit" value="${ticket.exists and _('Submit changes') or _('Create ticket')}" onclick="javascript: this.disabled='disabled'" />

This nobish hack is working for me and probably will work for everybody.

[contato@…]

Replying to contato@…:

I'm implementing Trac 0.12.3 at the company I work. I'm new to the system and yet, I'm teaching the developers and analysts to use it. One of the analysts double-clicks this freaking submit button every single time, creating duplicated tickets.

So, I've managed to create a small hack to solve this while there is no official "bug-fix". Keep in mind that my version is 0.12.3.

From the Trac source-code copy the file trac/ticket/templates/ticket.html (​http://trac.edgewall.org/browser/trunk/trac/ticket/templates/ticket.html) to your trac_env/templates

Now, open the ticket.html that you just copied to the trac_env/templates

Go to line 376 ​http://trac.edgewall.org/browser/trunk/trac/ticket/templates/ticket.html#L376

And, inside the input (submit) tag, write this

onclick="javascript: this.disabled='disabled'"

So, your line will look like this

<input type="submit" name="submit" value="${ticket.exists and _('Submit changes') or _('Create ticket')}" onclick="javascript: this.disabled='disabled'" />

This nobish hack is working for me and probably will work for everybody.

Fogrt this hack. The web developer biggest enemy, the doomed Internet Explorer, doesnt work with this hack…

[contato@…]

A NEW HACK HAS APPEARED

Again, keep in mind I'm working with Trac 0.12.3

The first step is the same, copy the ticket.html from trac/ticket/templates/ticket.html to your trac_env/template folder.

Go to line 398 ​http://trac.edgewall.org/browser/tags/trac-0.12.3/trac/ticket/templates/ticket.html#L398 and give an HTML ID to the input tag. Mine I called "freaking_submit_button", as you can see:

<input type="submit" name="submit" value="${ticket.exists and _('Submit changes') or _('Create ticket')}" id='freaking_submit_button' />

Now, you just have to make the form disable the button when it's submitted.

The tag FORM which creates the ticket form is located at line 190 ​http://trac.edgewall.org/browser/tags/trac-0.12.3/trac/ticket/templates/ticket.html#L190

Just create a onsubmit event to disable the button

onsubmit="document.getElementById('freaking_submit_button').disabled = 1;"

So, your FORM tag may look like this

<form py:if="has_property_editor" method="post" id="propertyform"
    action="${ticket.exists and href.ticket(ticket.id) + '#trac-add-comment' or href.newticket()}"
    onsubmit="document.getElementById('freaking_submit_button').disabled = 1;">

And this is it. Works on Iexplorer 9, Fire(awesome)Fox 12, the browsers people are using at my job.

I hope this one works fine for everybody.

[Jun Omae]

Simple patch to avoid the issue.

trac/htdocs/js/trac.js

@@ -131 +131 @@
   }
 
 })(jQuery);
+
+// Disable the submit buttons on post each form
+jQuery(document).ready(function($) {
+    $('form[method=post]').submit(function() {
+        $(':submit', this).attr('disabled', true);
+    }).find(':submit').attr('disabled', false);
+});

[Ryan J Ollos]

The issue is also reported in ​th:#10297.

The patch from comment:8 works well for the ticket form, but on /admin/ticket/milestones it is no longer possible to Add, Apply changes or Remove selected items (at least with Firefox 26 on Ubuntu). I did some reading and found a code snippet that seems to work well,

trac/templates/layout.html

@@ -48 +48 @@
         $(".trac-autofocus").focus();
         $(".trac-target-new").attr("target", "_blank");
         setTimeout(function() { $(".trac-scroll").scrollToTop() }, 1);
+        // Disable the submit buttons on post each form
+        $("form[method=post]").submit(function() {
+          $(this).submit(function() { return false; });
+        });
       });
     </script>
     ${select("*[local-name() != 'title']|text()|comment()")}

However, it sounds like this won't work for AJAX forms which rely on multiple submits between page loads. One suggested way to handle this is to add a special class to AJAX forms such as trac-multisubmit, and use a :not(trac-multisubmit) selector for the form when disabling the submit buttons.

trac/templates/layout.html

@@ -48 +48 @@
         $(".trac-autofocus").focus();
         $(".trac-target-new").attr("target", "_blank");
         setTimeout(function() { $(".trac-scroll").scrollToTop() }, 1);
+        // Disable the submit buttons on post each form
+        $("form[method=post]:not(.trac-multisubmit)").submit(function() {
+          $(this).submit(function() { return false; });
+        });
       });
     </script>
     ${select("*[local-name() != 'title']|text()|comment()")}

[Remy Blank]

Instead of a trac-multisubmit class, I would add a trac-disable-on-submit class to only those buttons where clicking isn't idempotent. For example, on the ticket page, you would set it on the "Submit changes" button but not on the "Preview" button. I expect that only relatively few buttons need to be disabled that way.

[Ryan J Ollos]

That sounds good. log:rjollos.git:t10138 uses the trac-disable-on-submit class to avoid multiple submits in the ticket form. It should handle the case in which the user clicks the back button in the browser, allowing the form to be submitted after back is clicked. I think this is desirable for our use cases, but I'll investigate further. Next I'll also test in some additional web browsers and add the trac-disable-on-submit class where necessary in Trac.

[Remy Blank]

This disables the whole form when clicking any button in the form. This is more than necessary. Would it be possible to tag only specific buttons (e.g. on the ticket page only the "Submit changes" button), so that it's still possible to double-click others (e.g. "Preview")?

comment:6 mentions that simply disabling the button doesn't work with IE, but maybe there are other solutions?

• Instead of setting the disabled attribute on the button, have its click() handler stop propagation of the event on the second click?
• Or add a click() handler on the button that sets a class on the form, and have the form submit() handler disable the form if the class is present?

[anonymous]

Possibly the ​Post/Redirect/Get pattern could be used to solve this?

[Ryan J Ollos]

Replying to rblank:

This disables the whole form when clicking any button in the form.

[be27039c/rjollos.git] aims to be more specific and only disable the submit action on buttons to which the class is applied. It is implemented for buttons on the New Ticket and Admin Milestone pages for which clicking is not idempotent. I've testing in Firefox 26 on Ubuntu 13.04 and IE 11 on Windows 7 so far.

• Instead of setting the disabled attribute on the button, have its click() handler stop propagation of the event on the second click?
• Or add a click() handler on the button that sets a class on the form, and have the form submit() handler disable the form if the class is present?

The second suggestion is what I've attempted to implement in the revised changes.

[Ryan J Ollos]

Replying to anonymous:

Possibly the ​Post/Redirect/Get pattern could be used to solve this?

I'm not sure, but that looks like quite bit of extra complexity for our case in which a duplicate submit is not all that harmful in any cases that I'm aware of.

[Remy Blank]

Replying to anonymous:

Possibly the ​Post/Redirect/Get pattern could be used to solve this?

We already do that for all mutating operations, but it's the solution to a different problem (avoiding a second POST on reload). In this case, double-clicking does the second POST before the response to the first one (the redirect) is received, so it doesn't help.

[Remy Blank]

Replying to rjollos:

[be27039c/rjollos.git] aims to be more specific and only disable the submit action on buttons to which the class is applied. It is implemented for buttons on the New Ticket and Admin Milestone pages for which clicking is not idempotent. I've testing in Firefox 26 on Ubuntu 13.04 and IE 11 on Windows 7 so far.

Looks nice.

• It would be nice to add comments about the operations performed on unload (basically, what you mention in comment:11).
• Nit: format the else as } else {.
• Technically, you wouldn't need to protect the milestone "Save" and "Remove" buttons, as they should be idempotent (save definitely is, remove may return an error on the second click).

[Jun Omae]

I think we should also prevent doubleclick in attach-file-form.

[Ryan J Ollos]

Replying to rblank:

• Technically, you wouldn't need to protect the milestone "Save" and "Remove" buttons, as they should be idempotent (save definitely is, remove may return an error on the second click).

With the Save button and a rename of the resource (component6 - > component7) or with the Remove button, I found that a double-click can result in:

Trac Error

Component component6 does not exist.

TracGuide — The Trac User and Administration Guide

---

This is with TracStandalone running locally, and I have to double or triple click the buttons pretty fast.

[Remy Blank]

Ok, the operations are idempotent but an error isn't nice, so let's protect these, too.

[Ryan J Ollos]

In [0d7c6f7db/rjollos.git] I've handled additional buttons for which the action is not idempotent, or an error results.

On the batch modification form, the comment operation is not idempotent (however, I didn't find any errors due to multi-submit of the other batch modification operations). Since there is another submit handler for the batch modification form, there is a problem on 1.0-stable - the button will be disabled until the page is refreshed if submit is pressed with no items in the table selected: branches/1.0-stable/trac/htdocs/js/query.js@12332:411,422-423#L410​. I've put a fix in place [cd20d667/rjollos.git]. This fix isn't ideal, but also isn't necessary on the trunk after [12374#file2], since the submit button is disabled until an item from the table is selected.

[Ryan J Ollos]

Replying to rjollos:

This fix isn't ideal, but also isn't necessary on the trunk after [12374#file2], since the submit button is disabled until an item from the table is selected.

Actually that is not entirely true yet, but will be after a follow-up change that I'll propose in #11056 - the remaining validation will be moved out of the submit handler and the code for handling radio buttons​ will be made consistent with the code for handling the checkboxes.

[Jun Omae]

I've tried log:rjollos.git:t10138 with IE 6, IE 8, Firefox 26 and Chrome 33 beta. That works well except one thing.

I think the Add milestone button should be protected. The button will be shonw if visiting not-existent milestone page, e.g. milestone/notfound.

trac/ticket/templates/milestone_edit.html

@@ -113 +113 @@
           <input py:when="True" type="submit" name="save"
                  value="${_('Submit changes')}" class="trac-disable-on-submit" />
           <input py:otherwise="" type="submit" name="add"
-                 value="${_('Add milestone')}" />
+                 value="${_('Add milestone')}" class="trac-disable-on-submit" />
           <input type="submit" name="cancel" value="${_('Cancel')}" />
         </div>
       </form>

[Ryan J Ollos]

Replying to jomae:

I think the Add milestone button should be protected. The button will be shonw if visiting not-existent milestone page, e.g. milestone/notfound.

I haven't been able to reproduce any issues due to multi-submit of this form. What issue were you seeing?

[Jun Omae]

Replying to rjollos:

Replying to jomae:

I think the Add milestone button should be protected. The button will be shonw if visiting not-existent milestone page, e.g. milestone/notfound.

I haven't been able to reproduce any issues due to multi-submit of this form. What issue were you seeing?

An IntegrityError rarely happens if the same milestone is concurrently added. But that is another issue, sorry. The button doesn't need the protection for mulit-submit.

2014-01-19 09:45:55,099 Trac[main] ERROR: Internal Server Error:
Traceback (most recent call last):
  File "/home/jun66j5/src/trac/edgewall/git/trac/web/main.py", line 504, in _dispatch_request
    dispatcher.dispatch(req)
  File "/home/jun66j5/src/trac/edgewall/git/trac/web/main.py", line 221, in dispatch
    resp = chosen_handler.process_request(req)
  File "/home/jun66j5/src/trac/edgewall/git/trac/ticket/roadmap.py", line 681, in process_request
    return self._do_save(req, milestone)
  File "/home/jun66j5/src/trac/edgewall/git/trac/ticket/roadmap.py", line 815, in _do_save
    milestone.insert()
  File "/home/jun66j5/src/trac/edgewall/git/trac/ticket/model.py", line 1042, in insert
    to_utimestamp(self.completed), self.description))
  File "/home/jun66j5/src/trac/edgewall/git/trac/db/util.py", line 121, in execute
    cursor.execute(query, params)
  File "/home/jun66j5/src/trac/edgewall/git/trac/db/util.py", line 54, in execute
    r = self.cursor.execute(sql_escape_percent(sql), args)
IntegrityError: duplicate key violates unique constraint "milestone_pkey"

[Ryan J Ollos]

Okay, thanks for testing!

[Ryan J Ollos]

Committed to 1.0-stable in [12458] and merged to trunk in [12459].

[Ryan J Ollos]

​DynamicFieldsPlugin prevents form submission on validation failure using a handler for the submit event. After form submission is prevented by DynamicFieldsPlugin, it won't be possible to submit the form again due to the presence of the trac-submit-is-disabled class on the form. The issue was reported in ​th:#12258, and was fixed in ​[th 14516]. However, I'll continue to consider whether there might be a better way solution.