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

Autoplay checkbox in demo UI is confusingly named #1114

Closed
sandersaares opened this issue Nov 8, 2017 · 9 comments
Closed

Autoplay checkbox in demo UI is confusingly named #1114

sandersaares opened this issue Nov 8, 2017 · 9 comments
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Milestone

Comments

@sandersaares
Copy link
Contributor

sandersaares commented Nov 8, 2017

I was just trying to inspect the state of the player before any playback in the demo app and realized it was starting playback even though I had not checked the autoplay checkbox. I assume that should not happen?

The state of the checkbox seems to have no impact on what happens after I press the load button. Tested on Chrome 62 and Firefox 56 by entering custom URL.

@joeyparrish joeyparrish added the status: working as intended The behavior is intended; this is not a bug label Nov 8, 2017
@joeyparrish
Copy link
Member

The demo app does its own autoplay at the app level, because the autoplay attribute on the video element doesn't work on Android. Take a look at demo/asset_section.js, the bottom of shakaDemo.load(), which says:

  // While the manifest is being loaded in parallel, go ahead and ask the video
  // to play.  This can help with autoplay on Android, since Android requires
  // user interaction to play a video and this function is called from a click
  // event.  This seems to work only because Shaka Player has already created a
  // MediaSource object and set video.src.
  shakaDemo.video_.play();

If you create your own application and don't call video.play(), you will find that the autoplay attribute does what you would expect (except on Android, where it is often ignored).

@sandersaares
Copy link
Contributor Author

Well, okay then. I was just looking at a checkbox saying "autoplay" not being checked and the video starting playback automatically, which seemed wrong to me as a plain user of the demo app. But if it is by design then it is by design, I guess.

@joeyparrish
Copy link
Member

Where is this checkbox? I don't see it in Chrome on Linux.

If this is a thing users can toggle outside of our demo UI, we could make the play() call dependent on the state of video.autoplay. It would be simple and might remove a source of surprise for users of the demo app.

I just want to find where that checkbox is so I can reproduce.

@joeyparrish joeyparrish reopened this Nov 8, 2017
@joeyparrish joeyparrish added needs triage and removed status: working as intended The behavior is intended; this is not a bug labels Nov 8, 2017
@joeyparrish
Copy link
Member

I didn't see it any checkbox in Firefox on Linux, either. What browser & OS are you using, and where does the autoplay checkbox appear?

@sandersaares
Copy link
Contributor Author

sandersaares commented Nov 9, 2017

Illustration attached (Windows 10, all browsers as far as I can tell):

image

@joeyparrish
Copy link
Member

Ah! I see!

Here's the source of confusion:

HTMLMediaElements have an autoplay attribute which has the effect of calling play() when a source is loaded. If this attribute is set, most platforms (not always on Android) will start playing the video as soon as you call player.load(). I was looking for a checkbox for that provided by the browser, similar to the one you get for the "loop" attribute when you right-click a video element.

That checkbox in configuration is something else. It has the effect of automating the "Load" button when you refresh the page. It's useful for manual testing and debugging, when you need to edit some code and reload the page to quickly reproduce an issue. It's also useful in bug reports, so that bugs reproduce when the page loads.

The checkbox you're referring to should not be named "Autoplay", to avoid confusion. Instead, we should call it something closer to what it does: "Load content on page load".

Does that makes sense? If that explanation helps, we will go ahead with renaming the checkbox in the demo UI.

@joeyparrish joeyparrish added type: bug Something isn't working correctly and removed needs triage labels Nov 9, 2017
@joeyparrish joeyparrish added this to the v2.3.0 milestone Nov 9, 2017
@joeyparrish joeyparrish changed the title Demo app starts playback even when autoplay checkbox is not checked Autoplay checkbox in demo UI is confusingly named Nov 9, 2017
@sandersaares
Copy link
Contributor Author

Yes, that sounds like it would indeed be an improvement.

@joeyparrish
Copy link
Member

Excellent. Thanks for the feedback!

joeyparrish added a commit that referenced this issue Nov 13, 2017
This is to disambiguate with video.autoplay and to more clearly state
what this checkbox does.

Closes #1114

Change-Id: I23138957bda1fe6159b42bd4ae3327a426d379f9
@joeyparrish
Copy link
Member

The name change has been cherry-picked to v2.2.6.

@shaka-project shaka-project locked and limited conversation to collaborators Mar 22, 2018
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Apr 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Development

No branches or pull requests

3 participants