Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSON deserialization fails for collapsed procedure call blocks containing at least one input variable #6091

Closed
cmjcharlton opened this issue Apr 20, 2022 · 12 comments
Labels
issue: bug Describes why the code or behaviour is wrong

Comments

@cmjcharlton
Copy link

cmjcharlton commented Apr 20, 2022

Describe the bug

If I attempt to load a workspace from JSON containing a collapsed procedure call block with at least one input variable then this block is missing from the loaded workspace.

To Reproduce

Steps to reproduce the behavior:

  1. Open the Blockly playground
  2. Click 'Functions' in the toolbox
  3. Drag a procedure block into the workspace
  4. Open its mutator and add an input
  5. Right-click the block and select 'Create "do something"'
  6. Right-click the created procedure call block and collapse it
  7. Click 'Save JSON'
  8. Click 'Load'

Expected behavior

Both the procedure definition and call procedure blocks should be present in the loaded workspace.

Screenshots

Workspace to be saved:
image

Workspace after load:
image

Desktop (please complete the following information):

  • OS: [Windows 10]
  • Browser [Chrome, Firefox]
  • Version [Chrome 100.0.4896.127 (Official Build) (64-bit), Firefox 99.0.1 (64-bit)]

Stack Traces

block.js:2022 Uncaught Error: Input not found: _TEMP_COLLAPSED_INPUT
    at BlockSvg.module$exports$Blockly$Block.Block.removeInput (block.js:2022:11)
    at BlockSvg.module$exports$Blockly$BlockSvg.BlockSvg.removeInput (block_svg.js:1376:27)
    at BlockSvg.module$exports$Blockly$BlockSvg.BlockSvg.updateCollapsed_ (block_svg.js:673:12)
    at BlockSvg.module$exports$Blockly$BlockSvg.BlockSvg.setCollapsed (block_svg.js:647:12)
    at BlockSvg.setProcedureParameters_ (procedures.js:739:10)
    at BlockSvg.loadExtraState (procedures.js:899:12)
    at module$contents$Blockly$serialization$blocks_loadExtraState (blocks.js:452:11)
    at module$contents$Blockly$serialization$blocks_appendPrivate (blocks.js:394:3)
    at module$exports$Blockly$serialization$blocks.appendInternal (blocks.js:345:17)
    at module$exports$Blockly$serialization$blocks.append (blocks.js:308:10)

Additional context

Although it involves collapsed blocks I don't believe that this is the exactly the same as issue #6076 as it appears to be related to extraState rather than icon visibility, however again the error does not appear to occur if saving/loading via XML. It also does not occur for either deserialization method if the procedure call block contains no input variables.

@cmjcharlton cmjcharlton added the issue: triage Issues awaiting triage by a Blockly team member label Apr 20, 2022
@tweini
Copy link
Contributor

tweini commented Apr 21, 2022

Hi! I just finished the fix for #6076 and can confirm, that it doesn't fix this problem. I am into it and can provide a fix within a few days, stay tuned 😄

@cmjcharlton
Copy link
Author

Thanks very much, that would be fantastic. I am currently working around both problems by pre-processing the JSON to expand collapsed blocks prior to deserialization, and then collapsing them again after the workspace is rendered. It will be nice to be able to remove my hack once this is fixed.

@tweini
Copy link
Contributor

tweini commented Apr 21, 2022

Hi @cmjcharlton bug is identified, fix follows. If you have some time, maybe you can do some manual test for my provided fix for #6076? Maybe you can find edge cases I haven't think of.

@gonfunko gonfunko added issue: bug Describes why the code or behaviour is wrong and removed issue: triage Issues awaiting triage by a Blockly team member labels Apr 21, 2022
@gonfunko
Copy link
Contributor

Thank you both for reporting and investigating this! I can repro the behavior you described.

tweini added a commit to tweini/blockly that referenced this issue Apr 21, 2022
@tweini
Copy link
Contributor

tweini commented Apr 21, 2022

Both types (i.e. "procedures_callnoreturn" and procedures_callreturn" ) are affected. The Precondition is: Call block contains at least one input variable

@cmjcharlton cmjcharlton changed the title JSON deserialization fails for collapsed procedure call blocks containing more than one input variable JSON deserialization fails for collapsed procedure call blocks containing at least one input variable Apr 24, 2022
@cmjcharlton
Copy link
Author

Thanks @tweini for pointing out my off-by-one error in the title/description (now corrected). I'm not sure how I managed to do that given my replication example only included one input.

tweini added a commit to tweini/blockly that referenced this issue Apr 27, 2022
BeksOmega pushed a commit that referenced this issue Apr 27, 2022
#6103)

* fix: JSON deserialization fails (bug #6091) (collapsed procedure call blocks)

* fix: JSON deserialization fails (bug #6091) changed fix, added tests (collapsed procedure call blocks)
@tweini
Copy link
Contributor

tweini commented Apr 27, 2022

fixed ✅ @BeksOmega, @cmjcharlton, who wants to close the issue?

@cmjcharlton
Copy link
Author

I just tested the develop branch of Blockly with my application and can confirm that this appears to fix the problem that I was seeing. I would therefore be happy to see the issue closed. Thanks very much for finding the cause, and fixing it.

@BeksOmega
Copy link
Collaborator

Closed by #6103

BeksOmega pushed a commit to BeksOmega/blockly that referenced this issue Apr 28, 2022
…e call… (google#6103)

* fix: JSON deserialization fails (bug google#6091) (collapsed procedure call blocks)

* fix: JSON deserialization fails (bug google#6091) changed fix, added tests (collapsed procedure call blocks)
BeksOmega pushed a commit to BeksOmega/blockly that referenced this issue Apr 28, 2022
…e call… (google#6103)

* fix: JSON deserialization fails (bug google#6091) (collapsed procedure call blocks)

* fix: JSON deserialization fails (bug google#6091) changed fix, added tests (collapsed procedure call blocks)
BeksOmega pushed a commit to BeksOmega/blockly that referenced this issue Apr 28, 2022
…e call… (google#6103)

* fix: JSON deserialization fails (bug google#6091) (collapsed procedure call blocks)

* fix: JSON deserialization fails (bug google#6091) changed fix, added tests (collapsed procedure call blocks)
@BeksOmega
Copy link
Collaborator

@cmjcharlton We've released patches for v7 and v8 containing this fix (and the fix to #6076) if you want to give them a try =)

@cmjcharlton
Copy link
Author

Thanks @BeksOmega for the new update. I updated to the new patched version and removed the workarounds that I previously had in my application. I am no longer seeing the errors that I did before and everything seems to be working so far. I also checked the v7 backport version and that seems to be fixed for me too.

@BeksOmega
Copy link
Collaborator

Thanks @BeksOmega for the new update. I updated to the new patched version and removed the workarounds that I previously had in my application. I am no longer seeing the errors that I did before and everything seems to be working so far. I also checked the v7 backport version and that seems to be fixed for me too.

That's awesome!!! Thank you so much for giving them a test for me :D Really appreciate your time 🌻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue: bug Describes why the code or behaviour is wrong
Projects
None yet
Development

No branches or pull requests

4 participants