fix: Check for empty array in thrasos.RenderInfo.addElemSpacing_

[Zauzolkov]

The basics

• I branched from develop
• My pull request is against develop
• My code follows the style guide
• I ran npm run format and npm run lint

The details

This pull-request was edited after digging for roots of the problem.

Resolves

#6135

Proposed Changes

• Check for possible undefined value in measure method of Blockly.blockRendering.InputRow class.
• Check for empty array in Blockly.thrasos.RenderInfo.addElemSpacing_ method.

Behavior Before Change

• There is no check for undefined value in measure method of Blockly.blockRendering.InputRow class.
• There is no check for empty array in Blockly.thrasos.RenderInfo.addElemSpacing_ method.
• Execution is interrupted with error if there is undefined value in elements array of InputRow (because of attempt to access a non-existent property): InputRow.measure method throws Uncaught TypeError: elem is undefined.

Behavior After Change

• measure method of InputRow will continue loop on undefined value instead of throwing error.
• Blockly.thrasos.RenderInfo.addElemSpacing_ method is checking for empty array just like common and geras renderers does.

Reason for Changes

This issue was discovered in process of development of custom Blockly-based application with integrated block-plus-minus plugin.

In some cases, it causes endless freeze of entire Blockly workspace after clicking "plus" button (attempt to add user-defined argument) on "Procedure" block (with active "block-plus-minus" plugin).

Test Coverage

• Probably no new tests required.
• Built-in tests are successfully passed.

Documentation

N/A

Additional Information

This error can break the entire rendering process of workspace, it will infinitely throw Uncaught TypeError: this.blockDragger_ is null in console on any gesture or mouse move. The Blockly workspace becomes unusable after triggering this error.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[gonfunko]

Thanks for reporting this! Is your application open source/otherwise available to take a look? I'm a bit confused about how an undefined value is getting in the elements array in the first place; the change works around that, but row.js declares the type of the elements array as * @type {!Array<!Measurable>}, ie undefined shouldn't be in there at all, and it would be great if we could address that root issue.

[Zauzolkov]

I've managed to prepare a small example to reproduce this issue: https://codepen.io/zauzolkov/pen/ZErwXMG

Steps to reproduce:

• Add a procedure block
• Try to add some user-defined arguments
• Entire workspace is broken (notice error logs in console)

I found that the problem is present when thrasos renderer is used. Everything works well with geras and zelos. There is probably some problem with thrasos renderer.

[Zauzolkov]

Probably related issue: #6135

[Zauzolkov]

After some debugging… Looks like I found possible roots of this issue, where pushing of undefined value to row.elements is happening. There is a check for empty array in geras renderer. Implementing a similar check in thrasos renderer resolves this issue.

https://github.com/google/blockly/blob/master/core/renderers/geras/info.js#L140-L144
https://github.com/google/blockly/blob/master/core/renderers/thrasos/info.js#L85-L86

[gonfunko]

Thanks a bunch for digging into this, I think you're exactly right that that's the root cause!