VideoPress: Add video settings to the VideoPress-enhanced video block

[mmtr]

Fixes #11837.

Changes proposed in this Pull Request:

This PR adds the video settings allowing to customize several options for the VideoPress player rendered by the video block (extended by our VideoPress extension for Gutenberg).

Now, after inserting a VideoPress-enhanced video block, a user can set the following settings:

• Autoplay: whether to start playing as soon as the player renders (note that some browsers can prevent a video from being autoplayed).
• Loop: whether to loop the video.
• Muted: whether the video should start in a muted state.
• Playback controls: whether show the controls to allow the user to control video playback.
• Preload: what content is loaded before the video is played:
  • None: Indicates that the video should not be preloaded.
  • Metadata: Indicates that only video metadata (e.g. length) is fetched.
  • Auto: Indicates that the whole video file can be downloaded, even if the user is not expected to use it.
• Poster image: image to be shown while the video is downloading.

Given that these settings modifies the URL saved in the block content, any existing post containing a VideoPress-enhanced video block would result in a "invalid content" error. In order to support those posts, a deprecated version of the block has been added.

This PR also introduces a new playsInline attribute (added in core in WordPress/gutenberg#14500), but it's currently unmodifiable, since VideoPress doesn't support it yet.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

See pafL3P-cU-p2 (Phase 3).

Testing instructions:

• Create a new JN site using this branch.
• Connect to WP.com using a premium subscription.
• Go to Jetpack → Settings → Performance and activate the VideoPress module (by enabling the "Enable high-speed, ad-free video player" option).
• Go to Posts → New.
• Insert a video block.
• Upload or select an existing video.
  • Note that VideoPress only converts videos uploaded via the Media Library, so don't use the "Upload" button (bug already reported: VideoPress: allow users to upload videos from Gutenberg [2] #11194).
• Make sure the VideoPress player is previewed on the editor.
• Make sure the video settings appear in the sidebar.
• Set any video setting you like.
• Make sure the previewed player reflects the video settings you have set.
• Publish the post.
• Check the published view and verify the player reflects the video settings you have set in the editor.

Proposed changelog entry for your changes:

• VideoPress: Add video settings to the VideoPress-enhanced video block.

[matticbot]

Caution: This PR has changes that must be merged to WordPress.com
Hello mmtr! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D30868-code before merging this PR. Thank you!

[jetpackbot]

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: August 6, 2019.
Scheduled code freeze: July 30, 2019

Generated by 🚫 dangerJS against 765e372

[gwwar]

Hmm @mmtr looks like tests are failing in D30868-code from a timeout. Is there a way of triggering another run?

[gwwar]

Nice work @mmtr! I always ❤️ the detailed testing instructions.

I smoke tested on a Jetpack site with various combinations of settings applied. No errors on second edit.

I think folks can get a video that doesn't play if they turn off controls / don't have autoplay/loop on, but I think we allowed the before.

[mmtr]

Hmm @mmtr looks like tests are failing in D30868-code from a timeout. Is there a way of triggering another run?

I just restarted the build in the diff. Let's see if they pass now.

[mmtr]

Just realized that the deprecated version included on this PR doesn't work when using the latest Gutenberg plugin version due to the changes introduced by WordPress/gutenberg#16348.

To reproduce the problem:

• Install and activate the latest version of the Gutenberg plugin.
• Open an existing post containing a video block that has been inserted with the current codebase of the master branch.
• The block will trigger an invalid content error:

Will investigate if the deprecations should be defined in a different way.

Pinging @talldan @epiqueras @youknowriad as well, since they seem to have been involved in the recents changes for deprecations.

[matticbot]

mmtr, Your synced wpcom patch D30868-code has been updated.

[mmtr]

Just realized that the deprecated version included on this PR doesn't work when using the latest Gutenberg plugin version due to the changes introduced by WordPress/gutenberg#16348.

Noted that this is caused by the deprecations returned by the blocks.registerBlocks filter used in the VideoPress extension to customize the core/video block.

After WordPress/gutenberg#16348, the deprecations now trigger the blocks.registerBlocks filter too, so our VideoPress extension was changing the original deprecations.

Added a workaround in 6a89012 that ensures that our extension doesn't run when the blocks.registerBlocks filter is triggered by a deprecation.

[epiqueras]

Added a workaround in 6a89012 that ensures that our extension doesn't run when the blocks.registerBlocks filter is triggered by a deprecation.

@youknowriad @talldan Maybe we should provide a way for the filter to infer whether it's running on a deprecation or not.

[gwwar]

I verified that this no longer breaks existing video blocks on edit. @mmtr is setting the poster image working for you from wp-admin?

[mmtr]

@mmtr is setting the poster image working for you from wp-admin?

Yup, but I noted that the previewed video player doesn't reflect always the settings I defined on the sidebar. Seems to be the same behavior as noted by @kwight at D30868-code#616950.

However, the player that appears on the published view is always correct. Will try to determine what's causing this.

[mmtr]

Yup, but I noted that the previewed video player doesn't reflect always the settings I defined on the sidebar. Seems to be the same behavior as noted by @kwight at D30868-code#616950.

However, the player that appears on the published view is always correct. Will try to determine what's causing this.

Turned out to be a bug in core caused by the SandBox component not being re-rendered when the injected HTML changes: WordPress/gutenberg#16831

We render a SandBox component to embed the preview.

Note that his only happens when we want to preview a URL that was already previewed before, since these previews are cached, so the block doesn't render the "Generating preview..." placeholder and will keep using the same SandBox component that was rendered before.

If the preview has never been generated, the block will render first the "Generating preview..." placeholder and then a new instance of the SandBox component, where the above issue is not present given the component has been freshly rendered.

I'm not sure if there is any workaround for this (AFAIK we cannot force a re-rendering of a child component from the parent), but if this is a blocker we could try something like invalidating the cached previews, so we make sure we render a new instance of the SandBox component every time.

[simison]

Please rebase with #12926

[mmtr]

I'm not sure if there is any workaround for this (AFAIK we cannot force a re-rendering of a child component from the parent), but if this is a blocker we could try something like invalidating the cached previews, so we make sure we render a new instance of the SandBox component every time.

Added a workaround in ac48fe8 that invalidates the cached previews before generating a new one.

[youknowriad]

@youknowriad @talldan Maybe we should provide a way for the filter to infer whether it's running on a deprecation or not.

This sounds reasonable to me.

[talldan]

Sorry for not responding, must have missed the notification. I'll work on a PR in gutenberg for this, shouldn't take long to implement.

[jeherve]

This is pretty cool, I love that it works for both local videos and VideoPress videos, nice work!

It tests well, I only found issues with VideoPress videos, when flipping one toggle and then turning it off. If you do that with the Mute toggle for example, &muted=true is added to the URL, and then it is switched to &muted=false.

• I think it would be cleaner if the parameter were removed altogether instead of keeping it with the false value
• muted=false does not actually work. The video is muted as soon as you have the parameter, regardless of its value; in all the examples below we get a muted video.
  • https://videopress.com/v/gj2bX2G4?muted=false
  • https://videopress.com/v/gj2bX2G4?muted=true
  • https://videopress.com/v/gj2bX2G4?muted=potato

[matticbot]

mmtr, Your synced wpcom patch D30868-code has been updated.

[mmtr]

Thanks for that suggestion @jeherve! Actually it makes sense to don't include any setting in the URL that will have the same effect as the default player, so the URL will look cleaner.

I applied some changes in 765e372 based on that approach, and now the VideoPress URLs will contain only the parameters having a value that differs from the default VideoPress player settings.

[jeherve]

This works better now, everything appears to be working for me right now, except for autoplay; does it work for you?

https://videopress.com/v/gj2bX2G4?autoPlay=true&loop=true&muted=true&persistVolume=false&preloadContent=metadata

I tried in both Chrome and Firefox. Do those browsers not allow autoplay at all?

[kwight]

The video is muted as soon as you have the parameter, regardless of its value

This seems to be the intended behaviour, for the autoplay attribute too: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/video. Removing the value will also clear up the inconsistency of true/false vs. 1/0 (eg. currently setting both autoplay and mute will give muted="true" autoplay="1").

everything appears to be working for me right now, except for autoplay; does it work for you?

I can't get autoplay to work at all in Chrome. This post implies that having both mute and autoplay active should enable autoplay, but this wasn't my experience (their other mentioned situations that allow autoplay didn't work for me either; and the post is old, but it was recently updated). Oddly enough, Firefox did behave that way; I could leave autoplay on, then toggle its effectiveness with the mute setting. It seems like browsers are all a bit inconsistent; since we've been able to determine each setting works as expected in at least one browser, we probably shouldn't hold up the PR on it 🙃

@mmtr I'm still having problems with the editor preview though; I can't interact with it directly at all ¯_(ツ)_/¯

[kwight]

@mmtr I'm still having problems with the editor preview though; I can't interact with it directly at all ¯_(ツ)_/¯

Oh! Sorry, just saw your comment in D30868-code:

This is expected because we deliberately disable the video player so users don't interact accidentally with it when they want to focus the block (#). Gutenberg also does the same with the default core/video block (#).

You can interact with the player when adding the video via URL because Gutenberg doesn't render the Disabled component on core/embed blocks (core/video blocks are transformed to core/embed when using URLs).

[mmtr]

This works better now, everything appears to be working for me right now, except for autoplay; does it work for you?

It's not working for me in Chrome but it is in Firefox as long as the video is muted, as @kwight mentioned. Not sure why is not autoplayed in Chrome, since the video element has both the autoplay and muted attributes required by the Chrome's autoplay policies:

Definitely not an issue on this PR and something we could investigate in the JavaScript API for VideoPress: https://github.com/Automattic/videopress/issues/38