Fix shapes update#7424
Conversation
There was a problem hiding this comment.
Some typos. Should be otherwise the _reconnect() scenario misses the cache and fails
|
@oslego Done with review. You also need to remove a duplicate unit test on line 470-479 since it is already on line 460-469. |
There was a problem hiding this comment.
Still one typo: otherwhise
|
@oslego Looks good. Before I merge it, do you want to remove |
|
Will do it in a few hours. Need to step out for a while. Either the merge can wait or I can do another PR later with the removal. |
|
Merging now since we need to investigate a couple of issues with the latest code. |
There was a problem hiding this comment.
@oslego We should also return if (!data). Otherwise, we need to check data again before accessing it on line 153 data.forceUpdate.
Attempts to recover from errors by reconnecting the editor. If that's not possible, turns off everything so the user can trigger it again manually.
These are invalid shape declarations and the editor will turn off:
circle(px at 50% 50%)circle(50% 50%)circle(50%, 50%)This is a valid shape declaration:
circle(50% at)but it crashes the browser due to an implementation bug in the browser. The issue has been fixed, but it will take time for the fix to trickle down into Chrome Stable. That notation will crash the browser before the shapes editor has a chance to load and attempt to catch it.