Skip to content

fix!: Remove event dependencies on XML #9536

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gonfunko merged 1 commit into from
Dec 22, 2025
Merged

fix!: Remove event dependencies on XML #9536

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 0 additions & 15 deletions core/events/events_block_create.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@
import type {Block} from '../block.js';
import * as registry from '../registry.js';
import * as blocks from '../serialization/blocks.js';
import * as utilsXml from '../utils/xml.js';
import {Workspace} from '../workspace.js';
import * as Xml from '../xml.js';
import {BlockBase, BlockBaseJson} from './events_block_base.js';
import {EventType} from './type.js';
import * as eventUtils from './utils.js';
Expand All @@ -28,9 +26,6 @@ import * as eventUtils from './utils.js';
export class BlockCreate extends BlockBase {
override type = EventType.BLOCK_CREATE;

/** The XML representation of the created block(s). */
xml?: Element | DocumentFragment;

/** The JSON respresentation of the created block(s). */
json?: blocks.State;

Expand All @@ -50,7 +45,6 @@ export class BlockCreate extends BlockBase {
this.recordUndo = false;
}

this.xml = Xml.blockToDomWithXY(opt_block);
this.ids = eventUtils.getDescendantIds(opt_block);

this.json = blocks.save(opt_block, {addCoordinates: true}) as blocks.State;
Expand All @@ -63,12 +57,6 @@ export class BlockCreate extends BlockBase {
*/
override toJson(): BlockCreateJson {
const json = super.toJson() as BlockCreateJson;
if (!this.xml) {
throw new Error(
'The block XML is undefined. Either pass a block to ' +
'the constructor, or call fromJson',
);
}
if (!this.ids) {
throw new Error(
'The block IDs are undefined. Either pass a block to ' +
Expand All @@ -81,7 +69,6 @@ export class BlockCreate extends BlockBase {
'the constructor, or call fromJson',
);
}
json['xml'] = Xml.domToText(this.xml);
json['ids'] = this.ids;
json['json'] = this.json;
if (!this.recordUndo) {
Expand Down Expand Up @@ -109,7 +96,6 @@ export class BlockCreate extends BlockBase {
workspace,
event ?? new BlockCreate(),
) as BlockCreate;
newEvent.xml = utilsXml.textToDom(json['xml']);
newEvent.ids = json['ids'];
newEvent.json = json['json'] as blocks.State;
if (json['recordUndo'] !== undefined) {
Expand Down Expand Up @@ -176,7 +162,6 @@ const allShadowBlocks = function (
};

export interface BlockCreateJson extends BlockBaseJson {
xml: string;
ids: string[];
json: object;
recordUndo?: boolean;
Expand Down
18 changes: 1 addition & 17 deletions core/events/events_block_delete.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@
import type {Block} from '../block.js';
import * as registry from '../registry.js';
import * as blocks from '../serialization/blocks.js';
import * as utilsXml from '../utils/xml.js';
import {Workspace} from '../workspace.js';
import * as Xml from '../xml.js';
import {BlockBase, BlockBaseJson} from './events_block_base.js';
import {EventType} from './type.js';
import * as eventUtils from './utils.js';
Expand All @@ -26,9 +24,6 @@ import * as eventUtils from './utils.js';
* deleted.
*/
export class BlockDelete extends BlockBase {
/** The XML representation of the deleted block(s). */
oldXml?: Element | DocumentFragment;

/** The JSON respresentation of the deleted block(s). */
oldJson?: blocks.State;

Expand Down Expand Up @@ -56,7 +51,6 @@ export class BlockDelete extends BlockBase {
this.recordUndo = false;
}

this.oldXml = Xml.blockToDomWithXY(opt_block);
this.ids = eventUtils.getDescendantIds(opt_block);
this.wasShadow = opt_block.isShadow();
this.oldJson = blocks.save(opt_block, {
Expand All @@ -71,12 +65,6 @@ export class BlockDelete extends BlockBase {
*/
override toJson(): BlockDeleteJson {
const json = super.toJson() as BlockDeleteJson;
if (!this.oldXml) {
throw new Error(
'The old block XML is undefined. Either pass a block ' +
'to the constructor, or call fromJson',
);
}
if (!this.ids) {
throw new Error(
'The block IDs are undefined. Either pass a block to ' +
Expand All @@ -95,7 +83,6 @@ export class BlockDelete extends BlockBase {
'to the constructor, or call fromJson',
);
}
json['oldXml'] = Xml.domToText(this.oldXml);
json['ids'] = this.ids;
json['wasShadow'] = this.wasShadow;
json['oldJson'] = this.oldJson;
Expand Down Expand Up @@ -124,10 +111,8 @@ export class BlockDelete extends BlockBase {
workspace,
event ?? new BlockDelete(),
) as BlockDelete;
newEvent.oldXml = utilsXml.textToDom(json['oldXml']);
newEvent.ids = json['ids'];
newEvent.wasShadow =
json['wasShadow'] || newEvent.oldXml.tagName.toLowerCase() === 'shadow';
newEvent.wasShadow = json['wasShadow'];
newEvent.oldJson = json['oldJson'];
if (json['recordUndo'] !== undefined) {
newEvent.recordUndo = json['recordUndo'];
Expand Down Expand Up @@ -172,7 +157,6 @@ export class BlockDelete extends BlockBase {
}

export interface BlockDeleteJson extends BlockBaseJson {
oldXml: string;
ids: string[];
wasShadow: boolean;
oldJson: blocks.State;
Expand Down
15 changes: 0 additions & 15 deletions core/events/events_comment_create.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@
import type {WorkspaceComment} from '../comments/workspace_comment.js';
import * as registry from '../registry.js';
import * as comments from '../serialization/workspace_comments.js';
import * as utilsXml from '../utils/xml.js';
import type {Workspace} from '../workspace.js';
import * as Xml from '../xml.js';
import {CommentBase, CommentBaseJson} from './events_comment_base.js';
import {EventType} from './type.js';

Expand All @@ -26,9 +24,6 @@ import {EventType} from './type.js';
export class CommentCreate extends CommentBase {
override type = EventType.COMMENT_CREATE;

/** The XML representation of the created workspace comment. */
xml?: Element | DocumentFragment;

/** The JSON representation of the created workspace comment. */
json?: comments.State;

Expand All @@ -43,7 +38,6 @@ export class CommentCreate extends CommentBase {
return; // Blank event to be populated by fromJson.
}

this.xml = Xml.saveWorkspaceComment(opt_comment);
this.json = comments.save(opt_comment, {addCoordinates: true});
}

Expand All @@ -55,19 +49,12 @@ export class CommentCreate extends CommentBase {
*/
override toJson(): CommentCreateJson {
const json = super.toJson() as CommentCreateJson;
if (!this.xml) {
throw new Error(
'The comment XML is undefined. Either pass a comment to ' +
'the constructor, or call fromJson',
);
}
if (!this.json) {
throw new Error(
'The comment JSON is undefined. Either pass a block to ' +
'the constructor, or call fromJson',
);
}
json['xml'] = Xml.domToText(this.xml);
json['json'] = this.json;
return json;
}
Expand All @@ -91,7 +78,6 @@ export class CommentCreate extends CommentBase {
workspace,
event ?? new CommentCreate(),
) as CommentCreate;
newEvent.xml = utilsXml.textToDom(json['xml']);
newEvent.json = json['json'];
return newEvent;
}
Expand All @@ -107,7 +93,6 @@ export class CommentCreate extends CommentBase {
}

export interface CommentCreateJson extends CommentBaseJson {
xml: string;
json: object;
}

Expand Down
15 changes: 0 additions & 15 deletions core/events/events_comment_delete.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@
import type {WorkspaceComment} from '../comments/workspace_comment.js';
import * as registry from '../registry.js';
import * as comments from '../serialization/workspace_comments.js';
import * as utilsXml from '../utils/xml.js';
import type {Workspace} from '../workspace.js';
import * as Xml from '../xml.js';
import {CommentBase, CommentBaseJson} from './events_comment_base.js';
import {EventType} from './type.js';

Expand All @@ -26,9 +24,6 @@ import {EventType} from './type.js';
export class CommentDelete extends CommentBase {
override type = EventType.COMMENT_DELETE;

/** The XML representation of the deleted workspace comment. */
xml?: Element;

/** The JSON representation of the created workspace comment. */
json?: comments.State;

Expand All @@ -43,7 +38,6 @@ export class CommentDelete extends CommentBase {
return; // Blank event to be populated by fromJson.
}

this.xml = Xml.saveWorkspaceComment(opt_comment);
this.json = comments.save(opt_comment, {addCoordinates: true});
}

Expand All @@ -63,19 +57,12 @@ export class CommentDelete extends CommentBase {
*/
override toJson(): CommentDeleteJson {
const json = super.toJson() as CommentDeleteJson;
if (!this.xml) {
throw new Error(
'The comment XML is undefined. Either pass a comment to ' +
'the constructor, or call fromJson',
);
}
if (!this.json) {
throw new Error(
'The comment JSON is undefined. Either pass a block to ' +
'the constructor, or call fromJson',
);
}
json['xml'] = Xml.domToText(this.xml);
json['json'] = this.json;
return json;
}
Expand All @@ -99,14 +86,12 @@ export class CommentDelete extends CommentBase {
workspace,
event ?? new CommentDelete(),
) as CommentDelete;
newEvent.xml = utilsXml.textToDom(json['xml']);
newEvent.json = json['json'];
return newEvent;
}
}

export interface CommentDeleteJson extends CommentBaseJson {
xml: string;
json: object;
}

Expand Down
5 changes: 0 additions & 5 deletions tests/mocha/event_block_create_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,6 @@ suite('Block Create Event', function () {
this.workspace.id,
'shadowId',
);
const calls = this.eventsFireStub.getCalls();
const event = calls[calls.length - 1].args[0];
assert.equal(event.xml.tagName, 'shadow');
});

test('Does not create extra shadow blocks', function () {
Expand Down Expand Up @@ -100,8 +97,6 @@ suite('Block Create Event', function () {

const json = origEvent.toJson();
const newEvent = new Blockly.Events.fromJson(json, this.workspace);
delete origEvent.xml; // xml fails deep equals for some reason.
delete newEvent.xml; // xml fails deep equals for some reason.

assert.deepEqual(newEvent, origEvent);
});
Expand Down
29 changes: 1 addition & 28 deletions tests/mocha/event_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -639,10 +639,6 @@ suite('Events', function () {
type: 'create',
group: '',
blockId: thisObj.block.id,
xml:
'<block xmlns="https://developers.google.com/blockly/xml"' +
' type="simple_test_block" id="testBlockId1" x="0" y="0">' +
'</block>',
ids: [thisObj.block.id],
json: {
'type': 'simple_test_block',
Expand All @@ -660,10 +656,6 @@ suite('Events', function () {
type: 'create',
group: '',
blockId: thisObj.shadowBlock.id,
xml:
'<shadow xmlns="https://developers.google.com/blockly/xml"' +
' type="simple_test_block" id="testBlockId2" x="0" y="0">' +
'</shadow>',
ids: [thisObj.shadowBlock.id],
json: {
'type': 'simple_test_block',
Expand All @@ -682,10 +674,6 @@ suite('Events', function () {
type: 'delete',
group: '',
blockId: thisObj.block.id,
oldXml:
'<block xmlns="https://developers.google.com/blockly/xml"' +
' type="simple_test_block" id="testBlockId1" x="0" y="0">' +
'</block>',
ids: [thisObj.block.id],
wasShadow: false,
oldJson: {
Expand All @@ -704,10 +692,6 @@ suite('Events', function () {
type: 'delete',
group: '',
blockId: thisObj.shadowBlock.id,
oldXml:
'<shadow xmlns="https://developers.google.com/blockly/xml"' +
' type="simple_test_block" id="testBlockId2" x="0" y="0">' +
'</shadow>',
ids: [thisObj.shadowBlock.id],
wasShadow: true,
oldJson: {
Expand Down Expand Up @@ -765,11 +749,6 @@ suite('Events', function () {
type: 'comment_create',
group: '',
commentId: thisObj.comment.id,
// TODO: Before merging, is this a dumb change detector?
xml: Blockly.Xml.domToText(
Blockly.Xml.saveWorkspaceComment(thisObj.comment),
{addCoordinates: true},
),
json: {
height: 100,
width: 120,
Expand All @@ -788,11 +767,6 @@ suite('Events', function () {
type: 'comment_delete',
group: '',
commentId: thisObj.comment.id,
// TODO: Before merging, is this a dumb change detector?
xml: Blockly.Xml.domToText(
Blockly.Xml.saveWorkspaceComment(thisObj.comment),
{addCoordinates: true},
),
json: {
height: 100,
width: 120,
Expand Down Expand Up @@ -1405,7 +1379,6 @@ suite('Events', function () {
const block = workspaceSvg.newBlock('');
block.initSvg();
block.setCommentText('test comment');
const expectedOldXml = Blockly.Xml.blockToDomWithXY(block);
const expectedId = block.id;

// Run all queued events.
Expand All @@ -1426,7 +1399,7 @@ suite('Events', function () {
this.eventsFireSpy,
0,
Blockly.Events.BlockDelete,
{oldXml: expectedOldXml, group: ''},
{group: ''},
workspaceSvg.id,
expectedId,
);
Expand Down
Loading