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

support src being an empty string #2852

Closed
gkatsev opened this issue Nov 25, 2015 · 18 comments
Closed

support src being an empty string #2852

gkatsev opened this issue Nov 25, 2015 · 18 comments
Assignees

Comments

@gkatsev
Copy link
Member

gkatsev commented Nov 25, 2015

the spec basically says that when the src is set to an empty string, you want to reset the player and then stop and not try to loading anything. See step 9 of http://www.w3.org/TR/html5/embedded-content-0.html#concept-media-load-algorithm

@Soviut
Copy link
Contributor

Soviut commented Nov 26, 2015

This is currently how I test my error styles. Perhaps it would be a good idea to document how to put the player into an error state for error testing.

@gkatsev
Copy link
Member Author

gkatsev commented Nov 26, 2015

You can trigger an error by calling player.error() with an object that has a code property and various others: player.error({code: 4}).

@Soviut
Copy link
Contributor

Soviut commented Nov 26, 2015

Ah nice, I assumed there was some way to do it; I should have guessed it was that simple.

@gkatsev
Copy link
Member Author

gkatsev commented Dec 2, 2015

So, after trying playing around with setting the src to '', I realized that it does throw an error. Tested on this page: http://www.w3.org/2010/05/video/mediaevents.html
However, setting the src to null didn't throw and that might be an acceptable use.
Also, perhaps we want something like a player.unload() instead of setting the src to null?

@Soviut
Copy link
Contributor

Soviut commented Dec 2, 2015

Could we not just assume that an empty call to player.src() means remove call sources?

Also, I'd expect player.unload() to remove videoJS from the target and return the <video> tag to its original state, not change sources.

@gkatsev
Copy link
Member Author

gkatsev commented Dec 2, 2015

@Soviut player.src() needs to act as a getter.
I guess unload isn't the right word for it. Just not sure that player.src(null) is the best way of implementing the user facing API for this.

@Soviut
Copy link
Contributor

Soviut commented Dec 2, 2015

Perhaps just document that player.src({}) or player.src([]) could clear the sources. That makes the most sense since it's empty/zero source.

@gkatsev
Copy link
Member Author

gkatsev commented Dec 2, 2015

Well, what I specifically want to happen with src(null) is to change the tech to Html5, and put in there a simple video element with no source. Basically, unset all state of the player as much as possible.

@Soviut
Copy link
Contributor

Soviut commented Dec 2, 2015

That sounds more like a reset. Perhaps a player.reset() or player.init() is a good idea and then that can be called if src is set to null as well.

@dmlap
Copy link
Member

dmlap commented Dec 2, 2015

If facilities exist in the HTML spec to "reset the video element" than I think we should stick to those alone. Having two ways of doing the same thing can be confusing.

@gkatsev
Copy link
Member Author

gkatsev commented Dec 2, 2015

Trying to read the spec to see if there's a "reset" functionality. Seems like it might be just removing all source elements, deleting the src attribute, and then calling load(). Which is what I have hacked together locally currently.

@misteroneill
Copy link
Member

I think player.reset() makes sense in that case. It's more meaningful to me than player.src(null).

@gkatsev
Copy link
Member Author

gkatsev commented Dec 2, 2015

Actually, I found the spec say exactly that: http://www.w3.org/TR/html5/embedded-content-0.html#best-practices-for-authors-using-media-elements

emoving the element's src attribute and any source element descendants, and invoking the element's load() method.

@dmlap
Copy link
Member

dmlap commented Dec 3, 2015

Well, questions about reset() aside, sounds like we should support nulling the src and calling load()

@gkatsev
Copy link
Member Author

gkatsev commented Dec 3, 2015

As in ↓?

player.src(null);
player.load();

Except it would cause an error to load because it will translate to

video.src = null;
video.load();

Which does cause an error. Unless we want special handling for src(null) which would clear out the source element and delete the attribute?

@dmlap
Copy link
Member

dmlap commented Dec 3, 2015

Ah, I overlooked the removal aspect. Tricky.

@gkatsev
Copy link
Member Author

gkatsev commented Dec 3, 2015

We've decided to go with Player#reset unless we figure out a better name for this before we merge in the PR :)

@gkatsev
Copy link
Member Author

gkatsev commented Dec 3, 2015

I have made a PR that adds the reset method to the player. Check it out in #2880.

@gkatsev gkatsev closed this as completed in e78e26b Dec 7, 2015
jgubman added a commit to jgubman/video.js that referenced this issue Jan 27, 2016
* upstream/stable: (479 commits)
  v5.4.4
  @gkatsev switched to use custom vtt.js from npm. closes videojs#2905
  v5.4.3
  @gkatsev updated options customizer and github-release options. closes videojs#2903
  v5.4.2
  @gkatsev updated grunt-release config. closes videojs#2900
  v5.4.1
  @gkatsev added chg- and github- release for next releases. closes videojs#2899
  v5.4.0
  @gkatsev added ability to release next tag from master. closes videojs#2894
  @gkatsev added nullcheck for cues in updateForTrack. Fixes videojs#2870. closes videojs#2896
  @chemoish emulated HTMLTrackElement to enable track load events. closes videojs#2804
  @gkatsev added a Player#reset method. Fixes videojs#2852. closes videojs#2880
  @nick11703 changed multiline comments in sass with single-line comments. closes videojs#2827
  @gkatsev added Player#tech. Fixes videojs#2617. closes videojs#2883
  @misteroneill updated videojs-ie8 to 1.1.1. closes videojs#2869
  v5.3.0
  @imbcmdth added sourceOrder option for source-first ordering in selectSource. closes videojs#2847
  @forbesjo updated formatTime to not go negative. closes videojs#2821
  v5.2.4
  ...
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants