Undeletable Blocks Created Through .domToWorkspace can be "Deleted" through Undo

[BeksOmega]

Problem statement

Bug: This behavior means that the user could accidentally delete an undeletable block.

Expected Behavior

When you press ctrl/cmd z to "un-create" an un-deletable block nothing should happen.

Actual Behavior

The block is deleted.

Steps to Reproduce

1. Add below code at ~ ln 124 in playground:

var workspaceBlocks = document.getElementById("workspaceBlocks");
Blockly.Xml.domToWorkspace(workspaceBlocks, workspace);

2. Add below code at ~ ln 366 in playground:

<xml xmlns="http://www.w3.org/1999/xhtml" id="workspaceBlocks" style="display:none">
    <block type="controls_if" deletable="false" id="1" x="12" y="13"></block>
</xml>

3. Load the playground & observe how a "controls_if" block appears.
4. Press ctrl/cmd z & observe how it is deleted (even though deletable is set to false).

Stack Traces

N/A

Operating System and Browser

I assume this affects all browers.

Additional Information

[rachel-fenichel]

This behaviour makes sense to me as long as all of the blocks that were added with domToWorkspace are deleted at the same time. I see the undo as applying to the domToWorkspace call, rather than an individual block creation.

For instance, if I load multiple blocks at the same time, including an undeletable one, hitting undo should remove all of the loaded blocks--not just the deletable ones.

[BeksOmega]

Hm... that does make sense if you're thinking about it in the context of domToWorkspace being a user action. For instance the following sequence of actions makes sense to me:

1. The user loads a blockly page and sees a "Load Blocks" button.
2. The user clicks said button.
3. Amazing wonders of blockly appear before their eyes.
4. The user does an undo action.
5. All of the amazing wonders disapear at once.

But if you think about it as something run by the webpage, it doesn't make as much sense:

1. The user loads a blockly page that has some starter blocks.
2. The user does an undo action (presumably accidentally).
3. The starter blocks are deleted (including the undeleteable ones).

In some cases, recovering from this state may be impossible, for instance:

4. The user reloads the page (presumably because they are confused).
5. The workspace gets saved to the cloud/locally.
6. The workspace "loads the saved blocks" (which is actually nothing) so the starter blocks do not appear.

I have run into this problem myself.

Perhaps, because in some cases you could want to record undo, and in other cases you do not, it should be a parameter of the domToWorkspace function, or there should be a note somewhere informing developers to disable recording events when calling it.

[rachel-fenichel]

Yeah, I think this has to be something that the developer does. Disabling events before your load and enabling them after sounds reasonable to me. That's essentially your way of saying "this stuff is not in the user's control", which is true enough.

domToWorkspace gets used a lot and I don't want to add parameters without great need. I'm going to leave it as is for now, and keep my ears open to see if there is a pattern of people having problems with this.