[MediaWiki-commits] [Gerrit] Improve client-side edit stash change detection - change (mediawiki/core)

2016-06-14 Thread Ori.livneh (Code Review)
Ori.livneh has submitted this change and it was merged.

Change subject: Improve client-side edit stash change detection
..


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
(cherry picked from commit eca800c7f02023030a8e97bfc611d222f0edcdb3)
(cherry picked from commit 3e48c0ff370819a560f90b00f72b45c62ca2adf0)
---
M resources/src/mediawiki.action/mediawiki.action.edit.stash.js
1 file changed, 9 insertions(+), 12 deletions(-)

Approvals:
  Ori.livneh: Verified; Looks good to me, approved



diff --git a/resources/src/mediawiki.action/mediawiki.action.edit.stash.js 
b/resources/src/mediawiki.action/mediawiki.action.edit.stash.js
index d354fc2..3288e1a 100644
--- a/resources/src/mediawiki.action/mediawiki.action.edit.stash.js
+++ b/resources/src/mediawiki.action/mediawiki.action.edit.stash.js
@@ -45,7 +45,7 @@
return newText !== data.wpTextbox1;
}
 
-   function onTextChanged() {
+   function onEditorIdle() {
if ( !isChanged() ) {
return;
}
@@ -53,20 +53,17 @@
stashEdit();
}
 
-   function onTextKeyPress( e ) {
+   function onTextKeyUp( e ) {
// Ignore keystrokes that don't modify text, like 
cursor movements.
-   // See .
-   if ( e.which === 0 ) {
+   // See  
and
+   // . 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() {
@@ -84,8 +81,8 @@
return;
}
 
-   $text.on( { change: onTextChanged, keypress: onTextKeyPress } );
-   $summary.on( { focus: onTextChanged } );
+   $text.on( { change: onEditorIdle, keyup: onTextKeyUp } );
+   $summary.on( { focus: onEditorIdle } );
onFormLoaded();
 
} );

-- 
To view, visit https://gerrit.wikimedia.org/r/294418
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Idfad8407c8e905d8de80bc54379545f1b586fc88
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: wmf/1.28.0-wmf.5
Gerrit-Owner: Ori.livneh 
Gerrit-Reviewer: Edokter 
Gerrit-Reviewer: Jack Phoenix 
Gerrit-Reviewer: Ori.livneh 

___
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits


[MediaWiki-commits] [Gerrit] Improve client-side edit stash change detection - change (mediawiki/core)

2016-06-14 Thread Ori.livneh (Code Review)
Ori.livneh has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/294418

Change subject: Improve client-side edit stash change detection
..

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
(cherry picked from commit eca800c7f02023030a8e97bfc611d222f0edcdb3)
(cherry picked from commit 3e48c0ff370819a560f90b00f72b45c62ca2adf0)
---
M resources/src/mediawiki.action/mediawiki.action.edit.stash.js
1 file changed, 9 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/18/294418/1

diff --git a/resources/src/mediawiki.action/mediawiki.action.edit.stash.js 
b/resources/src/mediawiki.action/mediawiki.action.edit.stash.js
index d354fc2..3288e1a 100644
--- a/resources/src/mediawiki.action/mediawiki.action.edit.stash.js
+++ b/resources/src/mediawiki.action/mediawiki.action.edit.stash.js
@@ -45,7 +45,7 @@
return newText !== data.wpTextbox1;
}
 
-   function onTextChanged() {
+   function onEditorIdle() {
if ( !isChanged() ) {
return;
}
@@ -53,20 +53,17 @@
stashEdit();
}
 
-   function onTextKeyPress( e ) {
+   function onTextKeyUp( e ) {
// Ignore keystrokes that don't modify text, like 
cursor movements.
-   // See .
-   if ( e.which === 0 ) {
+   // See  
and
+   // . 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() {
@@ -84,8 +81,8 @@
return;
}
 
-   $text.on( { change: onTextChanged, keypress: onTextKeyPress } );
-   $summary.on( { focus: onTextChanged } );
+   $text.on( { change: onEditorIdle, keyup: onTextKeyUp } );
+   $summary.on( { focus: onEditorIdle } );
onFormLoaded();
 
} );

-- 
To view, visit https://gerrit.wikimedia.org/r/294418
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Idfad8407c8e905d8de80bc54379545f1b586fc88
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: wmf/1.28.0-wmf.5
Gerrit-Owner: Ori.livneh 

___
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits


[MediaWiki-commits] [Gerrit] Improve client-side edit stash change detection - change (mediawiki/core)

2016-06-14 Thread jenkins-bot (Code Review)
jenkins-bot has submitted this change and it was merged.

Change subject: Improve client-side edit stash change detection
..


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
(cherry picked from commit eca800c7f02023030a8e97bfc611d222f0edcdb3)
---
M resources/src/mediawiki.action/mediawiki.action.edit.stash.js
1 file changed, 9 insertions(+), 12 deletions(-)

Approvals:
  Ori.livneh: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/resources/src/mediawiki.action/mediawiki.action.edit.stash.js 
b/resources/src/mediawiki.action/mediawiki.action.edit.stash.js
index 71ed44c..297f814 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 .
-   if ( e.which === 0 ) {
+   // See  
and
+   // . 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();
 
} );

-- 
To view, visit https://gerrit.wikimedia.org/r/294401
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Idfad8407c8e905d8de80bc54379545f1b586fc88
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/core
Gerrit-Branch: wmf/1.28.0-wmf.6
Gerrit-Owner: Ori.livneh 
Gerrit-Reviewer: Edokter 
Gerrit-Reviewer: Jack Phoenix 
Gerrit-Reviewer: Ori.livneh 
Gerrit-Reviewer: jenkins-bot <>

___
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits


[MediaWiki-commits] [Gerrit] Improve client-side edit stash change detection - change (mediawiki/core)

2016-06-14 Thread Ori.livneh (Code Review)
Ori.livneh has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/294401

Change subject: Improve client-side edit stash change detection
..

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
(cherry picked from commit eca800c7f02023030a8e97bfc611d222f0edcdb3)
---
M resources/src/mediawiki.action/mediawiki.action.edit.stash.js
1 file changed, 9 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/01/294401/1

diff --git a/resources/src/mediawiki.action/mediawiki.action.edit.stash.js 
b/resources/src/mediawiki.action/mediawiki.action.edit.stash.js
index 71ed44c..297f814 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 .
-   if ( e.which === 0 ) {
+   // See  
and
+   // . 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();
 
} );

-- 
To view, visit https://gerrit.wikimedia.org/r/294401
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Idfad8407c8e905d8de80bc54379545f1b586fc88
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: wmf/1.28.0-wmf.6
Gerrit-Owner: Ori.livneh 

___
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits


[MediaWiki-commits] [Gerrit] Improve client-side edit stash change detection - change (mediawiki/core)

2016-06-14 Thread jenkins-bot (Code Review)
jenkins-bot has submitted this change and it was merged.

Change subject: Improve client-side edit stash change detection
..


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
---
M resources/src/mediawiki.action/mediawiki.action.edit.stash.js
1 file changed, 9 insertions(+), 12 deletions(-)

Approvals:
  Ori.livneh: Looks good to me, approved
  Aaron Schulz: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/resources/src/mediawiki.action/mediawiki.action.edit.stash.js 
b/resources/src/mediawiki.action/mediawiki.action.edit.stash.js
index 71ed44c..297f814 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 .
-   if ( e.which === 0 ) {
+   // See  
and
+   // . 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();
 
} );

-- 
To view, visit https://gerrit.wikimedia.org/r/294370
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Idfad8407c8e905d8de80bc54379545f1b586fc88
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Ori.livneh 
Gerrit-Reviewer: Aaron Schulz 
Gerrit-Reviewer: Edokter 
Gerrit-Reviewer: Jack Phoenix 
Gerrit-Reviewer: Ori.livneh 
Gerrit-Reviewer: jenkins-bot <>

___
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits


[MediaWiki-commits] [Gerrit] Improve client-side edit stash change detection - change (mediawiki/core)

2016-06-14 Thread Ori.livneh (Code Review)
Ori.livneh has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/294370

Change subject: Improve client-side edit stash change detection
..

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
---
M resources/src/mediawiki.action/mediawiki.action.edit.stash.js
1 file changed, 10 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/70/294370/1

diff --git a/resources/src/mediawiki.action/mediawiki.action.edit.stash.js 
b/resources/src/mediawiki.action/mediawiki.action.edit.stash.js
index d354fc2..fcba766 100644
--- a/resources/src/mediawiki.action/mediawiki.action.edit.stash.js
+++ b/resources/src/mediawiki.action/mediawiki.action.edit.stash.js
@@ -45,7 +45,7 @@
return newText !== data.wpTextbox1;
}
 
-   function onTextChanged() {
+   function onEditorIdle() {
if ( !isChanged() ) {
return;
}
@@ -53,20 +53,22 @@
stashEdit();
}
 
-   function onTextKeyPress( e ) {
+   function onTextKeyUp( e ) {
// Ignore keystrokes that don't modify text, like 
cursor movements.
-   // See .
-   if ( e.which === 0 ) {
+   // See  
and
+   // . 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 ) {
+   if ( pending && !isChanged() ) {
pending.abort();
}
 
-   timer = setTimeout( onTextChanged, idleTimeout );
+   timer = setTimeout( onEditorIdle, idleTimeout );
}
 
function onFormLoaded() {
@@ -84,8 +86,8 @@
return;
}
 
-   $text.on( { change: onTextChanged, keypress: onTextKeyPress } );
-   $summary.on( { focus: onTextChanged } );
+   $text.on( { change: onEditorIdle, keyup: onTextKeyUp } );
+   $summary.on( { focus: onEditorIdle } );
onFormLoaded();
 
} );

-- 
To view, visit https://gerrit.wikimedia.org/r/294370
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Idfad8407c8e905d8de80bc54379545f1b586fc88
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Ori.livneh 

___
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits