jenkins-bot has submitted this change and it was merged.
Change subject: Correct code using plain DOM events documented as jQuery events
......................................................................
Correct code using plain DOM events documented as jQuery events
Many event handlers were documented as taking a jQuery.Event object,
implying that they were registered using $( ... ).on(), while they
actually 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 behaviour.
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(-)
Approvals:
Jforrester: Looks good to me, approved
jenkins-bot: Verified
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 93d0a7a..5b94611 100644
--- a/src/widgets/CapsuleMultiSelectWidget.js
+++ b/src/widgets/CapsuleMultiSelectWidget.js
@@ -454,7 +454,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 4e661ca..0dd683c 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 550e681..2f76744 100644
--- a/src/widgets/SelectWidget.js
+++ b/src/widgets/SelectWidget.js
@@ -172,7 +172,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;
@@ -200,7 +200,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;
@@ -212,7 +212,6 @@
this.selecting = item;
}
}
- return false;
};
/**
@@ -248,7 +247,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,
@@ -298,7 +297,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: merged
Gerrit-Change-Id: I029bd4923eb7737252d3004a6a8ce56009941bd0
Gerrit-PatchSet: 3
Gerrit-Project: oojs/ui
Gerrit-Branch: master
Gerrit-Owner: Bartosz Dziewoński <[email protected]>
Gerrit-Reviewer: Jforrester <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
