Skip to content

fix: forward extended event payload as trigger hash, not event property - #1091

Open
adimiz1 wants to merge 2 commits into
masterfrom
fix/extended-events-data-forwarding
Open

adimiz1 wants to merge 2 commits into
masterfrom
fix/extended-events-data-forwarding

Conversation

@adimiz1

@adimiz1 adimiz1 commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • _setExtendedEvents() forwarded internal extended-event payloads to the public player event bus via this.videojs.trigger({ type: _event, eventData: data }), passing only one argument to trigger().
  • videojs.trigger(event, hash) delivers hash as the second (data) argument to listeners — since it wasn't passed, every listener attached the normal way (player.on(type, (event, data) => ...)) received data === undefined.
  • Affects every extended event: percentsplayed, timeplayed, playerload, start, seek, pausenoseek, mute, unmute, qualitychanged — including the built-in analytics plugin, which destructures data and throws.

Fix: pass the payload as the actual trigger hash — this.videojs.trigger(_event, data).

Found while manually verifying an unrelated analytics fix on docs/analytics.html (percentsplayed enabled by default): page threw TypeError: Cannot destructure property 'percent' of 'data' as it is undefined on load.

Test plan

  • Manually reproduced on master: loaded docs/analytics.html (default config, percentsplayed events enabled), confirmed uncaught TypeError: Cannot destructure property 'percent' of 'data' as it is undefined.
  • Applied fix, reloaded demo, confirmed no throw and percentsplayed/timeplayed fire correctly while playing/seeking.
  • Reviewer: run npm run test:unit for regressions in extended-events / analytics coverage.

🤖 Generated with Claude Code

`this.videojs.trigger({ type: _event, eventData: data })` only passes
one argument to trigger(), so the payload never reaches listeners as
their second `data` argument. Every extended event (percentsplayed,
timeplayed, playerload, start, seek, pausenoseek, mute, unmute,
qualitychanged) fired with `data` undefined, breaking any listener
(including the analytics plugin) that destructures it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@adimiz1
adimiz1 requested a review from a team as a code owner September 20, 2026 07:36
@netlify

netlify Bot commented Sep 20, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for cld-video-player ready!

Name Link
🔨 Latest commit 70caf61
🔍 Latest deploy log https://app.netlify.com/projects/cld-video-player/deploys/6aaf8dcaded76b0008fcf092
😎 Deploy Preview https://deploy-preview-1091--cld-video-player.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@netlify

netlify Bot commented Sep 20, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for cld-vp-esm-pages ready!

Name Link
🔨 Latest commit 70caf61
🔍 Latest deploy log https://app.netlify.com/projects/cld-vp-esm-pages/deploys/6aaf8dca15c65500087341e4
😎 Deploy Preview https://deploy-preview-1091--cld-vp-esm-pages.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

The prior assertion spied on trigger()'s arguments and expected the
buggy single-object shape ({ type, eventData }), so it passed against
the broken code and started failing once the fix corrected the call
to trigger(_event, data). Assert what a real listener receives instead
- the actual behavior that was broken.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants