diff --git a/packages/blockly/core/constants.ts b/packages/blockly/core/constants.ts index 538bd378300..7eccc2658e3 100644 --- a/packages/blockly/core/constants.ts +++ b/packages/blockly/core/constants.ts @@ -21,3 +21,9 @@ export const COLLAPSED_FIELD_NAME = '_TEMP_COLLAPSED_FIELD'; * because the user manually disabled it, such as via the context menu. */ export const MANUALLY_DISABLED = 'MANUALLY_DISABLED'; + +/** + * The language-neutral ID for when the reason why a block is disabled is + * because it is not connected to a valid parent block. + */ +export const ORPHANED_BLOCK_DISABLED_REASON = 'ORPHANED_BLOCK'; diff --git a/packages/blockly/core/events/utils.ts b/packages/blockly/core/events/utils.ts index ac78c694273..6b104fdda8f 100644 --- a/packages/blockly/core/events/utils.ts +++ b/packages/blockly/core/events/utils.ts @@ -8,6 +8,7 @@ import type {Block} from '../block.js'; import * as common from '../common.js'; +import * as constants from '../constants.js'; import * as registry from '../registry.js'; import * as idGenerator from '../utils/idgenerator.js'; import type {Workspace} from '../workspace.js'; @@ -58,7 +59,7 @@ let disabled = 0; * The language-neutral ID for when the reason why a block is disabled is * because the block is not descended from a root block. */ -const ORPHANED_BLOCK_DISABLED_REASON = 'ORPHANED_BLOCK'; +const ORPHANED_BLOCK_DISABLED_REASON = constants.ORPHANED_BLOCK_DISABLED_REASON; /** * Type of events that cause objects to be bumped back into the visible diff --git a/packages/blockly/core/xml.ts b/packages/blockly/core/xml.ts index 3c06a39cca9..5a497b11047 100644 --- a/packages/blockly/core/xml.ts +++ b/packages/blockly/core/xml.ts @@ -11,7 +11,10 @@ import type {BlockSvg} from './block_svg.js'; import {RenderedWorkspaceComment} from './comments/rendered_workspace_comment.js'; import {WorkspaceComment} from './comments/workspace_comment.js'; import type {Connection} from './connection.js'; -import {MANUALLY_DISABLED} from './constants.js'; +import { + MANUALLY_DISABLED, + ORPHANED_BLOCK_DISABLED_REASON, +} from './constants.js'; import {EventType} from './events/type.js'; import * as eventUtils from './events/utils.js'; import type {Field} from './field.js'; @@ -473,7 +476,12 @@ export function domToWorkspace(xml: Element, workspace: Workspace): string[] { // Allow top-level shadow blocks if recordUndo is disabled since // that means an undo is in progress. Such a block is expected // to be moved to a nested destination in the next operation. - const block = domToBlockInternal(xmlChildElement, workspace); + const recoveredBlocks: RecoveredBlockInfo[] = []; + const block = domToBlockInternal( + xmlChildElement, + workspace, + recoveredBlocks, + ); newBlockIds.push(block.id); const blockX = parseInt(xmlChildElement.getAttribute('x') ?? '10', 10); const blockY = parseInt(xmlChildElement.getAttribute('y') ?? '10', 10); @@ -482,6 +490,10 @@ export function domToWorkspace(xml: Element, workspace: Workspace): string[] { 'create', ]); } + positionRecoveredBlocks(recoveredBlocks, workspace); + for (const recoveredBlock of recoveredBlocks) { + newBlockIds.push(recoveredBlock.block.id); + } variablesFirst = false; } else if (name === 'shadow') { throw TypeError('Shadow block cannot be a top-level block.'); @@ -632,24 +644,28 @@ export function domToBlock(xmlBlock: Element, workspace: Workspace): Block { export function domToBlockInternal( xmlBlock: Element, workspace: Workspace, + recoveredBlocks?: RecoveredBlockInfo[], ): Block { // Create top-level block. eventUtils.disable(); const variablesBeforeCreation = workspace.getVariableMap().getAllVariables(); let topBlock; + const localRecoveredBlocks = recoveredBlocks ?? []; try { - topBlock = domToBlockHeadless(xmlBlock, workspace); + topBlock = domToBlockHeadless( + xmlBlock, + workspace, + undefined, + undefined, + localRecoveredBlocks, + ); // Generate list of all blocks. if (workspace.rendered) { const topBlockSvg = topBlock as BlockSvg; - const blocks = topBlock.getDescendants(false); topBlockSvg.setConnectionTracking(false); - // Render each block. - for (let i = blocks.length - 1; i >= 0; i--) { - (blocks[i] as BlockSvg).initSvg(); - } - for (let i = blocks.length - 1; i >= 0; i--) { - (blocks[i] as BlockSvg).queueRender(); + initializeBlockTree(topBlock, workspace); + for (const recoveredBlock of localRecoveredBlocks) { + initializeBlockTree(recoveredBlock.block, workspace); } // Populating the connection database may be deferred until after the // blocks have rendered. @@ -662,14 +678,17 @@ export function domToBlockInternal( // TODO(@picklesrus): #387. Remove when domToBlock avoids resizing. (workspace as WorkspaceSvg).resizeContents(); } else { - const blocks = topBlock.getDescendants(false); - for (let i = blocks.length - 1; i >= 0; i--) { - blocks[i].initModel(); + initializeBlockTree(topBlock, workspace); + for (const recoveredBlock of localRecoveredBlocks) { + initializeBlockTree(recoveredBlock.block, workspace); } } } finally { eventUtils.enable(); } + if (!recoveredBlocks) { + positionRecoveredBlocks(localRecoveredBlocks, workspace); + } if (eventUtils.isEnabled()) { const newVariables = Variables.getAddedVariables( workspace, @@ -715,6 +734,53 @@ interface childNodeTagMap { next: Element[]; } +interface RecoveredBlockInfo { + block: Block; + intendedParent: Block; +} + +const RECOVERED_BLOCK_X_OFFSET = 80; +const RECOVERED_BLOCK_Y_OFFSET = 40; + +function initializeBlockTree(block: Block, workspace: Workspace) { + const blocks = block.getDescendants(false); + if (workspace.rendered) { + for (let i = blocks.length - 1; i >= 0; i--) { + const blockSvg = blocks[i] as BlockSvg; + blockSvg.initSvg(); + } + for (let i = blocks.length - 1; i >= 0; i--) { + (blocks[i] as BlockSvg).queueRender(); + } + } else { + for (let i = blocks.length - 1; i >= 0; i--) { + blocks[i].initModel(); + } + } +} + +function positionRecoveredBlocks( + recoveredBlocks: RecoveredBlockInfo[], + workspace: Workspace, +) { + const blocksByParent = new Map(); + for (const recoveredBlock of recoveredBlocks) { + const count = blocksByParent.get(recoveredBlock.intendedParent.id) ?? 0; + blocksByParent.set(recoveredBlock.intendedParent.id, count + 1); + + const parentXY = recoveredBlock.intendedParent.getRelativeToSurfaceXY(); + const blockXY = recoveredBlock.block.getRelativeToSurfaceXY(); + const xOffset = workspace.RTL + ? -RECOVERED_BLOCK_X_OFFSET + : RECOVERED_BLOCK_X_OFFSET; + const targetX = parentXY.x + xOffset; + const targetY = parentXY.y + RECOVERED_BLOCK_Y_OFFSET * (count + 1); + recoveredBlock.block.moveBy(targetX - blockXY.x, targetY - blockXY.y, [ + 'create', + ]); + } +} + /** * Creates a mapping of childNodes for each supported XML tag for the provided * xmlBlock. Logs a warning for any encountered unsupported tags. @@ -895,6 +961,7 @@ function applyInputTagNodes( workspace: Workspace, block: Block, prototypeName: string, + recoveredBlocks?: RecoveredBlockInfo[], ) { for (let i = 0; i < xmlChildren.length; i++) { const xmlChild = xmlChildren[i]; @@ -919,6 +986,7 @@ function applyInputTagNodes( workspace, input.connection, false, + recoveredBlocks, ); } // Set shadow after so we don't create a shadow we delete immediately. @@ -939,6 +1007,7 @@ function applyNextTagNodes( xmlChildren: Element[], workspace: Workspace, block: Block, + recoveredBlocks?: RecoveredBlockInfo[], ) { for (let i = 0; i < xmlChildren.length; i++) { const xmlChild = xmlChildren[i]; @@ -957,6 +1026,7 @@ function applyNextTagNodes( workspace, block.nextConnection, true, + recoveredBlocks, ); } // Set shadow after so we don't create a shadow we delete immediately. @@ -983,6 +1053,7 @@ function domToBlockHeadless( workspace: Workspace, parentConnection?: Connection, connectedToParentNext?: boolean, + recoveredBlocks?: RecoveredBlockInfo[], ): Block { let block = null; const prototypeName = xmlBlock.getAttribute('type'); @@ -1004,28 +1075,49 @@ function domToBlockHeadless( // Connect parent after processing mutation and before setting fields. if (parentConnection) { + let childConnection: Connection; + let connected = false; if (connectedToParentNext) { if (block.previousConnection) { - parentConnection.connect(block.previousConnection); + childConnection = block.previousConnection; + connected = parentConnection.connect(childConnection); } else { throw TypeError('Next block does not have previous statement.'); } } else { if (block.outputConnection) { - parentConnection.connect(block.outputConnection); + childConnection = block.outputConnection; + connected = parentConnection.connect(childConnection); } else if (block.previousConnection) { - parentConnection.connect(block.previousConnection); + childConnection = block.previousConnection; + connected = parentConnection.connect(childConnection); } else { throw TypeError( 'Child block does not have output or previous statement.', ); } } + if (!connected) { + recoveredBlocks?.push({ + block, + intendedParent: parentConnection.getSourceBlock(), + }); + block.setDisabledReason(true, ORPHANED_BLOCK_DISABLED_REASON); + console.warn( + `Recovered invalid XML connection: kept disconnected block ${block.type}.`, + ); + } } applyFieldTagNodes(xmlChildNameMap.field, block); - applyInputTagNodes(xmlChildNameMap.input, workspace, block, prototypeName); - applyNextTagNodes(xmlChildNameMap.next, workspace, block); + applyInputTagNodes( + xmlChildNameMap.input, + workspace, + block, + prototypeName, + recoveredBlocks, + ); + applyNextTagNodes(xmlChildNameMap.next, workspace, block, recoveredBlocks); if (shouldCallInitSvg) { // This shouldn't even be called here diff --git a/packages/blockly/tests/mocha/xml_test.js b/packages/blockly/tests/mocha/xml_test.js index 94219f9a067..68002910942 100644 --- a/packages/blockly/tests/mocha/xml_test.js +++ b/packages/blockly/tests/mocha/xml_test.js @@ -726,6 +726,39 @@ suite('XML', function () { }, ], }, + { + 'type': 'xml_parent_value_check_int', + 'message0': '%1', + 'args0': [ + { + 'type': 'input_value', + 'name': 'VALUE', + 'check': 'Int', + }, + ], + }, + { + 'type': 'xml_child_output_check_long', + 'message0': '%1', + 'args0': [ + { + 'type': 'field_input', + 'name': 'TEXT', + 'text': 'default text', + }, + ], + 'output': 'Long', + }, + { + 'type': 'xml_parent_next_check_int', + 'message0': '', + 'nextStatement': 'Int', + }, + { + 'type': 'xml_child_previous_check_long', + 'message0': '', + 'previousStatement': 'Long', + }, ]); }); teardown(function () { @@ -806,6 +839,177 @@ suite('XML', function () { Blockly.Xml.domToWorkspace(dom, this.workspace); }); }); + test('Recovers incompatible value input connection', function () { + const dom = Blockly.utils.xml.textToDom( + '' + + ' ' + + ' ' + + ' ' + + ' ' + + ' ' + + '', + ); + Blockly.Xml.domToWorkspace(dom, this.workspace); + const blocks = this.workspace.getAllBlocks(false); + assert.equal(blocks.length, 2, 'Block count'); + const parent = this.workspace.getTopBlocks(false).find((block) => { + return block.type === 'xml_parent_value_check_int'; + }); + const recovered = this.workspace.getTopBlocks(false).find((block) => { + return block.type === 'xml_child_output_check_long'; + }); + assert.isOk(parent, 'expected parent block'); + assert.isOk(recovered, 'expected recovered block'); + assert.isTrue(parent.initialized, 'parent should be initialized'); + assert.isTrue( + recovered.initialized, + 'recovered block should be initialized', + ); + assert.isNull( + recovered.getParent(), + 'recovered block should be top-level', + ); + assert.isFalse( + recovered.isEnabled(), + 'recovered block should be disabled', + ); + assert.isAbove( + recovered.getRelativeToSurfaceXY().y, + parent.getRelativeToSurfaceXY().y, + 'recovered block should be moved below the parent', + ); + }); + test('Recovers incompatible next connection', function () { + const dom = Blockly.utils.xml.textToDom( + '' + + ' ' + + ' ' + + ' ' + + ' ' + + ' ' + + '', + ); + Blockly.Xml.domToWorkspace(dom, this.workspace); + const blocks = this.workspace.getAllBlocks(false); + assert.equal(blocks.length, 2, 'Block count'); + const parent = this.workspace.getTopBlocks(false).find((block) => { + return block.type === 'xml_parent_next_check_int'; + }); + const recovered = this.workspace.getTopBlocks(false).find((block) => { + return block.type === 'xml_child_previous_check_long'; + }); + assert.isOk(parent, 'expected parent block'); + assert.isOk(recovered, 'expected recovered block'); + assert.isTrue(parent.initialized, 'parent should be initialized'); + assert.isTrue( + recovered.initialized, + 'recovered block should be initialized', + ); + assert.isNull( + recovered.getParent(), + 'recovered block should be top-level', + ); + assert.isFalse( + recovered.isEnabled(), + 'recovered block should be disabled', + ); + assert.isAbove( + recovered.getRelativeToSurfaceXY().y, + parent.getRelativeToSurfaceXY().y, + 'recovered block should be moved below the parent', + ); + }); + test('Recovered block preserves field values', function () { + const dom = Blockly.utils.xml.textToDom( + '' + + ' ' + + ' ' + + ' ' + + ' preserved text' + + ' ' + + ' ' + + ' ' + + '', + ); + Blockly.Xml.domToWorkspace(dom, this.workspace); + const recovered = this.workspace.getTopBlocks(false).find((block) => { + return block.type === 'xml_child_output_check_long'; + }); + assert.isOk(recovered, 'expected recovered block'); + assert.equal( + recovered.getFieldValue('TEXT'), + 'preserved text', + 'recovered block should preserve field values', + ); + }); + test('Returns recovered block ids from domToWorkspace', function () { + const dom = Blockly.utils.xml.textToDom( + '' + + ' ' + + ' ' + + ' ' + + ' ' + + ' ' + + '', + ); + const newBlockIds = Blockly.Xml.domToWorkspace(dom, this.workspace); + assert.sameMembers(newBlockIds, ['parent_id', 'child_id']); + }); + suite('Rendered recovery', function () { + setup(function () { + this.renderedWorkspace = Blockly.inject('blocklyDiv', {}); + }); + teardown(function () { + workspaceTeardown.call(this, this.renderedWorkspace); + }); + + test('Recovered block is rendered, disabled, and top-level', function () { + const dom = Blockly.utils.xml.textToDom( + '' + + ' ' + + ' ' + + ' ' + + ' render me' + + ' ' + + ' ' + + ' ' + + '', + ); + Blockly.Xml.domToWorkspace(dom, this.renderedWorkspace); + this.clock.runAll(); + + const parent = this.renderedWorkspace + .getTopBlocks(false) + .find((block) => block.type === 'xml_parent_value_check_int'); + const recovered = this.renderedWorkspace + .getTopBlocks(false) + .find((block) => block.type === 'xml_child_output_check_long'); + + assert.isOk(parent, 'expected parent block'); + assert.isOk(recovered, 'expected recovered block'); + assert.isNull( + recovered.getParent(), + 'recovered block should be top-level', + ); + assert.isFalse( + recovered.isEnabled(), + 'recovered block should be disabled', + ); + assert.isOk( + recovered.getSvgRoot(), + 'recovered block should be rendered', + ); + assert.isTrue( + recovered.getSvgRoot().classList.contains('blocklyDisabled'), + 'recovered block should have disabled styling', + ); + assert.isAbove( + recovered.getRelativeToSurfaceXY().y, + parent.getRelativeToSurfaceXY().y, + 'recovered block should be moved below the parent', + ); + }); + }); }); suite('appendDomToWorkspace', function () { setup(function () {