ArnoldSmith86 / virtualtabletop

a virtual surface in the browser on which you can play board, dice and card games
https://virtualtabletop.io
GNU General Public License v3.0
173 stars 31 forks source link

client crash when having invalid JSON in the editor #2341

Open ArnoldSmith86 opened 1 month ago

ArnoldSmith86 commented 1 month ago

If you currently have a JSON error in the editor and then your client receives a change for the widget from another client, it crashes.

96LawDawg commented 1 month ago

I don't know if this is the same thing or not, but there is another type of client crash with invalid JSON in the editor. Add the property dragLimit to a holder using the blue buttons then erase the curly braces and replace with a number. That invalid JSON triggers the client crash submit issue screen.

Edit: this was fixed by #2344

bjalder26 commented 4 weeks ago

jeStateNow is apparently null, when there is invalid JSON in the editor, which causes a crash because it can't find the id of jeStateNow when it is null. image

It looks like when you enter invalid JSON in the editor, the jeStateNow gets set to null in jeGetContext():

function jeGetContext() {
    const e = getSelection().anchorOffset
      , t = getSelection().focusOffset
      , o = Math.min(e, t)
      , n = Math.max(e, t)
      , a = jeGetEditorContent()
      , i = a.substr(o, Math.min(n - o, 100)).replace(/\n/g, "\\n");
    a.split("\n")[a.substr(0, o).split("\n").length - 1];
    if ("macro" == jeMode)
        return jeContext = ["Macro"],
        jeShowCommands(),
        jeContext;
    if ("empty" == jeMode)
        return jeShowCommands(),
        jeContext;
    if ("trace" == jeMode)
        return jeContext = ["Trace"],
        jeShowCommands(),
        jeContext;
    try {
        jeStateNow = JSON.parse(a.replace(/,(?=\n *[\]}],?$)/gm, "")),
        jeJSONerror = jeStateNow.id ? "string" != typeof jeStateNow.id ? "ID has to be a string." : JSON.parse(jeStateBefore).id != jeStateNow.id && widgets.has(jeStateNow.id) ? `ID ${jeStateNow.id} is already in use.` : void 0 === jeStateNow.parent || null === jeStateNow.parent || widgets.has(jeStateNow.parent) ? "card" != jeStateNow.type || jeStateNow.deck && widgets.has(jeStateNow.deck) ? "card" != jeStateNow.type || widgets.get(jeStateNow.deck).get("cardTypes") ? "card" != jeStateNow.type || jeStateNow.cardType && widgets.get(jeStateNow.deck).get("cardTypes")[jeStateNow.cardType] ? null : `Card type ${jeStateNow.cardType} does not exist in deck ${jeStateNow.deck}.` : `Given widget ${jeStateNow.deck} is not a deck or doesn't define cardTypes.` : `Deck ${jeStateNow.deck} does not exist.` : `Parent ${jeStateNow.parent} does not exist.` : "No ID given."
    } catch (n) {
        jeStateNow = null,
...

It seems to me that when a widget is moved, then either: 1) JSON in the JSON editor needs to be "thrown out" and possibly updated by the new state. In other words, if you're editing JSON, and someone moves the widget then your changes are deleted, or 2) the JSON in the JSON editor is allowed to "persist" even if it is invalid, and when it becomes valid, then it will cause a delta that will override the changes made by someone moving the widget. In other words, a moved widget might move back to the original location when the JSON is finished being edited in the JSON editor.