Skip to content

Commit 22f6429

Browse files
committed
test: add browser test for flyout copy shadow ID uniqueness
Adds a behavior test that exercises the actual flyout createBlock path (using CheckableContinuousFlyout) and verifies that two copies from the same flyout template get unique shadow IDs, even when the first copy's shadow was disposed (obscured by a reporter). This test reliably fails without the stripIds call in CheckableContinuousFlyout.serializeBlock (added in PR #3560).
1 parent c669994 commit 22f6429

1 file changed

Lines changed: 121 additions & 26 deletions

File tree

tests/browser/block_duplicate_shadow.test.ts

Lines changed: 121 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3,27 +3,27 @@
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55
import * as Blockly from 'blockly/core'
6-
import { afterEach, assert, beforeEach, describe, expect, it } from 'vitest'
6+
import { afterAll, afterEach, assert, beforeAll, beforeEach, describe, expect, it } from 'vitest'
7+
import { CheckableContinuousFlyout } from '../../src/checkable_continuous_flyout'
8+
import { registerRecyclableBlockFlyoutInflater } from '../../src/recyclable_block_flyout_inflater'
79
import { registerScratchBlockPaster } from '../../src/scratch_block_paster'
810

9-
// Browser test: duplicating a block with an obscured shadow must produce a
10-
// copy with unique shadow IDs. Without the stripIds fix in
11-
// ScratchBlockPaster.paste, the copy reuses the original's shadow IDs,
12-
// causing the VM to think both blocks share the same shadow. Deleting one
13-
// then destroys the other's shadow (forum topic 878291).
11+
// Browser tests for the shared-shadow-ID bug (forum topic 878291).
12+
// Both the duplicate/paste path and the flyout copy path must produce
13+
// blocks with unique shadow IDs. Without stripIds, disposed shadows
14+
// from the original reuse their IDs in the copy, causing the VM to
15+
// think both blocks share the same shadow.
1416

1517
const BLOCK_TYPES = ['test_value_block', 'test_text_shadow', 'test_reporter']
1618

17-
let container: HTMLElement
18-
let workspace: Blockly.WorkspaceSvg
19+
let container: HTMLElement | undefined
20+
let workspace: Blockly.WorkspaceSvg | undefined
1921

2022
beforeEach(() => {
2123
container = document.createElement('div')
2224
container.style.width = '800px'
2325
container.style.height = '600px'
2426
document.body.appendChild(container)
25-
workspace = Blockly.inject(container, {})
26-
registerScratchBlockPaster()
2727

2828
Blockly.defineBlocksWithJsonArray([
2929
{
@@ -47,36 +47,64 @@ beforeEach(() => {
4747
])
4848
})
4949

50+
// Save the default Blockly implementations so we can restore them after
51+
// all tests, preventing cross-suite leakage of Scratch-specific overrides.
52+
const DefaultInflater = Blockly.registry.getClass(Blockly.registry.Type.FLYOUT_INFLATER, 'block')
53+
54+
beforeAll(() => {
55+
registerScratchBlockPaster()
56+
registerRecyclableBlockFlyoutInflater()
57+
})
58+
59+
afterAll(() => {
60+
// Restore the default block paster
61+
Blockly.clipboard.registry.unregister(Blockly.clipboard.BlockPaster.TYPE)
62+
Blockly.clipboard.registry.register(Blockly.clipboard.BlockPaster.TYPE, new Blockly.clipboard.BlockPaster())
63+
// Restore the default flyout inflater
64+
if (DefaultInflater) {
65+
Blockly.registry.unregister(Blockly.registry.Type.FLYOUT_INFLATER, 'block')
66+
Blockly.registry.register(Blockly.registry.Type.FLYOUT_INFLATER, 'block', DefaultInflater)
67+
}
68+
})
69+
5070
afterEach(() => {
51-
workspace.dispose()
52-
container.remove()
71+
workspace?.dispose()
72+
container?.remove()
5373
for (const t of BLOCK_TYPES) {
5474
delete Blockly.Blocks[t]
5575
}
5676
})
5777

5878
describe('duplicate block shadow IDs (forum topic 878291)', () => {
5979
it('duplicated block gets unique shadow IDs, not shared with original', () => {
80+
assert(container, 'Expected container from beforeEach')
81+
workspace = Blockly.inject(container, {})
82+
6083
// Create the original block with a shadow on VALUE
84+
let original: Blockly.BlockSvg
85+
let reporter: Blockly.BlockSvg
6186
Blockly.Events.disable()
62-
const original = Blockly.serialization.blocks.append(
63-
{
64-
type: 'test_value_block',
65-
inputs: {
66-
VALUE: {
67-
shadow: {
68-
type: 'test_text_shadow',
69-
fields: { TEXT: '0' },
87+
try {
88+
original = Blockly.serialization.blocks.append(
89+
{
90+
type: 'test_value_block',
91+
inputs: {
92+
VALUE: {
93+
shadow: {
94+
type: 'test_text_shadow',
95+
fields: { TEXT: '0' },
96+
},
7097
},
7198
},
7299
},
73-
},
74-
workspace,
75-
) as Blockly.BlockSvg
100+
workspace,
101+
) as Blockly.BlockSvg
76102

77-
// Connect a reporter to obscure the shadow
78-
const reporter = workspace.newBlock('test_reporter')
79-
Blockly.Events.enable()
103+
// Connect a reporter to obscure the shadow
104+
reporter = workspace.newBlock('test_reporter')
105+
} finally {
106+
Blockly.Events.enable()
107+
}
80108

81109
const conn = original.getInput('VALUE')?.connection
82110
assert(conn, 'Expected VALUE connection')
@@ -113,4 +141,71 @@ describe('duplicate block shadow IDs (forum topic 878291)', () => {
113141
expect(respawned.isShadow()).toBe(true)
114142
expect(respawned.type).toBe('test_text_shadow')
115143
})
144+
145+
it('two flyout copies get unique shadow IDs when first copy shadow is obscured', () => {
146+
assert(container, 'Expected container from beforeEach')
147+
// Inject with CheckableContinuousFlyout (which has the stripIds fix)
148+
// and a toolbox that includes a block with a shadow input.
149+
workspace = Blockly.inject(container, {
150+
toolbox: {
151+
kind: 'flyoutToolbox',
152+
contents: [
153+
{
154+
kind: 'block',
155+
type: 'test_value_block',
156+
inputs: {
157+
VALUE: {
158+
shadow: {
159+
type: 'test_text_shadow',
160+
fields: { TEXT: '0' },
161+
},
162+
},
163+
},
164+
},
165+
],
166+
},
167+
plugins: {
168+
flyoutsVerticalToolbox: CheckableContinuousFlyout,
169+
},
170+
})
171+
172+
const flyout = workspace.getFlyout()
173+
assert(flyout, 'Expected workspace to have a flyout')
174+
175+
// Find the template block in the flyout
176+
const flyoutBlocks = flyout.getWorkspace().getAllBlocks(false)
177+
const template = flyoutBlocks.find((b) => b.type === 'test_value_block')
178+
assert(template, 'Expected template block in flyout')
179+
180+
// First copy from flyout
181+
const copy1 = flyout.createBlock(template)
182+
const conn1 = copy1.getInput('VALUE')?.connection
183+
assert(conn1, 'Expected VALUE connection on first copy')
184+
const shadow1 = conn1.targetBlock()
185+
assert(shadow1, 'Expected shadow on first copy')
186+
expect(shadow1.isShadow()).toBe(true)
187+
const shadow1Id = shadow1.id
188+
189+
// Obscure the first copy's shadow by connecting a reporter
190+
let reporter: Blockly.BlockSvg
191+
Blockly.Events.disable()
192+
try {
193+
reporter = workspace.newBlock('test_reporter')
194+
} finally {
195+
Blockly.Events.enable()
196+
}
197+
const reporterOutput = reporter.outputConnection
198+
assert(reporterOutput, 'Expected reporter output connection')
199+
conn1.connect(reporterOutput)
200+
201+
// Second copy from flyout — its shadow must get a different ID
202+
const copy2 = flyout.createBlock(template)
203+
const conn2 = copy2.getInput('VALUE')?.connection
204+
assert(conn2, 'Expected VALUE connection on second copy')
205+
const shadow2 = conn2.targetBlock()
206+
assert(shadow2, 'Expected shadow on second copy')
207+
expect(shadow2.isShadow()).toBe(true)
208+
209+
expect(shadow2.id).not.toBe(shadow1Id)
210+
})
116211
})

0 commit comments

Comments
 (0)