fix: Fix more browser tests

tests/browser/test/delete_blocks_test.mjs

@@ -8,6 +8,7 @@ import * as chai from 'chai';
 import {Key} from 'webdriverio';
 import {
   clickBlock,
+  clickWorkspace,
   contextMenuSelect,
   getAllBlocks,
   getBlockElementById,
@@ -176,8 +177,7 @@ suite('Delete blocks', function (done) {
     );
   });
 
-  // TODO(#9029) enable this test once deleting a block doesn't lose focus
-  test.skip('Undo block deletion', async function () {
+  test('Undo block deletion', async function () {
     const before = (await getAllBlocks(this.browser)).length;
     // Get first print block, click to select it, and delete it using backspace key.
     await clickBlock(this.browser, this.firstBlock.id, {button: 1});
@@ -204,6 +204,7 @@ suite('Delete blocks', function (done) {
     await this.browser.keys([Key.Ctrl, 'z']);
     await this.browser.pause(PAUSE_TIME);
     // Redo
+    await clickWorkspace(this.browser);
     await this.browser.keys([Key.Ctrl, Key.Shift, 'z']);
     await this.browser.pause(PAUSE_TIME);
     const after = (await getAllBlocks(this.browser)).length;

tests/browser/test/extensive_test.mjs

@@ -40,8 +40,7 @@ suite('This tests loading Large Configuration and Deletion', function (done) {
     chai.assert.equal(allBlocks.length, 10);
   });
 
-  // TODO(#8793) Re-enable test after deleting a block updates focus correctly.
-  test.skip('undoing delete block results in the correct number of blocks', async function () {
+  test('undoing delete block results in the correct number of blocks', async function () {
     await this.browser.keys([Key.Ctrl, 'z']);
     await this.browser.pause(PAUSE_TIME);
     const allBlocks = await getAllBlocks(this.browser);

tests/browser/test/procedure_test.mjs

@@ -11,9 +11,8 @@
 import * as chai from 'chai';
 import {
   connect,
-  getBlockTypeFromCategory,
-  getNthBlockOfCategory,
-  getSelectedBlockElement,
+  dragBlockTypeFromFlyout,
+  dragNthBlockFromFlyout,
   PAUSE_TIME,
   testFileLocations,
   testSetup,
@@ -33,43 +32,41 @@ suite('Testing Connecting Blocks', function (done) {
 
   test('Testing Procedure', async function () {
     // Drag out first function
-    let proceduresDefReturn = await getBlockTypeFromCategory(
+    const doSomething = await dragBlockTypeFromFlyout(
       this.browser,
       'Functions',
       'procedures_defreturn',
+      50,
+      20,
     );
-    await proceduresDefReturn.dragAndDrop({x: 50, y: 20});
-    const doSomething = await getSelectedBlockElement(this.browser);
 
-    // Drag out second function.
-    proceduresDefReturn = await getBlockTypeFromCategory(
+    const doSomething2 = await dragBlockTypeFromFlyout(
       this.browser,
       'Functions',
       'procedures_defreturn',
+      50,
+      20,
     );
-    await proceduresDefReturn.dragAndDrop({x: 300, y: 200});
-    const doSomething2 = await getSelectedBlockElement(this.browser);
 
-    // Drag out numeric
-    const mathNumeric = await getBlockTypeFromCategory(
+    const numeric = await dragBlockTypeFromFlyout(
       this.browser,
       'Math',
       'math_number',
+      50,
+      20,
     );
-    await mathNumeric.dragAndDrop({x: 50, y: 20});
-    const numeric = await getSelectedBlockElement(this.browser);
 
     // Connect numeric to first procedure
     await connect(this.browser, numeric, 'OUTPUT', doSomething, 'RETURN');
 
     // Drag out doSomething caller from flyout.
-    const doSomethingFlyout = await getNthBlockOfCategory(
+    const doSomethingCaller = await dragNthBlockFromFlyout(
       this.browser,
       'Functions',
       3,
+      50,
+      20,
     );
-    await doSomethingFlyout.dragAndDrop({x: 50, y: 20});
-    const doSomethingCaller = await getSelectedBlockElement(this.browser);
 
     // Connect the doSomething caller to doSomething2
     await connect(
@@ -81,22 +78,22 @@ suite('Testing Connecting Blocks', function (done) {
     );
 
     // Drag out print from flyout.
-    const printFlyout = await getBlockTypeFromCategory(
+    const print = await dragBlockTypeFromFlyout(
       this.browser,
       'Text',
       'text_print',
+      50,
+      0,
     );
-    await printFlyout.dragAndDrop({x: 50, y: 20});
-    const print = await getSelectedBlockElement(this.browser);
 
     // Drag out doSomething2 caller from flyout.
-    const doSomething2Flyout = await getNthBlockOfCategory(
+    const doSomething2Caller = await dragNthBlockFromFlyout(
       this.browser,
       'Functions',
       4,
+      50,
+      20,
     );
-    await doSomething2Flyout.dragAndDrop({x: 130, y: 20});
-    const doSomething2Caller = await getSelectedBlockElement(this.browser);
 
     // Connect doSomething2 caller with print.
     await connect(this.browser, doSomething2Caller, 'OUTPUT', print, 'TEXT');

tests/browser/test/test_setup.mjs

@@ -62,6 +62,8 @@ export async function driverSetup() {
   // Use Selenium to bring up the page
   console.log('Starting webdriverio...');
   driver = await webdriverio.remote(options);
+  driver.setWindowSize(800, 600);
+  driver.setViewport({width: 800, height: 600});
   return driver;
 }
 
@@ -170,43 +172,52 @@ export async function getBlockElementById(browser, id) {
  * @return A Promise that resolves when the actions are completed.
  */
 export async function clickBlock(browser, blockId, clickOptions) {
-  const findableId = 'clickTargetElement';
   // In the browser context, find the element that we want and give it a findable ID.
-  await browser.execute(
-    (blockId, newElemId) => {
-      const block = Blockly.getMainWorkspace().getBlockById(blockId);
-      // Ensure the block we want to click is within the viewport.
-      Blockly.getMainWorkspace().scrollBoundsIntoView(
-        block.getBoundingRectangleWithoutChildren(),
-        10,
-      );
+  const elem = await getTargetableBlockElement(browser, blockId, false);
+  await elem.click(clickOptions);
+}
+
+/**
+ * Find an element on the block that is suitable for a click or drag.
+ *
+ * We can't always use the block's SVG root because clicking will always happen
+ * in the middle of the block's bounds (including children) by default, which
+ * causes problems if it has holes (e.g. statement inputs). Instead, this tries
+ * to get the first text field on the block. It falls back on the block's SVG root.
+ * @param browser The active WebdriverIO Browser object.
+ * @param blockId The id of the block to click, as an interactable element.
+ * @param toolbox True if this block is in the toolbox (which must be open already).
+ * @return A Promise that returns an appropriate element.
+ */
+async function getTargetableBlockElement(browser, blockId, toolbox) {
+  const id = await browser.execute(
+    (blockId, toolbox, newElemId) => {
+      const ws = toolbox
+        ? Blockly.getMainWorkspace().getFlyout().getWorkspace()
+        : Blockly.getMainWorkspace();
+      const block = ws.getBlockById(blockId);
+      // Ensure the block we want to click/drag is within the viewport.
+      ws.scrollBoundsIntoView(block.getBoundingRectangleWithoutChildren(), 10);
       if (!block.isCollapsed()) {
         for (const input of block.inputList) {
           for (const field of input.fieldRow) {
             if (field instanceof Blockly.FieldLabel) {
-              field.getSvgRoot().id = newElemId;
-              return;
+              // Expose the id of the element we want to target
+              field.getSvgRoot().setAttribute('data-id', field.id_);
+              return field.getSvgRoot().id;
             }
           }
         }
       }
-      // No label field found. Fall back to the block's SVG root.
-      block.getSvgRoot().id = newElemId;
+      // No label field found. Fall back to the block's SVG root, which should
+      // already use the block id.
+      return block.id;
     },
     blockId,
-    findableId,
+    toolbox,
   );
 
-  // In the test context, get the Webdriverio Element that we've identified.
-  const elem = await browser.$(`#${findableId}`);
-
-  await elem.click(clickOptions);
-
-  // In the browser context, remove the ID.
-  await browser.execute((elemId) => {
-    const clickElem = document.getElementById(elemId);
-    clickElem.removeAttribute('id');
-  }, findableId);
+  return await getBlockElementById(browser, id);
 }
 
 /**
@@ -215,7 +226,7 @@ export async function clickBlock(browser, blockId, clickOptions) {
  * @return A Promise that resolves when the actions are completed.
  */
 export async function clickWorkspace(browser) {
-  const workspace = await browser.$('#blocklyDiv > div > svg.blocklySvg > g');
+  const workspace = await browser.$('svg.blocklySvg > g');
   await workspace.click();
   await browser.pause(PAUSE_TIME);
 }
@@ -253,27 +264,14 @@ export async function getCategory(browser, categoryName) {
 }
 
 /**
- * @param browser The active WebdriverIO Browser object.
- * @param categoryName The name of the toolbox category to search.
- * @param n Which block to select, 0-indexed from the top of the category.
- * @return A Promise that resolves to the root element of the nth
- *     block in the given category.
- */
-export async function getNthBlockOfCategory(browser, categoryName, n) {
-  const category = await getCategory(browser, categoryName);
-  await category.click();
-  const block = (
-    await browser.$$(`.blocklyFlyout .blocklyBlockCanvas > .blocklyDraggable`)
-  )[n];
-  return block;
-}
-
-/**
+ * Opens the specified category, finds the first block of the given type,
+ * scrolls it into view, and returns a draggable element on that block.
+ *
  * @param browser The active WebdriverIO Browser object.
  * @param categoryName The name of the toolbox category to search.
  *     Null if the toolbox has no categories (simple).
  * @param blockType The type of the block to search for.
- * @return A Promise that resolves to the root element of the first
+ * @return A Promise that resolves to a draggable element of the first
  *     block with the given type in the given category.
  */
 export async function getBlockTypeFromCategory(
@@ -286,13 +284,14 @@ export async function getBlockTypeFromCategory(
     await category.click();
   }
 
+  await browser.pause(PAUSE_TIME);
   const id = await browser.execute((blockType) => {
-    return Blockly.getMainWorkspace()
-      .getFlyout()
-      .getWorkspace()
-      .getBlocksByType(blockType)[0].id;
+    const ws = Blockly.getMainWorkspace().getFlyout().getWorkspace();
+    const block = ws.getBlocksByType(blockType)[0];
+    ws.scrollBoundsIntoView(block.getBoundingRectangleWithoutChildren());
+    return block.id;
   }, blockType);
-  return getBlockElementById(browser, id);
+  return getTargetableBlockElement(browser, id, true);
 }
 
 /**
@@ -447,7 +446,16 @@ export async function switchRTL(browser) {
  *     created block.
  */
 export async function dragNthBlockFromFlyout(browser, categoryName, n, x, y) {
-  const flyoutBlock = await getNthBlockOfCategory(browser, categoryName, n);
+  const category = await getCategory(browser, categoryName);
+  await category.click();
+
+  await browser.pause(PAUSE_TIME);
+  const id = await browser.execute((n) => {
+    const ws = Blockly.getMainWorkspace().getFlyout().getWorkspace();
+    const block = ws.getTopBlocks(true)[n];
+    return block.id;
+  }, n);
+  const flyoutBlock = await getTargetableBlockElement(browser, id, true);
   await flyoutBlock.dragAndDrop({x: x, y: y});
   return await getSelectedBlockElement(browser);
 }
@@ -480,6 +488,7 @@ export async function dragBlockTypeFromFlyout(
     type,
   );
   await flyoutBlock.dragAndDrop({x: x, y: y});
+  await browser.pause(PAUSE_TIME);
   return await getSelectedBlockElement(browser);
 }
 
@@ -584,26 +593,3 @@ export async function getAllBlocks(browser) {
       }));
   });
 }
-
-/**
- * Find the flyout's scrollbar and scroll by the specified amount.
- * This makes several assumptions:
- *  - A flyout with a valid scrollbar exists, is open, and is in view.
- *  - The workspace has a trash can, which means it has a second (hidden) flyout.
- * @param browser The active WebdriverIO Browser object.
- * @param xDelta How far to drag the flyout in the x direction. Positive is right.
- * @param yDelta How far to drag the flyout in the y direction. Positive is down.
- * @return A Promise that resolves when the actions are completed.
- */
-export async function scrollFlyout(browser, xDelta, yDelta) {
-  // There are two flyouts on the playground workspace: one for the trash can
-  // and one for the toolbox. We want the second one.
-  // This assumes there is only one scrollbar handle in the flyout, but it could
-  // be either horizontal or vertical.
-  await browser.pause(PAUSE_TIME);
-  const scrollbarHandle = await browser
-    .$$(`.blocklyFlyoutScrollbar`)[1]
-    .$(`rect.blocklyScrollbarHandle`);
-  await scrollbarHandle.dragAndDrop({x: xDelta, y: yDelta});
-  await browser.pause(PAUSE_TIME);
-}