Blocks with a field following a variable field break when a mutator is open

[MatzElectronics]

Describe the bug

In blocks that have mutators that create and delete inputs (dummy or value), if those blocks contain a variable field followed by another field break when the mutator is opened.

To Reproduce

Steps to reproduce the behavior:

1. Create a mutating block with a field layout described in above:

Blockly.Blocks['mutator_variable_test'] = {
    init: function () {
        this.itemCount_ = 2;
        this.appendDummyInput().appendField('mutator test block');
        this.updateShape_();
        this.setMutator(new Blockly.Mutator(['mutator_variable_test_item']));
    },
    mutationToDom: function () {
        var container = Blockly.utils.xml.createElement('mutation');
        container.setAttribute('items', this.itemCount_);
        return container;
    },
    domToMutation: function (xmlElement) {
        this.itemCount_ = parseInt(xmlElement.getAttribute('items'), 10);
        this.updateShape_();
    },
    decompose: function (workspace) {
        var containerBlock = workspace.newBlock('mutator_variable_test_container');
        containerBlock.initSvg();
        var connection = containerBlock.getInput('STACK').connection;
        for (var i = 0; i < this.itemCount_; i++) {
            var itemBlock = workspace.newBlock('mutator_variable_test_item');
            itemBlock.initSvg();
            connection.connect(itemBlock.previousConnection);
            connection = itemBlock.nextConnection;
        }
        return containerBlock;
    },
    compose: function (containerBlock) {
        var itemBlock = containerBlock.getInputTargetBlock('STACK');
        // Count number of inputs.
        var connections = [];
        while (itemBlock) {
            connections.push(itemBlock.valueConnection_);
            itemBlock = itemBlock.nextConnection &&
                itemBlock.nextConnection.targetBlock();
        }
        this.itemCount_ = connections.length;
        this.updateShape_();
    },
    updateShape_: function () {
        // Add new inputs.
        for (var i = 0; i < this.itemCount_; i++) {
            if (!this.getInput('ADD' + i)) {
                this.appendValueInput('ADD' + i)
                    .appendField('variable')
                    .appendField(new Blockly.FieldVariable('item'), 'VAR' + i)  // <-- THIS IS THE SOURCE OF THE BUG WHEN FOLLOWED BY ANOTHER FIELD.
                    .appendField('that stores stuff');
            }
        }
        // Remove deleted inputs.
        while (this.getInput('ADD' + i)) {
            this.removeInput('ADD' + i);
            i++;
        }
    }
};

Blockly.Blocks['mutator_variable_test_container'] = {
    init: function () {
        this.appendDummyInput().appendField('add things');
        this.appendStatementInput('STACK');
    }
};

Blockly.Blocks['mutator_variable_test_item'] = {
    init: function () {
        this.appendDummyInput().appendField('thing');
        this.setPreviousStatement(true);
        this.setNextStatement(true);
    }
};

2. Click on the mutator icon
3. Try mutating the block
4. See error
   

Expected behavior

The block should render correctly and the fields should be editable even when the mutator is open.

Desktop (please complete the following information):

• OS: MacOS 10.13.2, CrOS
• Browser: Chrome 78.0.3904.97, Firefox 70.0.1, Safari 11.0.2

Stack Traces

field.js:567 Uncaught TypeError: Cannot set property 'nodeValue' of undefined
    at Blockly.FieldLabel.Blockly.Field.render_ (field.js:567)
    at Blockly.FieldLabel.Blockly.Field.getSize (field.js:611)
    at new Blockly.blockRendering.Field (row_elements.js:101)
    at Blockly.minimalist.RenderInfo.Blockly.blockRendering.RenderInfo.createRows_ (info.js:235)
    at Blockly.minimalist.RenderInfo.Blockly.blockRendering.RenderInfo.measure (info.js:188)
    at rendererCtor.Blockly.blockRendering.Renderer.render (renderer.js:150)
    at Blockly.BlockSvg.render (block_svg.js:1616)
    at Blockly.FieldVariable.Blockly.Field.forceRerender (field.js:712)
    at Blockly.FieldVariable.Blockly.Field.setValue (field.js:762)
    at Blockly.FieldVariable.initModel (field_variable.js:155)

Additional context

We noticed this error within our implementation (solo.parallax.com), and tried replacing the blockly core with the 2019 Q3 patch 4 release, which still showed the same error. I then built the most minimal mutating block that still demonstrated the error in the playground (release 2019 Q2) to verify that it wasn't caused by something within our codebase.

[BeksOmega]

Thank you for reporting! This reproduces on develop with a slightly different stack trace:

drawer.js:345 Uncaught TypeError: Cannot read property 'setAttribute' of null
    at Blockly.geras.Drawer.Blockly.blockRendering.Drawer.layoutField_ (drawer.js:345)
    at Blockly.geras.Drawer.Blockly.blockRendering.Drawer.drawInternals_ (drawer.js:309)
    at Blockly.geras.Drawer.draw (drawer.js:59)
    at rendererCtor.Blockly.blockRendering.Renderer.render (renderer.js:181)
    at Blockly.BlockSvg.render (block_svg.js:1629)
    at Blockly.FieldVariable.Blockly.Field.forceRerender (field.js:756)
    at Blockly.FieldVariable.Blockly.Field.setValue (field.js:806)
    at Blockly.FieldVariable.initModel (field_variable.js:153)
    at Blockly.FieldVariable.Blockly.Field.init (field.js:295)
    at Blockly.Input.init (input.js:255)

But it's the same root cause.

In english: The variable field's initModel method is forcing a rerender of the block before the following fields' svgs can be initialized. This isn't a problem in the block's init method because the block isn't rendered yet, but in the case of a mutator the block is rendered.

I would /like/ to try and fix this by getting rid of initModel. (The single use case for initModel is outlined here. In my opinion, the toolbox point is moot because toolboxes don't exist on headless workspaces.)

But variable fields are /crazy/ so more drastic measures may need to be taken.

Thank you again for reporting!
--Beka

P.S. I really like your work around solution, very creative!

[MatzElectronics]

Thank you Beka for confirming this - for a while I was losing my mind trying to pin this one down :)

[rachel-fenichel]

@navilperez to confirm that this is fixed.

[samelhusseini]

Fixed.