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

expo-av doesn't forward "unknown" web-only props on web #21141

Open
SleeplessByte opened this issue Feb 9, 2023 · 1 comment
Open

expo-av doesn't forward "unknown" web-only props on web #21141

SleeplessByte opened this issue Feb 9, 2023 · 1 comment

Comments

@SleeplessByte
Copy link

Summary

It may be preferable to disable the download and fullscreen button an a video element (even though, in general, it's not hard for someone to script their way around this). In this case we can use controlsList when working with normal HTML.

The other thing we may want to do is disable Picture-In-Picture, which can be achieved using the disablePictureInPicture attribute.

Both things would be supported if the Video component forwarded these web properties, which is done for some other components (despite these being "web-only" properties!).

The reason this doesn't work for video is because the Video element throws away unknown properties here:
https://github.com/expo/expo/blob/main/packages/expo-av/src/ExponentVideo.web.tsx#L144

Despite the following invocations allowing it:
https://github.com/expo/expo/blob/main/packages/expo-av/src/Video.tsx#L355
https://github.com/expo/expo/blob/main/packages/expo-av/src/ExponentVideo.web.tsx#L40

Workaround

The workaround is to use a nativeId and set these properties using JavaScript, each render, because each render it will reset to the old way.

Solution

I think the preferred solution would be to at least allow forwarding these web only properties, just like how many other components have other types of Android or iOS only properties.

What platform(s) does this occur on?

Web

Environment


  expo-env-info 1.0.5 environment info:
    System:
      OS: Windows 10 10.0.22621
    Binaries:
      Node: 16.19.0 - ~\scoop\apps\nvm\current\nodejs\nodejs\node.EXE
      Yarn: 3.4.1 - ~\scoop\apps\nvm\current\nodejs\nodejs\yarn.CMD
      npm: 8.19.3 - ~\scoop\apps\nvm\current\nodejs\nodejs\npm.CMD
    npmPackages:
      @expo/webpack-config: ^0.17.4 => 0.17.4
      expo: ^47.0.13 => 47.0.13
      react: 18.1.0 => 18.1.0
      react-dom: 18.1.0 => 18.1.0
      react-native: 0.70.5 => 0.70.5
      react-native-web: ~0.18.12 => 0.18.12
    Expo Workflow: managed

Minimal reproducible example

https://snack.expo.dev/@derk-jan/web-video-controlslist

@SleeplessByte SleeplessByte added the needs validation Issue needs to be validated label Feb 9, 2023
@brentvatne brentvatne added Issue accepted and removed needs validation Issue needs to be validated labels Feb 10, 2023
@expo-bot
Copy link
Collaborator

Thank you for filing this issue!
This comment acknowledges we believe this may be a bug and there’s enough information to investigate it.
However, we can’t promise any sort of timeline for resolution. We prioritize issues based on severity, breadth of impact, and alignment with our roadmap. If you’d like to help move it more quickly, you can continue to investigate it more deeply and/or you can open a pull request that fixes the cause.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants