If a Workflow form receives a redirect response, don't re-enable the submit buttons

Summary:
See PHI488. Ref T13108. Currently, there is a narrow window between when the response returns and when the browser actually follows the redirect where the form is live and you can click the button again.

This is relativey easy if Phabricator is running //too fast// since the button may be disabled only momentarily. This seems to be easier in Firefox/Chrome than Safari.

Test Plan:
- In Firefox and Chrome, spam-clicked a comment submit button.
- Before: could sometimes get a double-submit.
- After: couldn't get a double-submit.
- This could probably be reproduced more reliabily by adding a `sleep(1)` to whatever we're redirecting //to//.
- Submitted an empty comment, got a dialog plus a still-enabled form (so this doesn't break the non-redirect case).

Maniphest Tasks: T13108

Differential Revision: https://secure.phabricator.com/D19245

epriestley 8 years ago 9e278a89 6ed123e0

+25 -18

2 changed files

expand all

resources

celerity

map.php

webroot

rsrc

externals

javelin

lib

Workflow.js

+14 -14

resources/celerity/map.php

··· 10 10 'conpherence.pkg.css' => 'e68cf1fa', 11 11 'conpherence.pkg.js' => '15191c65', 12 12 'core.pkg.css' => '6da3c0e5', 13 - 'core.pkg.js' => '932d60d4', 13 + 'core.pkg.js' => 'b305dbe2', 14 14 'differential.pkg.css' => '113e692c', 15 15 'differential.pkg.js' => 'f6d809c0', 16 16 'diffusion.pkg.css' => 'a2d17c7d', ··· 253 253 'rsrc/externals/javelin/lib/URI.js' => 'c989ade3', 254 254 'rsrc/externals/javelin/lib/Vector.js' => '2caa8fb8', 255 255 'rsrc/externals/javelin/lib/WebSocket.js' => '3ffe32d6', 256 - 'rsrc/externals/javelin/lib/Workflow.js' => '0eb1db0c', 256 + 'rsrc/externals/javelin/lib/Workflow.js' => '33fea02f', 257 257 'rsrc/externals/javelin/lib/__tests__/Cookie.js' => '5ed109e8', 258 258 'rsrc/externals/javelin/lib/__tests__/DOM.js' => 'c984504b', 259 259 'rsrc/externals/javelin/lib/__tests__/JSON.js' => '837a7d68', ··· 739 739 'javelin-workboard-card' => 'c587b80f', 740 740 'javelin-workboard-column' => '758b4758', 741 741 'javelin-workboard-controller' => '26167537', 742 - 'javelin-workflow' => '0eb1db0c', 742 + 'javelin-workflow' => '33fea02f', 743 743 'maniphest-report-css' => '9b9580b7', 744 744 'maniphest-task-edit-css' => 'fda62a9b', 745 745 'maniphest-task-summary-css' => '11cc5344', ··· 960 960 'javelin-dom', 961 961 'javelin-router', 962 962 ), 963 - '0eb1db0c' => array( 964 - 'javelin-stratcom', 965 - 'javelin-request', 966 - 'javelin-dom', 967 - 'javelin-vector', 968 - 'javelin-install', 969 - 'javelin-util', 970 - 'javelin-mask', 971 - 'javelin-uri', 972 - 'javelin-routable', 973 - ), 974 963 '0f764c35' => array( 975 964 'javelin-install', 976 965 'javelin-util', ··· 1107 1096 'javelin-event', 1108 1097 'javelin-util', 1109 1098 'javelin-magical-init', 1099 + ), 1100 + '33fea02f' => array( 1101 + 'javelin-stratcom', 1102 + 'javelin-request', 1103 + 'javelin-dom', 1104 + 'javelin-vector', 1105 + 'javelin-install', 1106 + 'javelin-util', 1107 + 'javelin-mask', 1108 + 'javelin-uri', 1109 + 'javelin-routable', 1110 1110 ), 1111 1111 '358b8c04' => array( 1112 1112 'javelin-install',

+11 -4

webroot/rsrc/externals/javelin/lib/Workflow.js

··· 59 59 60 60 workflow.setDataWithListOfPairs(pairs); 61 61 workflow.setMethod(form.getAttribute('method')); 62 - workflow.listen('finally', function() { 63 - // Re-enable form elements 64 - for (var ii = 0; ii < inputs.length; ii++) { 65 - inputs[ii] && (inputs[ii].disabled = false); 62 + 63 + var onfinally = JX.bind(workflow, function() { 64 + if (!this._keepControlsDisabled) { 65 + for (var ii = 0; ii < inputs.length; ii++) { 66 + inputs[ii] && (inputs[ii].disabled = false); 67 + } 66 68 } 67 69 }); 70 + workflow.listen('finally', onfinally); 68 71 69 72 return workflow; 70 73 }, ··· 242 245 _form: null, 243 246 _paused: 0, 244 247 _nextCallback: null, 248 + _keepControlsDisabled: false, 245 249 246 250 getSourceForm: function() { 247 251 return this._form; ··· 282 286 if (r.close) { 283 287 this._pop(); 284 288 } 289 + 290 + // If we're redirecting, don't re-enable for controls. 291 + this._keepControlsDisabled = true; 285 292 286 293 JX.$U(r.redirect).go(); 287 294 } else if (r && r.dialog) {

Configure Feed

Configure Feed