logic_compare: Type-verification creates top level shadow block as a side effect

[samelhusseini]

Problem statement

Dragging a block that's an incompatible type bumps the block and then creates a top level shadow block as a sideeffect.

• Bug report
• Feature request

Expected Behavior

The text block should be bumped, with no new (top level) shadow block created.

Actual Behavior

The text block is bumped and a new (top level) shadow block appears in the workspace. This is consistent, and happens for every bump.

Also happens when inserting a Color block into a Number field, but doesn't happen when inserting a number block into a String field.

Steps to Reproduce

Take this block XML and try to insert a text block inside of the LHS of the number comparator.

<xml xmlns="http://www.w3.org/1999/xhtml">
  <variables></variables>
  <block type="controls_if" id="(j:IsX.#.-!m#~^cjkq0" x="159" y="333">
    <mutation else="1"></mutation>
    <value name="IF0">
      <block type="logic_compare" id="aNPP`-9t{F%Nj47Xg=kV">
        <field name="OP">EQ</field>
        <value name="A">
          <shadow type="math_number" id="0VN(._,,t6|xuWU(JI+U">
            <field name="NUM">10</field>
          </shadow>
        </value>
        <value name="B">
          <shadow type="math_number" id="6%e5FECiW9q9{P53ny6E">
            <field name="NUM">10</field>
          </shadow>
        </value>
      </block>
    </value>
  </block>
</xml>

Stack Traces

Here's the generated XML as a result of this:

<xml xmlns="http://www.w3.org/1999/xhtml">
  <variables></variables>
  <shadow type="math_number" id="6%e5FECiW9q9{P53ny6E" x="372" y="339">
    <field name="NUM">10</field>
  </shadow>
  <block type="text" id=")-3pKY(#Zk_m%uMs$7Xv" x="213" y="363">
    <field name="TEXT"></field>
  </block>
  <block type="controls_if" id="(j:IsX.#.-!m#~^cjkq0" x="138" y="388">
    <mutation else="1"></mutation>
    <value name="IF0">
      <block type="logic_compare" id="aNPP`-9t{F%Nj47Xg=kV">
        <field name="OP">EQ</field>
        <value name="A">
          <shadow type="math_number" id="0VN(._,,t6|xuWU(JI+U">
            <field name="NUM">10</field>
          </shadow>
        </value>
        <value name="B">
          <shadow type="math_number" id="=-.oL;GbFGo;omj9Fldg">
            <field name="NUM">10</field>
          </shadow>
        </value>
      </block>
    </value>
  </block>
</xml>

Operating System and Browser

• Desktop:
  • Chrome
  • Firefox
  • Safari
  • Opera
  • IE 10+
  • IE 11
  • EDGE

Affects both develop and master

[picklesrus]

Oooh. Neat bug! The fact that the shadow gets bumped onto the workspace is definitely a problem.

Beyond that part, this is tricky. When you say you expect the text block to be bumped, do you mean it should never have connected in the first place?

What types do you want to be able to compare using the == block? Do you want to be able to compare a number and a string and use a cast in the generator?

I suspect the == block in the playground doesn't have shadows at all to avoid dealing with the complexity of switching types like this :)

[samelhusseini]

Yeah I don't think there's any issue with the connecting logic. The bug is just that it bumps the shadow out onto the workspace.

Although I do agree, it would be nice to have a union type for the equality block since you should be able to compare two strings, or a string and a number (false?), but that's a different story. (And perhaps this isn't a Blockly thing, and can be solved by having a union (string | number) block).

[NeilFraser]

I suspect the == block in the playground doesn't have shadows at all to avoid dealing with the complexity of switching types like this

Actually, this (awesome) bug exists because it never occurred to me to test the intersection of =='s custom connection logic and shadow blocks. :)

I'd vote to refuse connect a non-compatible block if the conflicting block is a shadow or unmovable. After all, if the developer places a shadow block in a == block, then I feel they have strongly defined the type. This bug may also affect the ?: ternary block as well.

[AnmAtAnm]

Neil's solution implies the toolbox would need to either remove shadow blocks, or have a comparison operator for every type (number, string, list, etc).

This toolbox change is going to affect anyone who has copied a demo toolbox since we introduced shadows. If we implement Neil's suggestion, and a developer does not update their toolbox shadows, their users will not be able to compare strings.

A more compatible approach would be to delete the shadow if it is incompatible.

A more user-friendly variant (but much more complicated) might replace include special handling of the number and string type cases. When the new connecting block is a text and the old is a math_number shadow, replace the shadow with a text block with the string form of the number. When a math_number attempts to compare to a text shadow, coerce the text into a number (or 0) shadow.

In this case, for other unrecognized types we could implement either shadow deletion or connection invalidation.

In terms of the demo toolboxes, the separation of number comparisons and string comparisons makes me feel the blocks belong in the Math and Text categories, instead of the logic category. (Or remove the shadows, as I previously mentioned.)

[AnmAtAnm]

Correction. Blockly toolbox does not (and presumably has not) used shadows in logic_compare.

[AnmAtAnm]

More weirdness:

Before

After

The behavior depends upon other logic_compare blocks on the workspace.

[plh97]

why you did not support === or !==

[plh97]

in javascript syntax it's need to support