fix: Fix bug that caused inadvertent scrolling when the WidgetDiv was shown. (#9291)

* fix: Fix bug that caused inadvertent scrolling when the `WidgetDiv` was shown.

* chore: Add test to verify that displaying the context menu does not scroll the page.

* chore: Clarify comments.

* fix: Remove errant `.only`.

* chore: Add test to verify that actively focusing a node does not scroll the page.

* fix: Remove inadvertent `.only`.

Author: gonfunko

core/focus_manager.ts

@@ -309,6 +309,8 @@ export class FocusManager {
    * Note that this may update the specified node's element's tabindex to ensure
    * that it can be properly read out by screenreaders while focused.
    *
+   * The focused node will not be automatically scrolled into view.
+   *
    * @param focusableNode The node that should receive active focus.
    */
   focusNode(focusableNode: IFocusableNode): void {
@@ -423,6 +425,8 @@ export class FocusManager {
    * the returned lambda is called. Additionally, only 1 ephemeral focus context
    * can be active at any given time (attempting to activate more than one
    * simultaneously will result in an error being thrown).
+   *
+   * This method does not scroll the ephemerally focused element into view.
    */
   takeEphemeralFocus(
     focusableElement: HTMLElement | SVGElement,
@@ -439,7 +443,7 @@ export class FocusManager {
     if (this.focusedNode) {
       this.passivelyFocusNode(this.focusedNode, null);
     }
-    focusableElement.focus();
+    focusableElement.focus({preventScroll: true});
 
     let hasFinishedEphemeralFocus = false;
     return () => {
@@ -574,7 +578,7 @@ export class FocusManager {
     }
 
     this.setNodeToVisualActiveFocus(node);
-    elem.focus();
+    elem.focus({preventScroll: true});
   }
 
   /**

core/interfaces/i_focusable_node.ts

@@ -59,6 +59,9 @@ export interface IFocusableNode {
    * they should avoid the following:
    * - Creating or removing DOM elements (including via the renderer or drawer).
    * - Affecting focus via DOM focus() calls or the FocusManager.
+   *
+   * Implementations may consider scrolling themselves into view here; that is
+   * not handled by the focus manager.
    */
   onNodeFocus(): void;
 

tests/browser/test/basic_playground_test.mjs

@@ -101,6 +101,45 @@ suite('Right Clicking on Blocks', function () {
     await contextMenuSelect(this.browser, this.block, 'Remove Comment');
     chai.assert.isNull(await getCommentText(this.browser, this.block.id));
   });
+
+  test('does not scroll the page when node is ephemerally focused', async function () {
+    const initialScroll = await this.browser.execute(() => {
+      return window.scrollY;
+    });
+    // This left-right-left sequence was necessary to reproduce unintended
+    // scrolling; regardless of the number of clicks/context menu activations,
+    // the page should not scroll.
+    this.block.click({button: 2});
+    this.block.click({button: 0});
+    this.block.click({button: 2});
+    await this.browser.pause(250);
+    const finalScroll = await this.browser.execute(() => {
+      return window.scrollY;
+    });
+
+    chai.assert.equal(initialScroll, finalScroll);
+  });
+
+  test('does not scroll the page when node is actively focused', async function () {
+    await this.browser.setWindowSize(500, 300);
+    await this.browser.setViewport({width: 500, height: 300});
+    const initialScroll = await this.browser.execute((blockId) => {
+      window.scrollTo(0, document.body.scrollHeight);
+      return window.scrollY;
+    }, this.block.id);
+    await this.browser.execute(() => {
+      Blockly.getFocusManager().focusNode(
+        Blockly.getMainWorkspace().getToolbox(),
+      );
+    });
+    const finalScroll = await this.browser.execute(() => {
+      return window.scrollY;
+    });
+
+    chai.assert.equal(initialScroll, finalScroll);
+    await this.browser.setWindowSize(800, 600);
+    await this.browser.setViewport({width: 800, height: 600});
+  });
 });
 
 suite('Disabling', function () {