Bartosz Dziewoński has uploaded a new change for review.
https://gerrit.wikimedia.org/r/269167
Change subject: Correct code dealing with plain DOM events that thought it had
jQuery events
......................................................................
Correct code dealing with plain DOM events that thought it had jQuery events
Many event handlers were documented as taking a jQuery.Event object,
implying that they were registered using $( ... ).on(), while they
actuall take a plain Event (MouseEvent or KeyboardEvent, in almost all
cases) and are registered using addEventListener().
This matters, because only jQuery event handlers support preventing
the default action by returning 'false' from the function; both jQuery
and plain support calling preventDefault(). This bad documentation
resulted in T126239.
Mostly comments changes, with some code changes that shouldn't change
the actual behavior.
Change-Id: I029bd4923eb7737252d3004a6a8ce56009941bd0
---
M src/ToolGroup.js
M src/mixins/ButtonElement.js
M src/toolgroups/PopupToolGroup.js
M src/widgets/CapsuleMultiSelectWidget.js
M src/widgets/MenuSelectWidget.js
M src/widgets/SelectWidget.js
6 files changed, 12 insertions(+), 13 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/oojs/ui refs/changes/67/269167/1
diff --git a/src/ToolGroup.js b/src/ToolGroup.js
index a0c598c..55607a1 100644
--- a/src/ToolGroup.js
+++ b/src/ToolGroup.js
@@ -176,7 +176,7 @@
* Handle captured mouse up and key up events.
*
* @protected
- * @param {Event} e Mouse up or key up event
+ * @param {MouseEvent|KeyboardEvent} e Mouse up or key up event
*/
OO.ui.ToolGroup.prototype.onCapturedMouseKeyUp = function ( e ) {
this.getElementDocument().removeEventListener( 'mouseup',
this.onCapturedMouseKeyUpHandler, true );
@@ -190,7 +190,7 @@
* Handle mouse up and key up events.
*
* @protected
- * @param {jQuery.Event} e Mouse up or key up event
+ * @param {MouseEvent|KeyboardEvent} e Mouse up or key up event
*/
OO.ui.ToolGroup.prototype.onMouseKeyUp = function ( e ) {
var tool = this.getTargetTool( e );
@@ -201,7 +201,8 @@
) {
this.pressed.onSelect();
this.pressed = null;
- return false;
+ e.preventDefault();
+ e.stopPropagation();
}
this.pressed = null;
diff --git a/src/mixins/ButtonElement.js b/src/mixins/ButtonElement.js
index bfbf20f..bf0de01 100644
--- a/src/mixins/ButtonElement.js
+++ b/src/mixins/ButtonElement.js
@@ -121,7 +121,7 @@
* Handles mouse up events.
*
* @protected
- * @param {jQuery.Event} e Mouse up event
+ * @param {MouseEvent} e Mouse up event
*/
OO.ui.mixin.ButtonElement.prototype.onMouseUp = function ( e ) {
if ( this.isDisabled() || e.which !== OO.ui.MouseButtons.LEFT ) {
@@ -167,7 +167,7 @@
* Handles key up events.
*
* @protected
- * @param {jQuery.Event} e Key up event
+ * @param {KeyboardEvent} e Key up event
*/
OO.ui.mixin.ButtonElement.prototype.onKeyUp = function ( e ) {
if ( this.isDisabled() || ( e.which !== OO.ui.Keys.SPACE && e.which !==
OO.ui.Keys.ENTER ) ) {
diff --git a/src/toolgroups/PopupToolGroup.js b/src/toolgroups/PopupToolGroup.js
index 80c11f0..b2099f7 100644
--- a/src/toolgroups/PopupToolGroup.js
+++ b/src/toolgroups/PopupToolGroup.js
@@ -102,7 +102,7 @@
* The event is actually generated from a mouseup/keyup, so it is not a normal
blur event object.
*
* @protected
- * @param {jQuery.Event} e Mouse up or key up event
+ * @param {MouseEvent|KeyboardEvent} e Mouse up or key up event
*/
OO.ui.PopupToolGroup.prototype.onBlur = function ( e ) {
// Only deactivate when clicking outside the dropdown element
diff --git a/src/widgets/CapsuleMultiSelectWidget.js
b/src/widgets/CapsuleMultiSelectWidget.js
index 0ea8429..ce7fb46 100644
--- a/src/widgets/CapsuleMultiSelectWidget.js
+++ b/src/widgets/CapsuleMultiSelectWidget.js
@@ -427,7 +427,7 @@
* Handles popup focus out events.
*
* @private
- * @param {Event} e Focus out event
+ * @param {jQuery.Event} e Focus out event
*/
OO.ui.CapsuleMultiSelectWidget.prototype.onPopupFocusOut = function () {
var widget = this.popup;
diff --git a/src/widgets/MenuSelectWidget.js b/src/widgets/MenuSelectWidget.js
index 0f4a269..88b1b2e 100644
--- a/src/widgets/MenuSelectWidget.js
+++ b/src/widgets/MenuSelectWidget.js
@@ -79,7 +79,7 @@
* Handles document mouse down events.
*
* @protected
- * @param {jQuery.Event} e Key down event
+ * @param {MouseEvent} e Mouse down event
*/
OO.ui.MenuSelectWidget.prototype.onDocumentMouseDown = function ( e ) {
if (
diff --git a/src/widgets/SelectWidget.js b/src/widgets/SelectWidget.js
index bfd9c19..0841339 100644
--- a/src/widgets/SelectWidget.js
+++ b/src/widgets/SelectWidget.js
@@ -173,7 +173,7 @@
* Handle mouse up events.
*
* @private
- * @param {jQuery.Event} e Mouse up event
+ * @param {MouseEvent} e Mouse up event
*/
OO.ui.SelectWidget.prototype.onMouseUp = function ( e ) {
var item;
@@ -201,7 +201,7 @@
* Handle mouse move events.
*
* @private
- * @param {jQuery.Event} e Mouse move event
+ * @param {MouseEvent} e Mouse move event
*/
OO.ui.SelectWidget.prototype.onMouseMove = function ( e ) {
var item;
@@ -213,7 +213,6 @@
this.selecting = item;
}
}
- return false;
};
/**
@@ -251,7 +250,7 @@
* Handle key down events.
*
* @protected
- * @param {jQuery.Event} e Key down event
+ * @param {KeyboardEvent} e Key down event
*/
OO.ui.SelectWidget.prototype.onKeyDown = function ( e ) {
var nextItem,
@@ -301,7 +300,6 @@
}
if ( handled ) {
- // Can't just return false, because e is not always a
jQuery event
e.preventDefault();
e.stopPropagation();
}
--
To view, visit https://gerrit.wikimedia.org/r/269167
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I029bd4923eb7737252d3004a6a8ce56009941bd0
Gerrit-PatchSet: 1
Gerrit-Project: oojs/ui
Gerrit-Branch: master
Gerrit-Owner: Bartosz Dziewoński <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
