Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fixed issue #18026: Session expire message when submit button is clic…
…ked twice (#2353)
  • Loading branch information
gabrieljenik committed Apr 25, 2022
1 parent a75c3f4 commit 8082779
Showing 1 changed file with 25 additions and 1 deletion.
26 changes: 25 additions & 1 deletion assets/packages/limesurvey/survey.js
Expand Up @@ -404,4 +404,28 @@ function resetQuestionTimers(sid) {
window.localStorage.removeItem('limesurvey_timers_' + timersessionname);
});
window.localStorage.removeItem(surveyTimersItemName);
}
}

/**
* Disable submit button to prevent multiple submits
* This is done on 'document' instead of the '#limesurvey' form in order to allow
* other scripts (custom themes?) to cancel the submit before we disable the button.
*/
$(document).on('submit', function (e) {
// If the target is not the '#limesurvey' form, don't do anything.
if (e.target.id != 'limesurvey') {
return;
}
// We only care about the final submit, not normal forward/backward navigation.
var submitter = $(e.originalEvent.submitter);
if (submitter.attr('value') != 'movesubmit') {
return;
}
// Still, we disable all submit buttons to make sure the "back" button is not
// pressed while submitting.
$('#limesurvey button[type="submit"]').prop('disabled', true);

// We also add a hidden input with the button's value, because it's not included
// in the request when the button is disabled.
$('#limesurvey').append('<input id="onsubmitbuttoninput" name=\'' + submitter.attr('name') + '\' value=\'' + submitter.attr('value') + '\' type=\'hidden\' />');
});

7 comments on commit 8082779

@Shnoulle
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Shnoulle
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@olleharstedt
Copy link
Contributor

@olleharstedt olleharstedt commented on 8082779 May 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, not good. @gabrieljenik Please add automatic integrity tests for all issues related to this regression. To both LS3 and LS5, if possible (LS5 has priority).

@Shnoulle
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really think we need only LS5 test, no ?

@olleharstedt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really think we need only LS5 test, no ?

Yeah, maybe

@gabrieljenik
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All those maybe fixed here?
https://bugs.limesurvey.org/view.php?id=18074

@olleharstedt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, the bug could have been avoided if we started to use Flow on vanilla JS. It doesn't allow to make assumptions on if something is undefined/null or not, so you're forced to do more checking/defensive programming.

https://flow.org/

Please sign in to comment.