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

add srcObject and mozSrcObject attributes #1474

Closed
wants to merge 1 commit into from
Closed

add srcObject and mozSrcObject attributes #1474

wants to merge 1 commit into from

Conversation

yocontra
Copy link
Contributor

@yocontra yocontra commented May 1, 2014

In previous versions of firefox in chrome (pre window.URL) this is how you attached MediaStreams to a video element.

el.srcObject = stream; // chrome and opera
el.mozSrcObject = stream; // firefox

To support those browsers I am proposing that these attributes be added so you can construct a video element like so:

// chrome
React.DOM.video({
  srcObject: stream
});

// firefox
React.DOM.video({
  mozSrcObject: stream
});

(CLA signed)

In previous versions of firefox in chrome (pre window.URL) this is how you attached MediaStreams to a video element.

```js
el.srcObject = stream; // chrome and opera
el.mozSrcObject = stream; // firefox
```

To support those browsers I am proposing that these attributes be added so you can construct a video element like so:

```js
// chrome
React.DOM.video({
  srcObject: stream
});

// firefox
React.DOM.video({
  mozSrcObject: stream
});
```

(CLA signed)
@yocontra
Copy link
Contributor Author

yocontra commented May 1, 2014

Currently I have my own Video tag abstraction to handle this cross-browser stuff, is this something you would be interested in having in core? I noticed that there are built-in abstractions over some elements. The ideal usage would be simply:

React.DOM.video({
  stream: stream
});

which would internally do the dance between URL.createObjectURL, srcObject, and mozSrcObject that is basically (pseudo-code)

if (props.stream) {
  if (hasURL) {
    // new chrome
    props.src = URL.createObjectURL(props.stream);
  } else {
    // old chrome, ff, opera, ie, etc.
    props.srcObject = props.stream;
    props.mozSrcObject = props.stream;
  }
}

and release the object URL on unmount

@syranide
Copy link
Contributor

syranide commented May 7, 2014

As for your second comment, seems reasonable on the surface, React tries to hide inconsistencies where possible (given a reasonable size/benefit ratio). However, if URL.createObjectURL(...) is the standards-compliant way of doing it then it seems to me that is the only acceptable solution (but I just gathered that from your comment, I've never used it myself and don't know).

If it requires shimming URL and createObjectURL though then it doesn't really make sense for React to resolve the inconsistency any more. Rather it should shimmed/exposed by a 3rd-party component (on NPM) that can solve it using whatever approach is the most widely supported right now without having to necessarily consider the availability of future/optional features.

As for the PR itself, if it can't reasonably resolved by React then simply exposing those attributes seems reasonable. Kind of related, but only mentioning for the sake of awareness #1449.

@yocontra
Copy link
Contributor Author

yocontra commented May 7, 2014

@syranide The standard is still in flux right now, it's unclear to me if the future is URL.createObjectURL or the .srcObject attribute. If you don't have WebRTC at all (no URL, no .srcObject) then the stream attribute would not do anything.

@syranide
Copy link
Contributor

syranide commented May 7, 2014

@contra True, but looking at http://www.w3.org/TR/html5/embedded-content-0.html#the-video-element, it makes no mention of srcObject which I would at least assume means they're moving away from srcObject, but that to what isn't necessarily final.

@yocontra
Copy link
Contributor Author

yocontra commented May 7, 2014

Does react conform to the spec or to the real world implementations? If to the spec, I'll probably have to close this and fork react. If implementation, then this should be merged. The messaging is slightly confusing because vendor prefixes are available in other parts of the system.

@syranide
Copy link
Contributor

syranide commented May 7, 2014

@contra I'm no official opinion on this, but my interpretation is that React does explicitly normalize behavior, polyfill features and resolve inconsistencies as long as it is consistent with a standard and falls strictly inside of React's namespace (i.e, we don't mess with the expected behavior outside of React) when there's also a reasonable relationship of benefit and size/scope. See; the normalized and polyfilled KeyboardEvent and autoFocus for example.

If I would word it myself, it makes sense to push people in the direction of newer standards when possible, allowing you to code in React as if they were already adopted by all (capable) browsers (see; KeyboardEvent.key). This way React is wherever the standards go and the standards are designed not to break expected behavior, and React can naturally remove "browser tweaking" code over time as those browsers fall out of use. If React explicitly deviates from the standard it will be indefinitely responsible for keeping that feature working as intended.

So it seems to me that if src = URL.createObjectURL() becomes accepted (or highly likely to become accepted) but it would require globally polyfilling URL then it's a no go. So the best path for React right now seems to be your PR as it is, adding srcObject and mozSrcObject to the list of accepted attributes, then you can implement your own MyFixedVideo-component that provides a uniform API for all capable browsers (as it is best solved today without consideration for standards) and even publish it on NPM if you want!

Again, I'm no official opinion on this but it's my honest interpretation of my experience, as I know the "official devs" have been very busy the last few weeks. So they may not be able to reply to everyone in a timely manner.

@yocontra
Copy link
Contributor Author

yocontra commented May 7, 2014

@syranide Yep that path sounds good to me

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@petehunt
Copy link
Contributor

ping @zpao, want to take this?

@yocontra
Copy link
Contributor Author

Closing in favor of a new (and broader) PR

@eskimor
Copy link

eskimor commented Nov 29, 2016

Hmm -this is really old. srcObject is standard these days: https://html.spec.whatwg.org/multipage/embedded-content.html#media-elements and implemented by major browsers, it is also more convenient than the objectURL approach, which requires manual resource cleanup. Can we allow srcObject in react?

@Jxck
Copy link

Jxck commented Feb 6, 2017

same problem here.
for WebRTC Apps, URL.objectCreateURL to vide.src has been deprecated.
so now it's necessary for video tag to support srcObject.

https://www.fxsitecompat.com/en-CA/docs/2017/url-createobjecturl-stream-has-been-deprecated/

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.

None yet

6 participants