From eca800c7f02023030a8e97bfc611d222f0edcdb3 Mon Sep 17 00:00:00 2001
From: Ori Livneh <ori@wikimedia.org>
Date: Tue, 14 Jun 2016 12:58:20 -0700
Subject: [PATCH] Improve client-side edit stash change detection

The keypress event does not fire for backspace or delete in IE, Chrome, or
Safari, which means we are missing out on stash opportunities when the last
action is to delete some text. Fix that by listening for the keyup event
instead.

Also add an isChanged() check before calling pending.abort(), because there are
a lot of special keys that don't modify the text, and not all of them are coded
consistently on different platforms (think volume up/down, mute, function keys,
etc.), so we can't be exhaustive, and should instead fall back to actually
checking for changes. Otherwise we risk aborting stash requests when the user
has not changed the text.

Lastly, rename 'onTextChanged' to 'onEditorIdle', which is more accurate.
On undo / rollback, onTextChanged will return true the first time it is called,
even though the text had not changed in that case.

Useful sources:

* Key codes of keydown and keyup events:
  http://www.javascripter.net/faq/keycodes.htm
* Quirksmode: detecting keystrokes
  http://www.quirksmode.org/js/keys.html
* Why isn't backspace being detected using jquery keypress event?
  http://stackoverflow.com/q/4418562

Change-Id: Idfad8407c8e905d8de80bc54379545f1b586fc88
---
 .../mediawiki.action.edit.stash.js            | 21 ++++++++-----------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/resources/src/mediawiki.action/mediawiki.action.edit.stash.js b/resources/src/mediawiki.action/mediawiki.action.edit.stash.js
index 71ed44ca7f..297f814f12 100644
--- a/resources/src/mediawiki.action/mediawiki.action.edit.stash.js
+++ b/resources/src/mediawiki.action/mediawiki.action.edit.stash.js
@@ -46,7 +46,7 @@
 			return newText !== data.wpTextbox1;
 		}
 
-		function onTextChanged() {
+		function onEditorIdle() {
 			if ( !isChanged() ) {
 				return;
 			}
@@ -54,20 +54,17 @@
 			stashEdit();
 		}
 
-		function onTextKeyPress( e ) {
+		function onTextKeyUp( e ) {
 			// Ignore keystrokes that don't modify text, like cursor movements.
-			// See <http://stackoverflow.com/q/2284844>.
-			if ( e.which === 0 ) {
+			// See <http://www.javascripter.net/faq/keycodes.htm> and
+			// <http://www.quirksmode.org/js/keys.html>. We don't have to be
+			// exhaustive, because the cost of misfiring is low.
+			if ( ( e.which >= 33 && e.which <= 40 ) || ( e.which >= 16 && e.which <= 18 ) ) {
 				return;
 			}
 
 			clearTimeout( timer );
-
-			if ( pending ) {
-				pending.abort();
-			}
-
-			timer = setTimeout( onTextChanged, idleTimeout );
+			timer = setTimeout( onEditorIdle, idleTimeout );
 		}
 
 		function onFormLoaded() {
@@ -90,8 +87,8 @@
 			return;
 		}
 
-		$text.on( { change: onTextChanged, keypress: onTextKeyPress } );
-		$summary.on( { focus: onTextChanged } );
+		$text.on( { change: onEditorIdle, keyup: onTextKeyUp } );
+		$summary.on( { focus: onEditorIdle } );
 		onFormLoaded();
 
 	} );
-- 
2.20.1