Merge pull request #528 from scratchfoundation/fix/top-level-shadow-blocks

fix: prevent shadow blocks from being top-level in SB3 round-trip

Author: cwillisf

packages/scratch-vm/src/serialization/sb3.js

@@ -201,7 +201,7 @@ const serializeBlock = function (block) {
     obj.inputs = serializeInputs(block.inputs);
     obj.fields = serializeFields(block.fields);
     obj.shadow = block.shadow;
-    if (block.topLevel) {
+    if (block.topLevel && !block.shadow) {
         obj.topLevel = true;
         obj.x = block.x ? Math.round(block.x) : 0;
         obj.y = block.y ? Math.round(block.y) : 0;
@@ -840,6 +840,12 @@ const deserializeBlocks = function (blocks) {
         block.id = blockId; // add id back to block since it wasn't serialized
         block.inputs = deserializeInputs(block.inputs, blockId, blocks);
         block.fields = deserializeFields(block.fields);
+        // Obscured shadow blocks can be serialized as top-level by
+        // production Scratch. Upstream Blockly throws if a shadow
+        // appears at the top level of the workspace XML.
+        if (block.shadow && block.topLevel) {
+            block.topLevel = false;
+        }
 
         if (block.comment) {
             // Pre-Blockly v12 Scratch used arbitrary IDs for block comments.

packages/scratch-vm/test/unit/serialization_sb3.js

@@ -255,6 +255,70 @@ test('serializeBlocks serializes x and y for topLevel blocks with x,y of 0,0', t
         });
 });
 
+test('serializeBlock does not serialize shadow blocks as top-level', t => {
+    const blocks = {
+        hatBlock: {
+            id: 'hatBlock',
+            opcode: 'event_whenflagclicked',
+            next: 'costumeBlock',
+            parent: null,
+            inputs: {},
+            fields: {},
+            shadow: false,
+            topLevel: true,
+            x: 0,
+            y: 0
+        },
+        costumeBlock: {
+            id: 'costumeBlock',
+            opcode: 'looks_switchcostumeto',
+            next: null,
+            parent: 'hatBlock',
+            inputs: {
+                COSTUME: {
+                    name: 'COSTUME',
+                    block: 'varReporter',
+                    shadow: 'costumeShadow'
+                }
+            },
+            fields: {},
+            shadow: false,
+            topLevel: false
+        },
+        varReporter: {
+            id: 'varReporter',
+            opcode: 'data_variable',
+            next: null,
+            parent: 'costumeBlock',
+            inputs: {},
+            fields: {VARIABLE: {name: 'VARIABLE', value: 'my variable', id: 'var1'}},
+            shadow: false,
+            topLevel: false
+        },
+        costumeShadow: {
+            id: 'costumeShadow',
+            opcode: 'looks_costume',
+            next: null,
+            parent: null,
+            inputs: {},
+            fields: {COSTUME: {name: 'COSTUME', value: 'costume1'}},
+            shadow: true,
+            topLevel: true // bug: should not be serialized as top-level
+        }
+    };
+
+    const result = sb3.serializeBlocks(blocks);
+    const serialized = result[0];
+
+    t.equal(serialized.costumeShadow.topLevel, false,
+        'shadow block should not be serialized as top-level');
+    t.notOk(serialized.costumeShadow.x,
+        'shadow block should not have x coordinate');
+    t.notOk(serialized.costumeShadow.y,
+        'shadow block should not have y coordinate');
+    t.end();
+});
+
 test('deserializeBlocks', t => {
     const vm = new VirtualMachine();
     vm.loadProject(readFileToBuffer(commentsSB3ProjectPath))
@@ -280,6 +344,45 @@ test('deserializeBlocks on already deserialized input', t => {
         });
 });
 
+test('deserializeBlocks clears topLevel on shadow blocks', t => {
+    // Production Scratch can serialize obscured shadow blocks as top-level
+    // (e.g. when a variable reporter covers a dropdown's shadow). Upstream
+    // Blockly throws "Shadow block cannot be a top-level block" if these
+    // survive into the XML, so deserialization must clear the flag.
+    const blocks = {
+        parentBlock: {
+            opcode: 'looks_switchcostumeto',
+            next: null,
+            parent: null,
+            inputs: {
+                COSTUME: [3, [12, 'my variable', 'var_id'], 'shadowBlock']
+            },
+            fields: {},
+            shadow: false,
+            topLevel: true,
+            x: 0,
+            y: 0
+        },
+        shadowBlock: {
+            opcode: 'looks_costume',
+            next: null,
+            parent: null,
+            inputs: {},
+            fields: {COSTUME: ['costume1', null]},
+            shadow: true,
+            topLevel: true,
+            x: 100,
+            y: 100
+        }
+    };
+
+    sb3.deserializeBlocks(blocks);
+
+    t.equal(blocks.shadowBlock.topLevel, false,
+        'shadow block should not be top-level after deserialization');
+    t.end();
+});
+
 test('getExtensionIdForOpcode', t => {
     t.equal(sb3.getExtensionIdForOpcode('wedo_loopy'), 'wedo');