Clamp glass, lens and flash numbers, and restrict music.src, in normalize()

Open Beginner friendly
  • Is something wrong?
#5 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
88/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
javascript, node.js
Domain
frontend, security, testing

Research direction

Read web/engine/storyboard.mjs around the generic number clamp and music source handling, then inspect the inspector inputs in web/app.mjs and the mentioned layer renderers. Add the normalization case to test/core.test.mjs and run npm test. Done means values are clamped, external music URLs are dropped while /library/audio/ sources remain, and the Motion inputs have matching min and max attributes.

Written by the indexing model from the issue text.

Description

bug good first issue

normalizeLayer() clamps every numeric default to ±1e12, so layer-specific limits are never enforced:

normalize({ scenes: [{ layers: [{ type: 'glass', size: -2, zoom: 0 }, { type: 'lens', size: -1 }, { type: 'flash', strength: 50 }] }] })
// glass size -2, zoom 0; lens size -1; flash strength 50 — all kept

A negative size matters. In lens() it gives a negative radius to roundRect, and in glass() it gives negative radii to ellipse. The canvas spec makes both of those throw (RangeError and IndexSizeError). The throw escapes renderer.draw() inside tick() before requestAnimationFrame(tick) is re-queued, so the preview stops. A zoom of 0 divides by zero in the glass refraction drawImage.

These values can come from a director model, an imported .json, or the Motion tab. Its Size, Zoom and Strength inputs are plain number() fields with no min or max.

There is a related gap in the same function. Image sources go through librarySrc(), which only allows /library/ and data:image/. music.src accepts any string, so an imported project can make the export fetch() an arbitrary URL.

Task:

  • clamp the per-type fields: glass size 0.05–1 and zoom 1–3, lens size 0.05–1, flash strength 0–1 (or whatever ranges look right in the preview)
  • pass matching min and max to the inspector inputs
  • only accept music.src values under /library/audio/
  • add a test to test/core.test.mjs that normalizes the bad values above

Where: web/engine/storyboard.mjs:61 (generic number clamp), web/engine/storyboard.mjs:113 (music); web/engine/layers.mjs:387-399 (lens), web/engine/layers.mjs:403-438 (glass); web/app.mjs:389-390 (inspector number fields), web/app.mjs:115-125 (tick).

Done when: a new test in test/core.test.mjs passes under npm test: normalizing the storyboard above gives glass/lens size > 0, glass zoom ≥ 1 and flash strength ≤ 1, a music.src of https://example.com/a.mp3 is dropped while /library/audio/... is kept; the Motion-tab Size, Zoom and Strength inputs carry matching min/max attributes.

Getting started: Node 22, no npm install needed: git clone, then npm test and npm run check. Comment here to claim it. Questions → Discord.

Dominant language
JavaScript
Stars
42
Forks
6
Avg merge
12h 31m
Merged PRs (30d)
3

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from blixvip/GenMotion

All issues in blixvip/GenMotion

Similar issues

More JavaScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.