r112702 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r112701‎ | r112702 | r112703 >
Date:18:03, 29 February 2012
Author:krinkle
Status:reverted (Comments)
Tags:
Comment:
[ApiSandbox] Prevent duplicate submission of the form when clicking "Make Request"
* The buttons had type=submit and were inside the <form> tree, when not prevented the form is naturally submitted in a non-ajax fashion (which is then hooked into from the submit handler)
* Fixes:
-- (bug 34790) [Regression] Pressing "Make Request" shouldn't make two requests to api.php
Modified paths:
  • /trunk/extensions/ApiSandbox/ext.apiSandbox.css (replaced) (history)
  • /trunk/extensions/ApiSandbox/ext.apiSandbox.js (replaced) (history)

Diff [purge]

Index: trunk/extensions/ApiSandbox/ext.apiSandbox.js
——@@ -516,6 +516,10 @@
517517 .appendTo( '#api-sandbox-parameters' )
518518 .add( $submit )
519519 .click( function ( e ) {
 520+ // Don't do default action (crawl up to <form> and trigger a submit).
 521+ // That would submit it twice (bug 34790)
 522+ e.preventDefault();
 523+
520524 $form.submit();
521525 } )
522526 .button({ disabled: true });
——@@ -602,6 +606,9 @@
603607 $form.submit( function ( e ) {
604608 var url, params, mustBePosted;
605609
 610+ // Prevent browser from submitting the form
 611+ // and reloading the page to the action-url.
 612+ // We're doing it with AJAX instead, below.
606613 e.preventDefault();
607614
608615 if ( $submit.button( 'option', 'disabled' ) === true ) {
——@@ -623,6 +630,7 @@
624631 }
625632
626633 showLoading( $output );
 634+
627635 if ( mustBePosted ) {
628636 $requestUrl.val( url );
629637 if ( params.length > 0 ) {
Property changes on: trunk/extensions/ApiSandbox/ext.apiSandbox.js
___________________________________________________________________
Deleted: svn:eol-style
630638 - native
Property changes on: trunk/extensions/ApiSandbox/ext.apiSandbox.css
___________________________________________________________________
Deleted: svn:eol-style
631639 - native

Follow-up revisions

RevisionCommit summaryAuthorDate
r112720revert r112702 bad commit...hashar20:50, 29 February 2012
r112721[ApiSandbox] Prevent duplicate submission of the form when clicking "Make Req...krinkle20:52, 29 February 2012
r112723MFT to 1.19wmf1 r112721...hashar20:56, 29 February 2012

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r109726[ApiSandbox] front-end makeover...krinkle07:10, 22 January 2012

Comments

#Comment by Krinkle (talk | contribs)   18:04, 29 February 2012 
Deleted: svn:eol-style
- native

uh ?

#Comment by Reedy (talk | contribs)   18:41, 29 February 2012

Did you use git via some way or another?

#Comment by Krinkle (talk | contribs)   19:34, 29 February 2012

No, I just modified the javascript file and committed it with svn commit -F /path/to.txt.

Then was surprised that the .css file was also being committed (albeit not showing up in "svn status" or "svn diff").

However when checking now, I can't re-add it either, it says the prop for "svn:eol-style" is still "native" as it should be.

#Comment by Hashar (talk | contribs)   20:44, 29 February 2012

Somehow this commit is replacing the text which cause some kind of tree conflict when attempting to merge the commit to a different branch. Yeah branching is fun with svn.

Shell session below:

Attempting to merge one file does not do anything: 

hashar/srv$ svn merge -c 112702 /srv/{trunk,1.19wmf1}/extensions/ApiSandbox/ext.apiSandbox.js
hashar/srv$


Directory merge cause a tree conflict: 

hashar/srv$ svn merge -c 112702 /srv/{trunk,1.19wmf1}/extensions/ApiSandbox
--- Merging [[Special:Code/MediaWiki/112702|r112702]] into '/srv/1.19wmf1/extensions/ApiSandbox':
   C /srv/1.19wmf1/extensions/ApiSandbox/ext.apiSandbox.js
R    /srv/1.19wmf1/extensions/ApiSandbox/ext.apiSandbox.css
Summary of conflicts:
  Tree conflicts: 1

Help! :-) 

hashar/srv$ svn status /srv/1.19wmf1/extensions/ApiSandbox/
 M      /srv/1.19wmf1/extensions/ApiSandbox
      C /srv/1.19wmf1/extensions/ApiSandbox/ext.apiSandbox.js
      >   local edit, incoming delete upon merge
R  +    /srv/1.19wmf1/extensions/ApiSandbox/ext.apiSandbox.css


This is mostly for history purpose. Talked about that issue with Reedy and Krinkle on IRC.

#Comment by Hashar (talk | contribs)   20:50, 29 February 2012

reverted using: 

$ cd /srv/trunk/extension/ApiSandbox
$ svn merge -c -112702 .

Status & tagging log