From dc44c66057305eb30393d57b73f1ece15ee44249 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Thu, 4 Aug 2022 17:35:02 +0000 Subject: [PATCH] refactor: remove AnyDuringMigration from browser events (#6303) * fix: bind data * fix: use correct event types * fix: make event happy with us clueging properties onto it * fix: change thisObject to type Object | null * chore: format * chore: call out event conversion wierdness * chore: fix build by changing things to use MouseEvent * chore: remove default value for object * fix: unbind trying to get an element of an empty array * chore: format * fix: call out potentially unsafe element access. * chore: add issue number to TODO * fix: attempt to fix CI build error? --- core/browser_events.ts | 62 +++++++++++++++++------------------ core/bubble.ts | 6 ++-- core/flyout_base.ts | 6 ++-- core/gesture.ts | 6 ++-- core/icon.ts | 2 +- core/mutator.ts | 2 +- core/scrollbar.ts | 4 +-- core/toolbox/toolbox.ts | 2 +- core/touch_gesture.ts | 8 ++--- core/utils.ts | 4 +-- core/workspace_comment_svg.ts | 4 +-- core/workspace_svg.ts | 6 ++-- 12 files changed, 55 insertions(+), 57 deletions(-) diff --git a/core/browser_events.ts b/core/browser_events.ts index 00d275ba3..eb798d03e 100644 --- a/core/browser_events.ts +++ b/core/browser_events.ts @@ -24,7 +24,7 @@ import * as userAgent from './utils/useragent.js'; * `bind` and `conditionalBind`. * @alias Blockly.browserEvents.Data */ -export type Data = AnyDuringMigration[][]; +export type Data = [EventTarget, string, (e: Event) => void][]; /** * The multiplier for scroll wheel deltas using the line delta mode. @@ -59,11 +59,10 @@ const PAGE_MODE_MULTIPLIER = 125; * @alias Blockly.browserEvents.conditionalBind */ export function conditionalBind( - node: EventTarget, name: string, thisObject: AnyDuringMigration|null, - func: Function, opt_noCaptureIdentifier?: boolean, - opt_noPreventDefault?: boolean): Data { + node: EventTarget, name: string, thisObject: Object|null, func: Function, + opt_noCaptureIdentifier?: boolean, opt_noPreventDefault?: boolean): Data { let handled = false; - function wrapFunc(e: AnyDuringMigration) { + function wrapFunc(e: Event) { const captureIdentifier = !opt_noCaptureIdentifier; // Handle each touch point separately. If the event was a mouse event, this // will hand back an array with one element, which we're fine handling. @@ -83,7 +82,7 @@ export function conditionalBind( } } - const bindData = []; + const bindData: Data = []; if (globalThis['PointerEvent'] && name in Touch.TOUCH_MAP) { for (let i = 0; i < Touch.TOUCH_MAP[name].length; i++) { const type = Touch.TOUCH_MAP[name][i]; @@ -96,7 +95,7 @@ export function conditionalBind( // Add equivalent touch event. if (name in Touch.TOUCH_MAP) { - const touchWrapFunc = (e: AnyDuringMigration) => { + const touchWrapFunc = (e: Event) => { wrapFunc(e); // Calling preventDefault stops the browser from scrolling/zooming the // page. @@ -128,9 +127,9 @@ export function conditionalBind( * @alias Blockly.browserEvents.bind */ export function bind( - node: EventTarget, name: string, thisObject: AnyDuringMigration|null, + node: EventTarget, name: string, thisObject: Object|null, func: Function): Data { - function wrapFunc(e: AnyDuringMigration) { + function wrapFunc(e: Event) { if (thisObject) { func.call(thisObject, e); } else { @@ -138,7 +137,7 @@ export function bind( } } - const bindData = []; + const bindData: Data = []; if (globalThis['PointerEvent'] && name in Touch.TOUCH_MAP) { for (let i = 0; i < Touch.TOUCH_MAP[name].length; i++) { const type = Touch.TOUCH_MAP[name][i]; @@ -151,13 +150,17 @@ export function bind( // Add equivalent touch event. if (name in Touch.TOUCH_MAP) { - const touchWrapFunc = (e: AnyDuringMigration) => { + const touchWrapFunc = (e: Event) => { // Punt on multitouch events. - if (e.changedTouches && e.changedTouches.length === 1) { + if (e instanceof TouchEvent && e.changedTouches && + e.changedTouches.length === 1) { // Map the touch event's properties to the event. const touchPoint = e.changedTouches[0]; - e.clientX = touchPoint.clientX; - e.clientY = touchPoint.clientY; + // TODO (6311): We are trying to make a touch event look like a mouse + // event, which is not allowed, because it requires adding more + // properties to the event. How do we want to deal with this? + (e as AnyDuringMigration).clientX = touchPoint.clientX; + (e as AnyDuringMigration).clientY = touchPoint.clientY; } wrapFunc(e); @@ -181,16 +184,19 @@ export function bind( * @return The function call. * @alias Blockly.browserEvents.unbind */ -export function unbind(bindData: Data): Function { - let func; +export function unbind(bindData: Data): (e: Event) => void { + // Accessing an element of the last property of the array is unsafe if the + // bindData is an empty array. But that should never happen because developers + // should only pass Data from bind or conditionalBind. + let callback = bindData[bindData.length - 1][2]; while (bindData.length) { const bindDatum = bindData.pop(); const node = bindDatum![0]; const name = bindDatum![1]; - func = bindDatum![2]; + const func = bindDatum![2]; node.removeEventListener(name, func, false); } - return func; + return callback; } /** @@ -228,17 +234,13 @@ export function isTargetInput(e: Event): boolean { * @return True if right-click. * @alias Blockly.browserEvents.isRightButton */ -export function isRightButton(e: Event): boolean { - // AnyDuringMigration because: Property 'ctrlKey' does not exist on type - // 'Event'. - if ((e as AnyDuringMigration).ctrlKey && userAgent.MAC) { +export function isRightButton(e: MouseEvent): boolean { + if (e.ctrlKey && userAgent.MAC) { // Control-clicking on Mac OS X is treated as a right-click. // WebKit on Mac OS X fails to change button to 2 (but Gecko does). return true; } - // AnyDuringMigration because: Property 'button' does not exist on type - // 'Event'. - return (e as AnyDuringMigration).button === 2; + return e.button === 2; } /** @@ -251,14 +253,10 @@ export function isRightButton(e: Event): boolean { * @alias Blockly.browserEvents.mouseToSvg */ export function mouseToSvg( - e: Event, svg: SVGSVGElement, matrix: SVGMatrix|null): SVGPoint { + e: MouseEvent, svg: SVGSVGElement, matrix: SVGMatrix|null): SVGPoint { const svgPoint = svg.createSVGPoint(); - // AnyDuringMigration because: Property 'clientX' does not exist on type - // 'Event'. - svgPoint.x = (e as AnyDuringMigration).clientX; - // AnyDuringMigration because: Property 'clientY' does not exist on type - // 'Event'. - svgPoint.y = (e as AnyDuringMigration).clientY; + svgPoint.x = e.clientX; + svgPoint.y = e.clientY; if (!matrix) { matrix = svg.getScreenCTM()!.inverse(); diff --git a/core/bubble.ts b/core/bubble.ts index f9a70eddd..4ec2563ae 100644 --- a/core/bubble.ts +++ b/core/bubble.ts @@ -321,7 +321,7 @@ export class Bubble implements IBubble { * Handle a mouse-down on bubble's resize corner. * @param e Mouse down event. */ - private resizeMouseDown_(e: Event) { + private resizeMouseDown_(e: MouseEvent) { this.promote(); Bubble.unbindDragEvents_(); if (browserEvents.isRightButton(e)) { @@ -348,7 +348,7 @@ export class Bubble implements IBubble { * Resize this bubble to follow the mouse. * @param e Mouse move event. */ - private resizeMouseMove_(e: Event) { + private resizeMouseMove_(e: MouseEvent) { this.autoLayout_ = false; const newXY = this.workspace_.moveDrag(e); this.setBubbleSize(this.workspace_.RTL ? -newXY.x : newXY.x, newXY.y); @@ -833,7 +833,7 @@ export class Bubble implements IBubble { * Handle a mouse-up event while dragging a bubble's border or resize handle. * @param _e Mouse up event. */ - private static bubbleMouseUp_(_e: Event) { + private static bubbleMouseUp_(_e: MouseEvent) { Touch.clearTouchIdentifier(); Bubble.unbindDragEvents_(); } diff --git a/core/flyout_base.ts b/core/flyout_base.ts index fb21496d5..547dac15e 100644 --- a/core/flyout_base.ts +++ b/core/flyout_base.ts @@ -132,7 +132,7 @@ export abstract class Flyout extends DeleteArea implements IFlyout { /** * Opaque data that can be passed to Blockly.unbindEvent_. */ - private eventWrappers_: AnyDuringMigration[][] = []; + private eventWrappers_: browserEvents.Data = []; /** * Function that will be registered as a change listener on the workspace @@ -904,7 +904,7 @@ export abstract class Flyout extends DeleteArea implements IFlyout { */ private blockMouseDown_(block: BlockSvg): Function { const flyout = this; - return (e: Event) => { + return (e: MouseEvent) => { const gesture = flyout.targetWorkspace.getGesture(e); if (gesture) { gesture.setStartBlock(block); @@ -917,7 +917,7 @@ export abstract class Flyout extends DeleteArea implements IFlyout { * Mouse down on the flyout background. Start a vertical scroll drag. * @param e Mouse down event. */ - private onMouseDown_(e: Event) { + private onMouseDown_(e: MouseEvent) { const gesture = this.targetWorkspace.getGesture(e); if (gesture) { gesture.handleFlyoutStart(e, this); diff --git a/core/gesture.ts b/core/gesture.ts index 5d9ff2604..caf42ec78 100644 --- a/core/gesture.ts +++ b/core/gesture.ts @@ -417,7 +417,7 @@ export class Gesture { * @param e A mouse down or touch start event. * @internal */ - doStart(e: Event) { + doStart(e: MouseEvent) { if (browserEvents.isTargetInput(e)) { this.cancel(); return; @@ -603,7 +603,7 @@ export class Gesture { * @param ws The workspace the event hit. * @internal */ - handleWsStart(e: Event, ws: WorkspaceSvg) { + handleWsStart(e: MouseEvent, ws: WorkspaceSvg) { if (this.hasStarted_) { throw Error( 'Tried to call gesture.handleWsStart, ' + @@ -629,7 +629,7 @@ export class Gesture { * @param flyout The flyout the event hit. * @internal */ - handleFlyoutStart(e: Event, flyout: IFlyout) { + handleFlyoutStart(e: MouseEvent, flyout: IFlyout) { if (this.hasStarted_) { throw Error( 'Tried to call gesture.handleFlyoutStart, ' + diff --git a/core/icon.ts b/core/icon.ts index 6cf57f2a9..aec490590 100644 --- a/core/icon.ts +++ b/core/icon.ts @@ -110,7 +110,7 @@ export abstract class Icon { * Clicking on the icon toggles if the bubble is visible. * @param e Mouse click event. */ - protected iconClick_(e: Event) { + protected iconClick_(e: MouseEvent) { if (this.block_.workspace.isDragging()) { // Drag operation is concluding. Don't open the editor. return; diff --git a/core/mutator.ts b/core/mutator.ts index 881a382e2..de85081c4 100644 --- a/core/mutator.ts +++ b/core/mutator.ts @@ -143,7 +143,7 @@ export class Mutator extends Icon { * Disable if block is uneditable. * @param e Mouse click event. */ - protected override iconClick_(e: Event) { + protected override iconClick_(e: MouseEvent) { if (this.block_.isEditable()) { super.iconClick_(e); } diff --git a/core/scrollbar.ts b/core/scrollbar.ts index e2707e60f..a50501353 100644 --- a/core/scrollbar.ts +++ b/core/scrollbar.ts @@ -660,7 +660,7 @@ export class Scrollbar { * Called when scrollbar background is clicked. * @param e Mouse down event. */ - private onMouseDownBar_(e: Event) { + private onMouseDownBar_(e: MouseEvent) { this.workspace.markFocused(); Touch.clearTouchIdentifier(); // This is really a click. this.cleanUp_(); @@ -699,7 +699,7 @@ export class Scrollbar { * Called when scrollbar handle is clicked. * @param e Mouse down event. */ - private onMouseDownHandle_(e: Event) { + private onMouseDownHandle_(e: MouseEvent) { this.workspace.markFocused(); this.cleanUp_(); if (browserEvents.isRightButton(e)) { diff --git a/core/toolbox/toolbox.ts b/core/toolbox/toolbox.ts index d6ad4eda3..69fe4e863 100644 --- a/core/toolbox/toolbox.ts +++ b/core/toolbox/toolbox.ts @@ -243,7 +243,7 @@ export class Toolbox extends DeleteArea implements IAutoHideable, * Handles on click events for when the toolbox or toolbox items are clicked. * @param e Click event to handle. */ - protected onClick_(e: Event) { + protected onClick_(e: MouseEvent) { if (browserEvents.isRightButton(e) || e.target === this.HtmlDiv) { // Close flyout. (common.getMainWorkspace() as WorkspaceSvg).hideChaff(false); diff --git a/core/touch_gesture.ts b/core/touch_gesture.ts index cb7e502b6..21de9fcf1 100644 --- a/core/touch_gesture.ts +++ b/core/touch_gesture.ts @@ -85,7 +85,7 @@ export class TouchGesture extends Gesture { * @param e A mouse down, touch start or pointer down event. * @internal */ - override doStart(e: Event) { + override doStart(e: MouseEvent) { this.isPinchZoomEnabled_ = this.startWorkspace_.options.zoomOptions && this.startWorkspace_.options.zoomOptions.pinch; super.doStart(e); @@ -143,7 +143,7 @@ export class TouchGesture extends Gesture { * @param e A mouse move, touch move, or pointer move event. * @internal */ - override handleMove(e: Event) { + override handleMove(e: MouseEvent) { if (this.isDragging()) { // We are in the middle of a drag, only handle the relevant events if (Touch.shouldHandleEvent(e)) { @@ -233,7 +233,7 @@ export class TouchGesture extends Gesture { * @param e A touch move, or pointer move event. * @internal */ - handleTouchMove(e: Event) { + handleTouchMove(e: MouseEvent) { const pointerId = Touch.getTouchIdentifierFromEvent(e); // Update the cache // AnyDuringMigration because: Type 'Coordinate | null' is not assignable @@ -252,7 +252,7 @@ export class TouchGesture extends Gesture { * Handle pinch zoom gesture. * @param e A touch move, or pointer move event. */ - private handlePinch_(e: Event) { + private handlePinch_(e: MouseEvent) { const pointers = Object.keys(this.cachedPoints_); // Calculate the distance between the two pointers const point0 = (this.cachedPoints_[pointers[0]]); diff --git a/core/utils.ts b/core/utils.ts index a7768b784..6069bd2be 100644 --- a/core/utils.ts +++ b/core/utils.ts @@ -155,7 +155,7 @@ export function isRightButton(e: Event): boolean { .warn( 'Blockly.utils.isRightButton', 'September 2021', 'September 2022', 'Blockly.browserEvents.isRightButton'); - return browserEvents.isRightButton(e); + return browserEvents.isRightButton(e as MouseEvent); } /** @@ -175,7 +175,7 @@ export function mouseToSvg( .warn( 'Blockly.utils.mouseToSvg', 'September 2021', 'September 2022', 'Blockly.browserEvents.mouseToSvg'); - return browserEvents.mouseToSvg(e, svg, matrix); + return browserEvents.mouseToSvg(e as MouseEvent, svg, matrix); } /** diff --git a/core/workspace_comment_svg.ts b/core/workspace_comment_svg.ts index a7489500d..bae97f5cd 100644 --- a/core/workspace_comment_svg.ts +++ b/core/workspace_comment_svg.ts @@ -802,7 +802,7 @@ export class WorkspaceCommentSvg extends WorkspaceComment implements * Handle a mouse-down on comment's resize corner. * @param e Mouse down event. */ - private resizeMouseDown_(e: Event) { + private resizeMouseDown_(e: MouseEvent) { this.unbindDragEvents_(); if (browserEvents.isRightButton(e)) { // No right-click. @@ -882,7 +882,7 @@ export class WorkspaceCommentSvg extends WorkspaceComment implements * Resize this comment to follow the mouse. * @param e Mouse move event. */ - private resizeMouseMove_(e: Event) { + private resizeMouseMove_(e: MouseEvent) { this.autoLayout_ = false; const newXY = this.workspace.moveDrag(e); this.setSize_(this.RTL ? -newXY.x : newXY.x, newXY.y); diff --git a/core/workspace_svg.ts b/core/workspace_svg.ts index 032835e88..2ce258bf2 100644 --- a/core/workspace_svg.ts +++ b/core/workspace_svg.ts @@ -1700,7 +1700,7 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg { * Handle a mouse-down on SVG drawing surface. * @param e Mouse down event. */ - private onMouseDown_(e: Event) { + private onMouseDown_(e: MouseEvent) { const gesture = this.getGesture(e); if (gesture) { gesture.handleWsStart(e, this); @@ -1712,7 +1712,7 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg { * @param e Mouse down event. * @param xy Starting location of object. */ - startDrag(e: Event, xy: Coordinate) { + startDrag(e: MouseEvent, xy: Coordinate) { // Record the starting offset between the bubble's location and the mouse. const point = browserEvents.mouseToSvg( e, this.getParentSvg(), this.getInverseScreenCTM()); @@ -1727,7 +1727,7 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg { * @param e Mouse move event. * @return New location of object. */ - moveDrag(e: Event): Coordinate { + moveDrag(e: MouseEvent): Coordinate { const point = browserEvents.mouseToSvg( e, this.getParentSvg(), this.getInverseScreenCTM()); // Fix scale of mouse event.