Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: skip addition of src attribute for HLS videos #1446

Merged
merged 2 commits into from
Oct 1, 2024
Merged

Conversation

daibhin
Copy link
Contributor

@daibhin daibhin commented Oct 1, 2024

Changes

Follow on from problem mentioned in PostHog/posthog#25174 (comment)

When testing locally by adding the hls-src attribute to an exported recording I hadn't considered that actual data would add a src attribute to the video, which is picked up as a mutation.

This local URL (something akin to blob:http://localhost:3000/4200b9dc-71b1-42f9-a753-486f93222f0a) then overrides the src that we need to write during the plugin playback.

Rather than trying to detect the insertion of the src attribute using the handler and skip it (not sure this is possible with plugins) we should be able to ignore src attributes on videos that point to local files e.g. HLS videos

Copy link

vercel bot commented Oct 1, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
posthog-js ✅ Ready (Inspect) Visit Preview Oct 1, 2024 0:38am

@daibhin daibhin requested a review from a team October 1, 2024 12:38
@daibhin daibhin added the bump patch Bump patch version when this PR gets merged label Oct 1, 2024
@daibhin daibhin changed the title add hls example fix: skip addition of src attribute for HLS videos Oct 1, 2024
Copy link

github-actions bot commented Oct 1, 2024

Size Change: +392 B (+0.01%)

Total Size: 2.81 MB

Filename Size Change
dist/all-external-dependencies.js 191 kB +56 B (+0.03%)
dist/array.full.js 351 kB +56 B (+0.02%)
dist/array.full.no-external.js 350 kB +56 B (+0.02%)
dist/module.full.js 351 kB +56 B (+0.02%)
dist/module.full.no-external.js 350 kB +56 B (+0.02%)
dist/recorder-v2.js 111 kB +56 B (+0.05%)
dist/recorder.js 111 kB +56 B (+0.05%)
ℹ️ View Unchanged
Filename Size
dist/array.js 167 kB
dist/array.no-external.js 166 kB
dist/exception-autocapture.js 10.5 kB
dist/external-scripts-loader.js 2.35 kB
dist/main.js 167 kB
dist/module.js 167 kB
dist/module.no-external.js 166 kB
dist/surveys-preview.js 59.8 kB
dist/surveys.js 66 kB
dist/tracing-headers.js 8.36 kB
dist/web-vitals.js 10.3 kB

compressed-size-action

@daibhin daibhin merged commit 79e0657 into main Oct 1, 2024
17 of 18 checks passed
@daibhin daibhin deleted the dn-fix/hls-support branch October 1, 2024 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bump patch Bump patch version when this PR gets merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants