Merge pull request #8798 from google/develop-v12-merge

chore: Merge develop into rc/v12.0.0

.github/workflows/appengine_deploy.yml

@@ -42,7 +42,7 @@ jobs:
           path: _deploy/
 
       - name: Deploy to App Engine
-        uses: google-github-actions/deploy-appengine@v2.1.4
+        uses: google-github-actions/deploy-appengine@v2.1.5
         # For parameters see:
         # https://github.com/google-github-actions/deploy-appengine#inputs
         with:

core/block_animations.ts

@@ -176,7 +176,7 @@ export function disconnectUiEffect(block: BlockSvg) {
   }
   // Start the animation.
   wobblingBlock = block;
-  disconnectUiStep(block, magnitude, new Date());
+  disconnectUiStep(block, magnitude, new Date(), 0);
 }
 
 /**
@@ -184,22 +184,30 @@ export function disconnectUiEffect(block: BlockSvg) {
  *
  * @param block Block to animate.
  * @param magnitude Maximum degrees skew (reversed for RTL).
- * @param start Date of animation's start.
+ * @param start Date of animation's start for deciding when to stop.
+ * @param step Which step of the animation we're on.
  */
-function disconnectUiStep(block: BlockSvg, magnitude: number, start: Date) {
+function disconnectUiStep(
+  block: BlockSvg,
+  magnitude: number,
+  start: Date,
+  step: number,
+) {
   const DURATION = 200; // Milliseconds.
-  const WIGGLES = 3; // Half oscillations.
-
-  const ms = new Date().getTime() - start.getTime();
-  const percent = ms / DURATION;
+  const WIGGLES = [0.66, 1, 0.66, 0, -0.66, -1, -0.66, 0]; // Single cycle
 
   let skew = '';
-  if (percent <= 1) {
-    const val = Math.round(
-      Math.sin(percent * Math.PI * WIGGLES) * (1 - percent) * magnitude,
-    );
+  if (start.getTime() + DURATION > new Date().getTime()) {
+    const val = Math.round(WIGGLES[step % WIGGLES.length] * magnitude);
     skew = `skewX(${val})`;
-    disconnectPid = setTimeout(disconnectUiStep, 10, block, magnitude, start);
+    disconnectPid = setTimeout(
+      disconnectUiStep,
+      15,
+      block,
+      magnitude,
+      start,
+      step + 1,
+    );
   }
 
   block

core/bubbles/text_bubble.ts

@@ -16,7 +16,7 @@ import {Bubble} from './bubble.js';
  * A bubble that displays non-editable text. Used by the warning icon.
  */
 export class TextBubble extends Bubble {
-  private paragraph: SVGTextElement;
+  private paragraph: SVGGElement;
 
   constructor(
     private text: string,
@@ -49,43 +49,52 @@ export class TextBubble extends Bubble {
    */
   private stringToSvg(text: string, container: SVGGElement) {
     const paragraph = this.createParagraph(container);
-    const spans = this.createSpans(paragraph, text);
+    const fragments = this.createTextFragments(paragraph, text);
     if (this.workspace.RTL)
-      this.rightAlignSpans(paragraph.getBBox().width, spans);
+      this.rightAlignTextFragments(paragraph.getBBox().width, fragments);
     return paragraph;
   }
 
-  /** Creates the paragraph container for this bubble's view's spans. */
-  private createParagraph(container: SVGGElement): SVGTextElement {
+  /** Creates the paragraph container for this bubble's view's text fragments. */
+  private createParagraph(container: SVGGElement): SVGGElement {
     return dom.createSvgElement(
-      Svg.TEXT,
+      Svg.G,
       {
         'class': 'blocklyText blocklyBubbleText blocklyNoPointerEvents',
-        'y': Bubble.BORDER_WIDTH,
+        'transform': `translate(0,${Bubble.BORDER_WIDTH})`,
+        'style': `direction: ${this.workspace.RTL ? 'rtl' : 'ltr'}`,
       },
       container,
     );
   }
 
-  /** Creates the spans visualizing the text of this bubble. */
-  private createSpans(parent: SVGTextElement, text: string): SVGTSpanElement[] {
+  /** Creates the text fragments visualizing the text of this bubble. */
+  private createTextFragments(
+    parent: SVGGElement,
+    text: string,
+  ): SVGTextElement[] {
+    let lineNum = 1;
     return text.split('\n').map((line) => {
-      const tspan = dom.createSvgElement(
-        Svg.TSPAN,
-        {'dy': '1em', 'x': Bubble.BORDER_WIDTH},
+      const fragment = dom.createSvgElement(
+        Svg.TEXT,
+        {'y': `${lineNum}em`, 'x': Bubble.BORDER_WIDTH},
         parent,
       );
       const textNode = document.createTextNode(line);
-      tspan.appendChild(textNode);
-      return tspan;
+      fragment.appendChild(textNode);
+      lineNum += 1;
+      return fragment;
     });
   }
 
-  /** Right aligns the given spans. */
-  private rightAlignSpans(maxWidth: number, spans: SVGTSpanElement[]) {
-    for (const span of spans) {
-      span.setAttribute('text-anchor', 'end');
-      span.setAttribute('x', `${maxWidth + Bubble.BORDER_WIDTH}`);
+  /** Right aligns the given text fragments. */
+  private rightAlignTextFragments(
+    maxWidth: number,
+    fragments: SVGTextElement[],
+  ) {
+    for (const text of fragments) {
+      text.setAttribute('text-anchor', 'start');
+      text.setAttribute('x', `${maxWidth + Bubble.BORDER_WIDTH}`);
     }
   }
 

core/comments/rendered_workspace_comment.ts

@@ -213,8 +213,14 @@ export class RenderedWorkspaceComment
   private startGesture(e: PointerEvent) {
     const gesture = this.workspace.getGesture(e);
     if (gesture) {
-      gesture.handleCommentStart(e, this);
-      this.workspace.getLayerManager()?.append(this, layers.BLOCK);
+      if (browserEvents.isTargetInput(e)) {
+        // If the text area was the focus, don't allow this event to bubble up
+        // and steal focus away from the editor/comment.
+        e.stopPropagation();
+      } else {
+        gesture.handleCommentStart(e, this);
+        this.workspace.getLayerManager()?.append(this, layers.BLOCK);
+      }
       common.setSelected(this);
     }
   }

core/css.ts

@@ -321,6 +321,7 @@ input[type=number] {
 
 .blocklyScrollbarBackground {
   opacity: 0;
+  pointer-events: none;
 }
 
 .blocklyScrollbarHandle {

core/field_dropdown.ts

@@ -30,6 +30,7 @@ import {Coordinate} from './utils/coordinate.js';
 import * as dom from './utils/dom.js';
 import * as parsing from './utils/parsing.js';
 import * as utilsString from './utils/string.js';
+import * as style from './utils/style.js';
 import {Svg} from './utils/svg.js';
 
 /**
@@ -302,6 +303,11 @@ export class FieldDropdown extends Field<string> {
 
     if (this.selectedMenuItem) {
       this.menu_!.setHighlighted(this.selectedMenuItem);
+      style.scrollIntoContainerView(
+        this.selectedMenuItem.getElement()!,
+        dropDownDiv.getContentDiv(),
+        true,
+      );
     }
 
     this.applyColour();

core/inject.ts

@@ -77,6 +77,16 @@ export function inject(
   });
 
   browserEvents.conditionalBind(subContainer, 'keydown', null, onKeyDown);
+  browserEvents.conditionalBind(
+    dropDownDiv.getContentDiv(),
+    'keydown',
+    null,
+    onKeyDown,
+  );
+  const widgetContainer = WidgetDiv.getDiv();
+  if (widgetContainer) {
+    browserEvents.conditionalBind(widgetContainer, 'keydown', null, onKeyDown);
+  }
 
   return workspace;
 }

core/menu.ts

@@ -29,8 +29,8 @@ export class Menu {
   private readonly menuItems: Array<MenuItem | MenuSeparator> = [];
 
   /**
-   * Coordinates of the mousedown event that caused this menu to open. Used to
-   * prevent the consequent mouseup event due to a simple click from
+   * Coordinates of the pointerdown event that caused this menu to open. Used to
+   * prevent the consequent pointerup event due to a simple click from
    * activating a menu item immediately.
    */
   openingCoords: Coordinate | null = null;
@@ -41,17 +41,17 @@ export class Menu {
    */
   private highlightedItem: MenuItem | null = null;
 
-  /** Mouse over event data. */
-  private mouseOverHandler: browserEvents.Data | null = null;
+  /** Pointer over event data. */
+  private pointerMoveHandler: browserEvents.Data | null = null;
 
   /** Click event data. */
   private clickHandler: browserEvents.Data | null = null;
 
-  /** Mouse enter event data. */
-  private mouseEnterHandler: browserEvents.Data | null = null;
+  /** Pointer enter event data. */
+  private pointerEnterHandler: browserEvents.Data | null = null;
 
-  /** Mouse leave event data. */
-  private mouseLeaveHandler: browserEvents.Data | null = null;
+  /** Pointer leave event data. */
+  private pointerLeaveHandler: browserEvents.Data | null = null;
 
   /** Key down event data. */
   private onKeyDownHandler: browserEvents.Data | null = null;
@@ -97,11 +97,11 @@ export class Menu {
     }
 
     // Add event handlers.
-    this.mouseOverHandler = browserEvents.conditionalBind(
+    this.pointerMoveHandler = browserEvents.conditionalBind(
       element,
-      'pointerover',
+      'pointermove',
       this,
-      this.handleMouseOver,
+      this.handlePointerMove,
       true,
     );
     this.clickHandler = browserEvents.conditionalBind(
@@ -111,18 +111,18 @@ export class Menu {
       this.handleClick,
       true,
     );
-    this.mouseEnterHandler = browserEvents.conditionalBind(
+    this.pointerEnterHandler = browserEvents.conditionalBind(
       element,
       'pointerenter',
       this,
-      this.handleMouseEnter,
+      this.handlePointerEnter,
       true,
     );
-    this.mouseLeaveHandler = browserEvents.conditionalBind(
+    this.pointerLeaveHandler = browserEvents.conditionalBind(
       element,
       'pointerleave',
       this,
-      this.handleMouseLeave,
+      this.handlePointerLeave,
       true,
     );
     this.onKeyDownHandler = browserEvents.conditionalBind(
@@ -179,21 +179,21 @@ export class Menu {
   /** Dispose of this menu. */
   dispose() {
     // Remove event handlers.
-    if (this.mouseOverHandler) {
-      browserEvents.unbind(this.mouseOverHandler);
-      this.mouseOverHandler = null;
+    if (this.pointerMoveHandler) {
+      browserEvents.unbind(this.pointerMoveHandler);
+      this.pointerMoveHandler = null;
     }
     if (this.clickHandler) {
       browserEvents.unbind(this.clickHandler);
       this.clickHandler = null;
     }
-    if (this.mouseEnterHandler) {
-      browserEvents.unbind(this.mouseEnterHandler);
-      this.mouseEnterHandler = null;
+    if (this.pointerEnterHandler) {
+      browserEvents.unbind(this.pointerEnterHandler);
+      this.pointerEnterHandler = null;
     }
-    if (this.mouseLeaveHandler) {
-      browserEvents.unbind(this.mouseLeaveHandler);
-      this.mouseLeaveHandler = null;
+    if (this.pointerLeaveHandler) {
+      browserEvents.unbind(this.pointerLeaveHandler);
+      this.pointerLeaveHandler = null;
     }
     if (this.onKeyDownHandler) {
       browserEvents.unbind(this.onKeyDownHandler);
@@ -257,11 +257,13 @@ export class Menu {
       this.highlightedItem = item;
       // Bring the highlighted item into view. This has no effect if the menu is
       // not scrollable.
-      const el = this.getElement();
-      if (el) {
-        aria.setState(el, aria.State.ACTIVEDESCENDANT, item.getId());
-      }
-      item.getElement()?.scrollIntoView();
+      const menuElement = this.getElement();
+      const scrollingParent = menuElement?.parentElement;
+      const menuItemElement = item.getElement();
+      if (!scrollingParent || !menuItemElement) return;
+
+      style.scrollIntoContainerView(menuItemElement, scrollingParent);
+      aria.setState(menuElement, aria.State.ACTIVEDESCENDANT, item.getId());
     }
   }
 
@@ -321,14 +323,26 @@ export class Menu {
     }
   }
 
-  // Mouse events.
+  // Pointer events.
 
   /**
-   * Handles mouseover events. Highlight menuitems as the user hovers over them.
+   * Handles pointermove events. Highlight menu items as the user hovers over
+   * them.
    *
-   * @param e Mouse event to handle.
+   * @param e Pointer event to handle.
    */
-  private handleMouseOver(e: PointerEvent) {
+  private handlePointerMove(e: PointerEvent) {
+    // Check whether the pointer actually did move. Move events are triggered if
+    // the element underneath the pointer moves, even if the pointer itself has
+    // remained stationary. In the case where the pointer is hovering over
+    // the menu but the user is navigating through the list of items via the
+    // keyboard and causing items off the end of the menu to scroll into view,
+    // a pointermove event would be triggered due to the pointer now being over
+    // a new child, but we don't want to highlight the item that's now under the
+    // pointer.
+    const delta = Math.max(Math.abs(e.movementX), Math.abs(e.movementY));
+    if (delta === 0) return;
+
     const menuItem = this.getMenuItem(e.target as Element);
 
     if (menuItem) {
@@ -354,11 +368,11 @@ export class Menu {
     if (oldCoords && typeof e.clientX === 'number') {
       const newCoords = new Coordinate(e.clientX, e.clientY);
       if (Coordinate.distance(oldCoords, newCoords) < 1) {
-        // This menu was opened by a mousedown and we're handling the consequent
-        // click event. The coords haven't changed, meaning this was the same
-        // opening event. Don't do the usual behavior because the menu just
-        // popped up under the mouse and the user didn't mean to activate this
-        // item.
+        // This menu was opened by a pointerdown and we're handling the
+        // consequent click event. The coords haven't changed, meaning this was
+        // the same opening event. Don't do the usual behavior because the menu
+        // just popped up under the pointer and the user didn't mean to activate
+        // this item.
         return;
       }
     }
@@ -370,22 +384,21 @@ export class Menu {
   }
 
   /**
-   * Handles mouse enter events. Focus the element.
+   * Handles pointer enter events. Focus the element.
    *
-   * @param _e Mouse event to handle.
+   * @param _e Pointer event to handle.
    */
-  private handleMouseEnter(_e: PointerEvent) {
+  private handlePointerEnter(_e: PointerEvent) {
     this.focus();
   }
 
   /**
-   * Handles mouse leave events. Blur and clear highlight.
+   * Handles pointer leave events by clearing the active highlight.
    *
-   * @param _e Mouse event to handle.
+   * @param _e Pointer event to handle.
    */
-  private handleMouseLeave(_e: PointerEvent) {
+  private handlePointerLeave(_e: PointerEvent) {
     if (this.getElement()) {
-      this.blur();
       this.setHighlighted(null);
     }
   }

core/rendered_connection.ts

@@ -533,6 +533,21 @@ export class RenderedConnection extends Connection {
     childBlock.updateDisabled();
     childBlock.queueRender();
 
+    // If either block being connected was selected, visually un- and reselect
+    // it. This has the effect of moving the selection path to the end of the
+    // list of child nodes in the DOM. Since SVG z-order is determined by node
+    // order in the DOM, this works around an issue where the selection outline
+    // path could be partially obscured by a new block inserted after it in the
+    // DOM.
+    const selection = common.getSelected();
+    const selectedBlock =
+      (selection === parentBlock && parentBlock) ||
+      (selection === childBlock && childBlock);
+    if (selectedBlock) {
+      selectedBlock.removeSelect();
+      selectedBlock.addSelect();
+    }
+
     // The input the child block is connected to (if any).
     const parentInput = parentBlock.getInputWithBlock(childBlock);
     if (parentInput) {

core/renderers/zelos/constants.ts

@@ -876,11 +876,11 @@ export class ConstantProvider extends BaseConstantProvider {
       `}`,
 
       // Widget and Dropdown Div
-      `${selector}.blocklyWidgetDiv .goog-menuitem,`,
-      `${selector}.blocklyDropDownDiv .goog-menuitem {`,
+      `${selector}.blocklyWidgetDiv .blocklyMenuItem,`,
+      `${selector}.blocklyDropDownDiv .blocklyMenuItem {`,
       `font-family: ${this.FIELD_TEXT_FONTFAMILY};`,
       `}`,
-      `${selector}.blocklyDropDownDiv .goog-menuitem-content {`,
+      `${selector}.blocklyDropDownDiv .blocklyMenuItemContent {`,
       `color: #fff;`,
       `}`,
 

core/utils/style.ts

@@ -7,7 +7,6 @@
 // Former goog.module ID: Blockly.utils.style
 
 import {Coordinate} from './coordinate.js';
-import * as deprecation from './deprecation.js';
 import {Rect} from './rect.js';
 import {Size} from './size.js';
 
@@ -59,7 +58,6 @@ function getSizeInternal(element: Element): Size {
  * @returns Object with width/height properties.
  */
 function getSizeWithDisplay(element: Element): Size {
-  deprecation.warn(`Blockly.utils.style.getSizeWithDisplay()`, 'v11.2', 'v13');
   const offsetWidth = (element as HTMLElement).offsetWidth;
   const offsetHeight = (element as HTMLElement).offsetHeight;
   return new Size(offsetWidth, offsetHeight);
@@ -132,7 +130,6 @@ export function getViewportPageOffset(): Coordinate {
  * @returns The computed border widths.
  */
 export function getBorderBox(element: Element): Rect {
-  deprecation.warn(`Blockly.utils.style.getBorderBox()`, 'v11.2', 'v13');
   const left = parseFloat(getComputedStyle(element, 'borderLeftWidth'));
   const right = parseFloat(getComputedStyle(element, 'borderRightWidth'));
   const top = parseFloat(getComputedStyle(element, 'borderTopWidth'));
@@ -159,12 +156,6 @@ export function scrollIntoContainerView(
   container: Element,
   opt_center?: boolean,
 ) {
-  deprecation.warn(
-    `Blockly.utils.style.scrollIntoContainerView()`,
-    'v11.2',
-    'v13',
-    'the native Element.scrollIntoView()',
-  );
   const offset = getContainerOffsetToScrollInto(element, container, opt_center);
   container.scrollLeft = offset.x;
   container.scrollTop = offset.y;
@@ -189,11 +180,6 @@ export function getContainerOffsetToScrollInto(
   container: Element,
   opt_center?: boolean,
 ): Coordinate {
-  deprecation.warn(
-    `Blockly.utils.style.getContainerOffsetToScrollInto()`,
-    'v11.2',
-    'v13',
-  );
   // Absolute position of the element's border's top left corner.
   const elementPos = getPageOffset(element);
   // Absolute position of the container's border's top left corner.

generators/dart/logic.ts

@@ -46,7 +46,9 @@ export function controls_if(block: Block, generator: DartGenerator) {
   } while (block.getInput('IF' + n));
 
   if (block.getInput('ELSE') || generator.STATEMENT_SUFFIX) {
-    branchCode = generator.statementToCode(block, 'ELSE');
+    branchCode = block.getInput('ELSE')
+      ? generator.statementToCode(block, 'ELSE')
+      : '';
     if (generator.STATEMENT_SUFFIX) {
       branchCode =
         generator.prefixLines(

generators/javascript/logic.ts

@@ -44,7 +44,9 @@ export function controls_if(block: Block, generator: JavascriptGenerator) {
   } while (block.getInput('IF' + n));
 
   if (block.getInput('ELSE') || generator.STATEMENT_SUFFIX) {
-    let branchCode = generator.statementToCode(block, 'ELSE');
+    let branchCode = block.getInput('ELSE')
+      ? generator.statementToCode(block, 'ELSE')
+      : '';
     if (generator.STATEMENT_SUFFIX) {
       branchCode =
         generator.prefixLines(

generators/lua/logic.ts

@@ -39,7 +39,9 @@ export function controls_if(block: Block, generator: LuaGenerator): string {
   } while (block.getInput('IF' + n));
 
   if (block.getInput('ELSE') || generator.STATEMENT_SUFFIX) {
-    let branchCode = generator.statementToCode(block, 'ELSE');
+    let branchCode = block.getInput('ELSE')
+      ? generator.statementToCode(block, 'ELSE')
+      : '';
     if (generator.STATEMENT_SUFFIX) {
       branchCode =
         generator.prefixLines(

generators/php/logic.ts

@@ -46,7 +46,9 @@ export function controls_if(block: Block, generator: PhpGenerator) {
   } while (block.getInput('IF' + n));
 
   if (block.getInput('ELSE') || generator.STATEMENT_SUFFIX) {
-    branchCode = generator.statementToCode(block, 'ELSE');
+    branchCode = block.getInput('ELSE')
+      ? generator.statementToCode(block, 'ELSE')
+      : '';
     if (generator.STATEMENT_SUFFIX) {
       branchCode =
         generator.prefixLines(

generators/python/logic.ts

@@ -40,7 +40,11 @@ export function controls_if(block: Block, generator: PythonGenerator) {
   } while (block.getInput('IF' + n));
 
   if (block.getInput('ELSE') || generator.STATEMENT_SUFFIX) {
-    branchCode = generator.statementToCode(block, 'ELSE') || generator.PASS;
+    if (block.getInput('ELSE')) {
+      branchCode = generator.statementToCode(block, 'ELSE') || generator.PASS;
+    } else {
+      branchCode = generator.PASS;
+    }
     if (generator.STATEMENT_SUFFIX) {
       branchCode =
         generator.prefixLines(

package.json

@@ -115,7 +115,7 @@
     "eslint-config-prettier": "^9.1.0",
     "eslint-plugin-jsdoc": "^50.5.0",
     "eslint-plugin-prettier": "^5.2.1",
-    "glob": "^10.3.4",
+    "glob": "^11.0.1",
     "globals": "^15.12.0",
     "google-closure-compiler": "^20240317.0.0",
     "gulp": "^5.0.0",

tests/browser/test/basic_playground_test.mjs

@@ -137,7 +137,7 @@ suite('Disabling', function () {
         110,
       );
       await connect(this.browser, child, 'OUTPUT', parent, 'IF0');
-
+      await this.browser.pause(PAUSE_TIME);
       await contextMenuSelect(this.browser, parent, 'Disable Block');
 
       chai.assert.isTrue(await getIsDisabled(this.browser, child.id));

tests/browser/test/delete_blocks_test.mjs

@@ -123,10 +123,14 @@ suite('Delete blocks', function (done) {
       )
       .waitForExist({timeout: 2000, reverse: true});
 
-    // Load the start blocks
-    await this.browser.execute((blocks) => {
-      Blockly.serialization.workspaces.load(blocks, Blockly.getMainWorkspace());
-    }, startBlocks);
+    // Load the start blocks. This hangs indefinitely if `startBlocks` is
+    // passed without being stringified.
+    this.browser.execute((blocks) => {
+      Blockly.serialization.workspaces.load(
+        JSON.parse(blocks),
+        Blockly.getMainWorkspace(),
+      );
+    }, JSON.stringify(startBlocks));
     // Wait for there to be a block on the main workspace before continuing
     (await getBlockElementById(this.browser, firstBlockId)).waitForExist({
       timeout: 2000,

tests/browser/test/procedure_test.mjs

@@ -26,6 +26,9 @@ suite('Testing Connecting Blocks', function (done) {
   // Setup Selenium for all of the tests
   suiteSetup(async function () {
     this.browser = await testSetup(testFileLocations.CODE_DEMO);
+    // Prevent WebDriver from suppressing alerts
+    // https://github.com/webdriverio/webdriverio/issues/13610#issuecomment-2357768103
+    this.browser.on('dialog', (dialog) => {});
   });
 
   test('Testing Procedure', async function () {

tests/browser/test/test_setup.mjs

@@ -38,6 +38,7 @@ export async function driverSetup() {
   const options = {
     capabilities: {
       'browserName': 'chrome',
+      'unhandledPromptBehavior': 'ignore',
       'goog:chromeOptions': {
         args: ['--allow-file-access-from-files'],
       },
@@ -254,9 +255,9 @@ export async function getCategory(browser, categoryName) {
 export async function getNthBlockOfCategory(browser, categoryName, n) {
   const category = await getCategory(browser, categoryName);
   await category.click();
-  const block = await browser.$(
-    `.blocklyFlyout .blocklyBlockCanvas > g:nth-child(${3 + n * 2})`,
-  );
+  const block = (
+    await browser.$$(`.blocklyFlyout .blocklyBlockCanvas > .blocklyDraggable`)
+  )[n];
   return block;
 }
 

tests/browser/test/workspace_comment_test.mjs

@@ -5,7 +5,6 @@
  */
 
 import * as chai from 'chai';
-import * as sinon from 'sinon';
 import {testFileLocations, testSetup} from './test_setup.mjs';
 
 suite('Workspace comments', function () {
@@ -20,8 +19,6 @@ suite('Workspace comments', function () {
   });
 
   teardown(async function () {
-    sinon.restore();
-
     await this.browser.execute(() => {
       Blockly.getMainWorkspace().clear();
     });

tests/mocha/connection_checker_test.js

@@ -503,8 +503,9 @@ suite('Connection checker', function () {
       </xml>`),
           this.workspace,
         );
-        [this.blockA, this.blockB, this.blockC] =
-          this.workspace.getAllBlocks(true);
+        this.blockA = this.workspace.getBlockById('A');
+        this.blockB = this.workspace.getBlockById('B');
+        this.blockC = this.workspace.getBlockById('C');
         this.checker = this.workspace.connectionChecker;
       });
 

tests/mocha/workspace_svg_test.js

@@ -408,6 +408,53 @@ suite('WorkspaceSvg', function () {
   });
 
   suite('cleanUp', function () {
+    assert.blockIsAtOrigin = function (actual, message) {
+      assert.blockHasPosition(actual, 0, 0, message || 'block is at origin');
+    };
+
+    assert.blockHasPositionX = function (actual, expectedX, message) {
+      const position = actual.getRelativeToSurfaceXY();
+      message = message || 'block has x value of ' + expectedX;
+      assert.equal(position.x, expectedX, message);
+    };
+
+    assert.blockHasPositionY = function (actual, expectedY, message) {
+      const position = actual.getRelativeToSurfaceXY();
+      message = message || 'block has y value of ' + expectedY;
+      assert.equal(position.y, expectedY, message);
+    };
+
+    assert.blockHasPosition = function (actual, expectedX, expectedY, message) {
+      assert.blockHasPositionX(actual, expectedX, message);
+      assert.blockHasPositionY(actual, expectedY, message);
+    };
+
+    assert.blockIsAtNotOrigin = function (actual, message) {
+      const position = actual.getRelativeToSurfaceXY();
+      message = message || 'block is not at origin';
+      assert.isTrue(position.x != 0 || position.y != 0, message);
+    };
+
+    assert.blocksDoNotIntersect = function (a, b, message) {
+      const rectA = a.getBoundingRectangle();
+      const rectB = b.getBoundingRectangle();
+      assert.isFalse(rectA.intersects(rectB), message || "a,b don't intersect");
+    };
+
+    assert.blockIsAbove = function (a, b, message) {
+      // Block a is above b iff a's bottom extreme is < b's top extreme.
+      const rectA = a.getBoundingRectangle();
+      const rectB = b.getBoundingRectangle();
+      assert.isBelow(rectA.bottom, rectB.top, message || 'a is above b');
+    };
+
+    assert.blockIsBelow = function (a, b, message) {
+      // Block a is below b iff a's top extreme is > b's bottom extreme.
+      const rectA = a.getBoundingRectangle();
+      const rectB = b.getBoundingRectangle();
+      assert.isAbove(rectA.top, rectB.bottom, message || 'a is below b');
+    };
+
     test('empty workspace does not change', function () {
       this.workspace.cleanUp();
 
@@ -429,13 +476,8 @@ suite('WorkspaceSvg', function () {
       this.workspace.cleanUp();
 
       const blocks = this.workspace.getTopBlocks(true);
-      const origin = new Blockly.utils.Coordinate(0, 0);
       assert.equal(blocks.length, 1, 'workspace has one top-level block');
-      assert.deepEqual(
-        blocks[0].getRelativeToSurfaceXY(),
-        origin,
-        'block is at origin',
-      );
+      assert.blockIsAtOrigin(blocks[0]);
     });
 
     test('single block at (10, 15) is moved to (0, 0)', function () {
@@ -453,14 +495,9 @@ suite('WorkspaceSvg', function () {
 
       const topBlocks = this.workspace.getTopBlocks(true);
       const allBlocks = this.workspace.getAllBlocks(false);
-      const origin = new Blockly.utils.Coordinate(0, 0);
       assert.equal(topBlocks.length, 1, 'workspace has one top-level block');
       assert.equal(allBlocks.length, 1, 'workspace has one block overall');
-      assert.deepEqual(
-        topBlocks[0].getRelativeToSurfaceXY(),
-        origin,
-        'block is at origin',
-      );
+      assert.blockIsAtOrigin(topBlocks[0]);
     });
 
     test('single block at (10, 15) with child is moved as unit to (0, 0)', function () {
@@ -487,19 +524,10 @@ suite('WorkspaceSvg', function () {
 
       const topBlocks = this.workspace.getTopBlocks(true);
       const allBlocks = this.workspace.getAllBlocks(false);
-      const origin = new Blockly.utils.Coordinate(0, 0);
       assert.equal(topBlocks.length, 1, 'workspace has one top-level block');
       assert.equal(allBlocks.length, 2, 'workspace has two blocks overall');
-      assert.deepEqual(
-        topBlocks[0].getRelativeToSurfaceXY(),
-        origin,
-        'block is at origin',
-      );
-      assert.notDeepEqual(
-        allBlocks[1].getRelativeToSurfaceXY(),
-        origin,
-        'child is not at origin',
-      );
+      assert.blockIsAtOrigin(topBlocks[0]); // Parent block.
+      assert.blockIsAtNotOrigin(allBlocks[1]); // Child block.
     });
 
     // TODO(#8676): Reenable once test passes reliably.
@@ -524,19 +552,9 @@ suite('WorkspaceSvg', function () {
       const topBlocks = this.workspace.getTopBlocks(true);
       const block1 = this.workspace.getBlockById('block1');
       const block2 = this.workspace.getBlockById('block2');
-      const origin = new Blockly.utils.Coordinate(0, 0);
-      const belowBlock2 = new Blockly.utils.Coordinate(0, 50);
       assert.equal(topBlocks.length, 2, 'workspace has two top-level blocks');
-      assert.deepEqual(
-        block2.getRelativeToSurfaceXY(),
-        origin,
-        'block2 is at origin',
-      );
-      assert.deepEqual(
-        block1.getRelativeToSurfaceXY(),
-        belowBlock2,
-        'block1 is below block2',
-      );
+      assert.blockIsAtOrigin(block2);
+      assert.blockIsBelow(block1, block2);
     });
 
     // TODO(#8676): Reenable once test passes reliably.
@@ -564,19 +582,9 @@ suite('WorkspaceSvg', function () {
       const topBlocks = this.workspace.getTopBlocks(true);
       const block1 = this.workspace.getBlockById('block1');
       const block2 = this.workspace.getBlockById('block2');
-      const origin = new Blockly.utils.Coordinate(0, 0);
-      const belowBlock1 = new Blockly.utils.Coordinate(0, 50);
       assert.equal(topBlocks.length, 2, 'workspace has two top-level blocks');
-      assert.deepEqual(
-        block1.getRelativeToSurfaceXY(),
-        origin,
-        'block1 is at origin',
-      );
-      assert.deepEqual(
-        block2.getRelativeToSurfaceXY(),
-        belowBlock1,
-        'block2 is below block1',
-      );
+      assert.blockIsAtOrigin(block1);
+      assert.blockIsBelow(block2, block1);
     });
 
     test('two overlapping blocks with snapping are moved to grid-aligned positions', function () {
@@ -605,19 +613,9 @@ suite('WorkspaceSvg', function () {
       const topBlocks = this.workspace.getTopBlocks(true);
       const block1 = this.workspace.getBlockById('block1');
       const block2 = this.workspace.getBlockById('block2');
-      const snappedOffOrigin = new Blockly.utils.Coordinate(10, 10);
-      const belowBlock1 = new Blockly.utils.Coordinate(10, 70);
       assert.equal(topBlocks.length, 2, 'workspace has two top-level blocks');
-      assert.deepEqual(
-        block1.getRelativeToSurfaceXY(),
-        snappedOffOrigin,
-        'block1 is near origin',
-      );
-      assert.deepEqual(
-        block2.getRelativeToSurfaceXY(),
-        belowBlock1,
-        'block2 is below block1',
-      );
+      assert.blockHasPosition(block1, 10, 10, 'block1 is at snapped origin');
+      assert.blockIsBelow(block2, block1);
     });
 
     // TODO(#8676): Reenable once test passes reliably.
@@ -653,36 +651,28 @@ suite('WorkspaceSvg', function () {
       const allBlocks = this.workspace.getAllBlocks(false);
       const block1 = this.workspace.getBlockById('block1');
       const block2 = this.workspace.getBlockById('block2');
-      const origin = new Blockly.utils.Coordinate(0, 0);
-      const belowBlock1 = new Blockly.utils.Coordinate(0, 50);
-      const block1Pos = block1.getRelativeToSurfaceXY();
-      const block2Pos = block2.getRelativeToSurfaceXY();
-      const block1ChildPos = block1.getChildren()[0].getRelativeToSurfaceXY();
-      const block2ChildPos = block2.getChildren()[0].getRelativeToSurfaceXY();
+      const block1Child = block1.getChildren()[0];
+      const block2Child = block2.getChildren()[0];
+
+      // Note that the x position tests below are verifying that each block's
+      // child isn't exactly aligned with it (however, they does overlap since
+      // the child block has an input connection with its parent).
       assert.equal(topBlocks.length, 2, 'workspace has two top-level block2');
       assert.equal(allBlocks.length, 4, 'workspace has four blocks overall');
-      assert.deepEqual(block1Pos, origin, 'block1 is at origin');
-      assert.deepEqual(block2Pos, belowBlock1, 'block2 is below block1');
+      assert.blockIsAtOrigin(block1);
+      assert.blockIsBelow(block2, block1);
       assert.isAbove(
-        block1ChildPos.x,
-        block1Pos.x,
-        "block1's child is right of it",
-      );
-      assert.isBelow(
-        block1ChildPos.y,
-        block2Pos.y,
-        "block1's child is above block 2",
+        block1.getChildren()[0].getRelativeToSurfaceXY().x,
+        block1.getRelativeToSurfaceXY().x,
+        "block1's child is right of its start",
       );
+      assert.blockIsAbove(block1Child, block2);
       assert.isAbove(
-        block2ChildPos.x,
-        block2Pos.x,
-        "block2's child is right of it",
-      );
-      assert.isAbove(
-        block2ChildPos.y,
-        block1Pos.y,
-        "block2's child is below block 1",
+        block2.getChildren()[0].getRelativeToSurfaceXY().x,
+        block2.getRelativeToSurfaceXY().x,
+        "block2's child is right of its start",
       );
+      assert.blockIsBelow(block2Child, block1);
     });
 
     // TODO(#8676): Reenable once test passes reliably.
@@ -742,19 +732,9 @@ suite('WorkspaceSvg', function () {
       const topBlocks = this.workspace.getTopBlocks(true);
       const block1 = this.workspace.getBlockById('block1');
       const block2 = this.workspace.getBlockById('block2');
-      const origin = new Blockly.utils.Coordinate(0, 0);
-      const belowBlock1 = new Blockly.utils.Coordinate(0, 144);
       assert.equal(topBlocks.length, 2, 'workspace has two top-level blocks');
-      assert.deepEqual(
-        block1.getRelativeToSurfaceXY(),
-        origin,
-        'block1 is at origin',
-      );
-      assert.deepEqual(
-        block2.getRelativeToSurfaceXY(),
-        belowBlock1,
-        'block2 is below block1',
-      );
+      assert.blockIsAtOrigin(block1);
+      assert.blockIsBelow(block2, block1);
     });
 
     test('five overlapping blocks are moved in-order as one column', function () {
@@ -780,32 +760,21 @@ suite('WorkspaceSvg', function () {
       this.workspace.cleanUp();
 
       const topBlocks = this.workspace.getTopBlocks(true);
-      const block1Pos = this.workspace
-        .getBlockById('block1')
-        .getRelativeToSurfaceXY();
-      const block2Pos = this.workspace
-        .getBlockById('block2')
-        .getRelativeToSurfaceXY();
-      const block3Pos = this.workspace
-        .getBlockById('block3')
-        .getRelativeToSurfaceXY();
-      const block4Pos = this.workspace
-        .getBlockById('block4')
-        .getRelativeToSurfaceXY();
-      const block5Pos = this.workspace
-        .getBlockById('block5')
-        .getRelativeToSurfaceXY();
-      const origin = new Blockly.utils.Coordinate(0, 0);
+      const block1 = this.workspace.getBlockById('block1');
+      const block2 = this.workspace.getBlockById('block2');
+      const block3 = this.workspace.getBlockById('block3');
+      const block4 = this.workspace.getBlockById('block4');
+      const block5 = this.workspace.getBlockById('block5');
       assert.equal(topBlocks.length, 5, 'workspace has five top-level blocks');
-      assert.deepEqual(block1Pos, origin, 'block1 is at origin');
-      assert.equal(block2Pos.x, 0, 'block2.x is at 0');
-      assert.equal(block3Pos.x, 0, 'block3.x is at 0');
-      assert.equal(block4Pos.x, 0, 'block4.x is at 0');
-      assert.equal(block5Pos.x, 0, 'block5.x is at 0');
-      assert.isAbove(block2Pos.y, block1Pos.y, 'block2 is below block1');
-      assert.isAbove(block3Pos.y, block2Pos.y, 'block3 is below block2');
-      assert.isAbove(block4Pos.y, block3Pos.y, 'block4 is below block3');
-      assert.isAbove(block5Pos.y, block4Pos.y, 'block5 is below block4');
+      assert.blockIsAtOrigin(block1);
+      assert.blockHasPositionX(block2, 0);
+      assert.blockHasPositionX(block3, 0);
+      assert.blockHasPositionX(block4, 0);
+      assert.blockHasPositionX(block5, 0);
+      assert.blockIsBelow(block2, block1);
+      assert.blockIsBelow(block3, block2);
+      assert.blockIsBelow(block4, block3);
+      assert.blockIsBelow(block5, block4);
     });
 
     test('single immovable block at (10, 15) is not moved', function () {
@@ -824,14 +793,9 @@ suite('WorkspaceSvg', function () {
 
       const topBlocks = this.workspace.getTopBlocks(true);
       const allBlocks = this.workspace.getAllBlocks(false);
-      const origPos = new Blockly.utils.Coordinate(10, 15);
       assert.equal(topBlocks.length, 1, 'workspace has one top-level block');
       assert.equal(allBlocks.length, 1, 'workspace has one block overall');
-      assert.deepEqual(
-        topBlocks[0].getRelativeToSurfaceXY(),
-        origPos,
-        'block is at (10, 15)',
-      );
+      assert.blockHasPosition(topBlocks[0], 10, 15);
     });
 
     test('multiple block types immovable blocks are not moved', function () {
@@ -914,53 +878,29 @@ suite('WorkspaceSvg', function () {
       this.workspace.cleanUp();
 
       const topBlocks = this.workspace.getTopBlocks(true);
-      const block1Rect = this.workspace
-        .getBlockById('block1')
-        .getBoundingRectangle();
-      const block2Rect = this.workspace
-        .getBlockById('block2')
-        .getBoundingRectangle();
-      const block3Rect = this.workspace
-        .getBlockById('block3')
-        .getBoundingRectangle();
-      const block4Rect = this.workspace
-        .getBlockById('block4')
-        .getBoundingRectangle();
-      const block5Rect = this.workspace
-        .getBlockById('block5')
-        .getBoundingRectangle();
+      const block1 = this.workspace.getBlockById('block1');
+      const block2 = this.workspace.getBlockById('block2');
+      const block3 = this.workspace.getBlockById('block3');
+      const block4 = this.workspace.getBlockById('block4');
+      const block5 = this.workspace.getBlockById('block5');
       assert.equal(topBlocks.length, 5, 'workspace has five top-level blocks');
       // Check that immovable blocks haven't moved.
-      assert.equal(block2Rect.left, 10, 'block2.x is at 10');
-      assert.equal(block2Rect.top, 20, 'block2.y is at 20');
-      assert.equal(block5Rect.left, 20, 'block5.x is at 20');
-      assert.equal(block5Rect.top, 200, 'block5.y is at 200');
+      assert.blockHasPosition(block2, 10, 20);
+      assert.blockHasPosition(block5, 20, 200);
       // Check that movable positions have correctly been left-aligned.
-      assert.equal(block1Rect.left, 0, 'block1.x is at 0');
-      assert.equal(block3Rect.left, 0, 'block3.x is at 0');
-      assert.equal(block4Rect.left, 0, 'block4.x is at 0');
+      assert.blockHasPositionX(block1, 0);
+      assert.blockHasPositionX(block3, 0);
+      assert.blockHasPositionX(block4, 0);
       // Block order should be: 2, 1, 3, 5, 4 since 2 and 5 are immovable.
-      assert.isAbove(block1Rect.top, block2Rect.top, 'block1 is below block2');
-      assert.isAbove(block3Rect.top, block1Rect.top, 'block3 is below block1');
-      assert.isAbove(block5Rect.top, block3Rect.top, 'block5 is below block3');
-      assert.isAbove(block4Rect.top, block5Rect.top, 'block4 is below block5');
+      assert.blockIsBelow(block1, block2);
+      assert.blockIsBelow(block3, block1);
+      assert.blockIsBelow(block5, block3);
+      assert.blockIsBelow(block4, block5);
       // Ensure no blocks intersect (can check in order due to the position verification above).
-      assert.isFalse(
-        block2Rect.intersects(block1Rect),
-        'block2/block1 do not intersect',
-      );
-      assert.isFalse(
-        block1Rect.intersects(block3Rect),
-        'block1/block3 do not intersect',
-      );
-      assert.isFalse(
-        block3Rect.intersects(block5Rect),
-        'block3/block5 do not intersect',
-      );
-      assert.isFalse(
-        block5Rect.intersects(block4Rect),
-        'block5/block4 do not intersect',
-      );
+      assert.blocksDoNotIntersect(block2, block1);
+      assert.blocksDoNotIntersect(block1, block3);
+      assert.blocksDoNotIntersect(block3, block5);
+      assert.blocksDoNotIntersect(block5, block4);
     });
   });