Blockly: Checkboxes in mutators are not toggleable

[benjie]

e.g. visit https://blockly-demo.appspot.com/static/demos/toolbox/index.html and add a function with return value block; then open the mutator and attempt to toggle 'allow statements'. The checkbox cannot un-check.

This is also true of checkboxes in custom mutators.

[benjie]

After some digging, it seems the issue is here:

Blockly.BlockSvg.prototype.onMouseDown_ = function(e) {
  ...
  } else if (!this.isMovable()) {
    // Allow immovable blocks to be selected and context menued, but not
    // dragged.  Let this event bubble up to document, so the workspace may be
    // dragged instead.
    return;
  } else {
  ...
    Blockly.BlockSvg.onMouseUpWrapper_ = Blockly.bindEventWithChecks_(document,
        'mouseup', this, this.onMouseUp_);
  ...

Because the root block in a mutator is marked as not moveable we don't add the mouseup event listener; and without that we cannot trigger the check/uncheck event on checkboxes.

[benjie]

Here's a monkey-patch to make it work again:

const oldBlocklyBlockSvgOnMouseDown_ = Blockly.BlockSvg.prototype.onMouseDown_;
Blockly.BlockSvg.prototype.onMouseDown_ = function(e) {
  oldBlocklyBlockSvgOnMouseDown_.call(this, e);
  if (!this.workspace.options.readOnly && !this.isMovable() && this.workspace.isMutator) {
    Blockly.BlockSvg.onMouseUpWrapper_ = Blockly.bindEventWithChecks_(document, 'mouseup', this, this.onMouseUp_);
    e.stopPropagation();
  }
}

The stopPropagation is necessary for it to work, so I added this.workspace.isMutator to the condition list to try and ensure this doesn't cause too many side effects.

[rachel-fenichel]

Thanks for the patch. Have you checked if it works on touch devices (iOS in particular)? That's where our mouse event handling sometimes breaks down.

I'll look into this more after the holidays.

[benjie]

I have not checked on mobile; it's just a temporary workaround - it probably causes other issues I've not discovered yet.

[RoboErikG]

Hi benjie,

This looks like another instance of #742 where non-movable blocks in general don't generate click events. I think Neil has a CL up that's being reviewed for it. Please re-open if that CL doesn't fix this issue in the mutator dialog.

[rachel-fenichel]

This is actually a combination of a lot of parts of our touch handling. Conditions for breaking are:
-- editable field
-- immovable block
-- non-scrollable workspace
all of which are met by the checkbox in the mutator.
Fixing #742 doesn't fix this, so I'm reopening.

[rachel-fenichel]

Fixed! Finally, and by rewriting half of blockly, but this is now fixed in develop by #1078

[benjie]

Whoa, that's a big PR! Congrats (and thanks!) on the fix 👍

[rachel-fenichel]

Thanks! Yeah, this was one of a suite of issues related to how our events code had evolved.