Add a default value for w.size in getWPixelSize

[sambaneko]

getWPixelSize throws the error "Uncaught (in promise) TypeError: e.size is undefined" upon Webamp initialization if the config option 'windowLayout' is used but doesn't specify all windows, or lacks a 'size' property on some windows.

So for example, either of these windowLayout values will trigger the issue:

// equalizer not specified
windowLayout: {
    main: { position: { left: 0, top: 0 } },
    playlist: {
      position: { left: 0, top: 232 },
      size: { extraHeight: 4, extraWidth: 0 },
    }
}

// playlist lacks 'size'
windowLayout: {
    main: { position: { left: 0, top: 0 } },
    equalizer: { position: { left: 0, top: 116 } },
    playlist: {
      position: { left: 0, top: 232 }
    }
}

My solution was to default 'size' to [0,0] if not present, which seems to work fine (error is resolved and windows render as expected), but not sure if this would be your preferred fix. The problem windows are designated 'open: false' so that might be checked somewhere, alternatively.

[captbaritone]

Instead of setting the default in the getter, can you apply the default to the passed in config? I think the types in the rest of Webamp assume the size property is set.

Maybe also update the config types and docs to indicate that size is now optional?

[netlify[bot]]

✅ Deploy Preview for vigorous-lalande-5190c2 ready!

Name	Link
🔨 Latest commit	9227f1d531277906b781e76208d4c2d2e403c71e
🔍 Latest deploy log	https://app.netlify.com/sites/vigorous-lalande-5190c2/deploys/65261cb7e56ecb0008b50643
😎 Deploy Preview	https://deploy-preview-1233--vigorous-lalande-5190c2.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[sambaneko]

Alternatively, I tried short circuiting the getter if window isn't open:

function getWPixelSize(w: WebampWindow, doubled: boolean) {
    if (!w.open) {
        return {height: 0, width: 0}
    }
    ...

Doing that seems fine with omitting the size property; I think my previous testing was - in both cases - throwing the error only because of unspecified (thus, not open) windows (the second example leaves out the milkdrop window).

[captbaritone]

I really don't like applying the default here. The types say this value will always have a size property. I believe everywhere within Webamp the types are checked and therefore safe. The only place we can end up with a missing size is if the user passes us a window with a missing size. I think it's fine to support that, but we should do so at the boundary where the user provides the incomplete window object.

Alternatively we can validate that input object better and throw if the size is missing.

[sambaneko]

I'd say this PR can be closed, but wanting to clarify that my original description of the issue is wrong. User passing a window with missing size is working fine; it's already applying default values in that case.

The issue may be specifically related to the milkdrop window, as this for example throws the error:

windowLayout: {
    main: { position: { left: 0, top: 0 } },
    equalizer: { position: { left: 0, top: 116 } },
    playlist: { position: { left: 0, top: 232 } }
}

Checking the window objects going through getWPixelSize() shows main, eq and playlist as expected, followed by a fourth window object that has only a single property, open, which is false. Error is then thrown as the size property is undefined.

If milkdrop is in there, or if another window is omitted, no error is thrown.