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

Video block: Video is not displayed when post is saved in the web version #30987

Closed
fluiddot opened this issue Apr 20, 2021 · 7 comments · Fixed by Automattic/jetpack#24548, wordpress-mobile/gutenberg-mobile#4899 or #58015
Assignees
Labels
Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) [Priority] High Used to indicate top priority items that need quick attention [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@fluiddot
Copy link
Contributor

Description

This issue was initially identified here: wordpress-mobile/WordPress-iOS#15184

When a video block is saved in the web version then the video can't be displayed in the mobile app and shows the message “Problem displaying block” within the block. Looks like there's some kind of incompatibility on the HTML produced when this block is used in the mobile app and web.

Step-by-step reproduction instructions

The steps for reproducing it are:

  1. In the app
    1. Create/edit a post
    2. Add a video block
    3. Insert a video
    4. Save the post
  2. In the web
    1. Open the post
    2. Observe that the HTML code is different when changing to code editor
    3. Save the post
  3. In the app again
    1. Open the post
    2. Observe that the video block shows an error

NOTE: This issue can be also reproduced by adding a video block in the web version and then opening the post in the mobile app.

As an example, here is the HTML produced in each case:

Native app (WPiOS)

<!-- wp:video {"id":<MEDIA_ID>} -->
<figure class="wp-block-video"><video controls src="<VIDEO_SRC>"></video></figure>
<!-- /wp:video -->

Web

When opened first time with code editor:

<!-- wp:video {"autoplay":false,"id":<MEDIA_ID>,"loop":false,"muted":false,"src":"<VIDEO_SRC>"} -->
<figure class="wp-block-video"><video controls src="<VIDEO_SRC>"></video></figure>
<!-- /wp:video -->

When opened with visual editor:

In this case the web version generates a preview, maybe this is related to why it produces a different HTML code.

<!-- wp:video {"autoplay":false,"guid":"<VIDEO_GUID>","id":<MEDIA_ID>,"loop":false,"muted":false,"src":"<VIDEO_SRC>","videoPressClassNames":"wp-block-embed is-type-video is-provider-videopress"} -->
<figure class="wp-block-video wp-block-embed is-type-video is-provider-videopress"><div class="wp-block-embed__wrapper">
https://videopress.com/v/<VIDEO_GUID>?preloadContent=metadata
</div></figure>
<!-- /wp:video -->

Expected behaviour

The video should be displayed in the mobile app.

Actual behaviour

The video is not displayed in the mobile app.

Screenshots or screen recording (optional)

WordPress information

  • WordPress version: N/A
  • Gutenberg version: v10.4.1
  • Are all plugins except Gutenberg deactivated? N/A
  • Are you using a default theme (e.g. Twenty Twenty-One)? N/A

Device information

  • Device: iPhone 11
  • Operating system: iOS 14.2
  • WordPress app version: 17.0.1.0
@fluiddot fluiddot added [Type] Bug An existing feature does not function as intended Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) labels Apr 20, 2021
@fluiddot
Copy link
Contributor Author

fluiddot commented Apr 20, 2021

On Android, it produces a block validation error when testing with a debug build (see attached screenshot).

I saw that we already have an issue, in this case related to the Cover block, that shows a similar error, maybe both issues are caused by the same problem 🤔 .

@mchowning mchowning added the [Priority] High Used to indicate top priority items that need quick attention label Apr 20, 2021
@AmandaRiu AmandaRiu self-assigned this May 13, 2021
@jd-alexander
Copy link
Contributor

jd-alexander commented Sep 14, 2021

Thanks for the investigation you did here @fluiddot so I can confirm what you have suggested. On the web, it seems the video content of the blocks is converted to an embed hence the failure.

An initial idea, I had was to utilize the replaceBlock functionality from the blockEditorStore to make the block compliant and valid ( Done here) as a fallback mechanism within the BlockInvalidWarning native component as this is done in the web variant. This worked to an extent, however, the problem I ran into is that the src is stripped from the attributes.
The PR that introduced this change is #11410 I think its approach has stripped the attributes.


Below is how the Video block looks as an Embed block, and it also shows that it's not playable in the mobile app due to the missing src.

 {"attributes": {"autoplay": false, "caption": "", "className": "wp-block-embed is-type-video is-provider-videopress", "controls": false, "id": 369, "loop": false, "muted": false, "playsInline": false, "preload": "metadata", "tracks": []}, "clientId": "3281c2c6-1fbf-4cc1-b93b-07170c78723e", "innerBlocks": [], "isValid": false, "name": "core/video", "originalContent": "<figure class=\"wp-block-video wp-block-embed is-type-video is-provider-videopress\"><div class=\"wp-block-embed__wrapper\">
https://videopress.com/v/eIfxnIud?resizeToParent=true&amp;preloadContent=metadata
</div></figure>"}


A solution to this may be to create a rollback mechanism on mobile that replaces the Embed blocks in Video and Cover blocks that are upgraded on the web.

@fluiddot
Copy link
Contributor Author

Thanks for the investigation you did here @fluiddot so I can confirm what you have suggested. On the web, it seems the video content of the blocks is converted to an embed hence the failure.

Good catch 🎊 !

An initial idea, I had was to utilize the replaceBlock functionality from the blockEditorStore to make the block compliant and valid ( Done here) as a fallback mechanism within the BlockInvalidWarning native component as this is done in the web variant. This worked to an extent, however, the problem I ran into is that the src is stripped from the attributes.
The PR that introduced this change is #11410 I think its approach has stripped the attributes.

Below is how the Video block looks as an Embed block, and it also shows that it's not playable in the mobile app due to the missing src.

 {"attributes": {"autoplay": false, "caption": "", "className": "wp-block-embed is-type-video is-provider-videopress", "controls": false, "id": 369, "loop": false, "muted": false, "playsInline": false, "preload": "metadata", "tracks": []}, "clientId": "3281c2c6-1fbf-4cc1-b93b-07170c78723e", "innerBlocks": [], "isValid": false, "name": "core/video", "originalContent": "<figure class=\"wp-block-video wp-block-embed is-type-video is-provider-videopress\"><div class=\"wp-block-embed__wrapper\">
https://videopress.com/v/eIfxnIud?resizeToParent=true&amp;preloadContent=metadata
</div></figure>"}

I'm wondering why the src is not being included 🤔. I did a quick test and I saw that the src attribute was present when switching to HTML mode in the web version of the editor, so maybe the native version of the Video block is the one that is stripping out the src when loading the post, does it make sense?

A solution to this may be to create a rollback mechanism on mobile that replaces the Embed blocks in Video and Cover blocks that are upgraded on the web.

Yeah, that's a good option although we should investigate potential side effects.

@jd-alexander
Copy link
Contributor

I'm wondering why the src is not being included 🤔. I did a quick test and I saw that the src attribute was present when switching to HTML mode in the web version of the editor, so maybe the native version of the Video block is the one that is stripping out the src when loading the post, does it make sense?

Interesting. That's a hypothesis worth checking out. 👍🏾

@SiobhyB SiobhyB assigned SiobhyB and unassigned jd-alexander May 10, 2022
@SiobhyB
Copy link
Contributor

SiobhyB commented May 19, 2022

To tie up all the pieces to this I've found in my investigations so far: I've only been able to reproduce this behaviour on WP.com sites or sites connected to the Jetpack plugin (with VideoPress enabled). Videos uploaded to self-hosted sites without any plugins that automatically change the video block, like VideoPress, displayed as expected within the app.

There's an older issue related to this here, also, that outlines some discussions and possible solutions, particularly in this comment: wordpress-mobile/gutenberg-mobile#1152

There are also some other related issues that may be resolved by addressing this, such as wordpress-mobile/gutenberg-mobile#3292.

I'm planning to look into recreating the PR in #15674 to improve the way unsupported blocks are handled.

@fluiddot
Copy link
Contributor Author

I'm re-opening the issue as I managed to reproduce a similar issue to this one when testing version 24.0. I tested previous versions (from 24.0 to 23.5) and can also be reproduced in old versions. No error is displayed within the Video block but it's rendered as empty (see attached video capture).

ios-video-block-empty.MP4

@fluiddot fluiddot reopened this Jan 18, 2024
@fluiddot fluiddot assigned fluiddot and unassigned SiobhyB Jan 18, 2024
@fluiddot
Copy link
Contributor Author

I noticed that this regression was introduced in this change #49858. After further investigations, I checked the following cases and found two scenarios for the condition:

if ( ! src ) {
return (
<View style={ { flex: 1 } }>
<MediaPlaceholder

Scenarios

Video uploaded (hosted in VideoPress)

<!-- wp:video {"id":1234} -->
<figure class="wp-block-video"><video controls src="https://videos.files.wordpress.com/<VIDEO_FILE>.mp4"></video></figure>
<!-- /wp:video -->
  • Source can be validated with src attribute.

Video from media library:

<!-- wp:video {"id":1234} -->
<figure class="wp-block-video"><video controls src="https://<SITE>.files.wordpress.com/<VIDEO_FILE>.mp4"></video></figure>
<!-- /wp:video -->
  • Source can be validated with src attribute.

Video from URL

<!-- wp:video -->
<figure class="wp-block-video"><video controls src="http://<VIDEO_FILE>.mp4"></video></figure>
<!-- /wp:video -->
  • Source can be validated with src attribute.

Video uploaded from the web (hosted in VideoPress)

<!-- wp:video {"guid":"ABCD1234","id":1234,"videoPressClassNames":"wp-block-embed is-type-video is-provider-videopress"} -->
<figure class="wp-block-video wp-block-embed is-type-video is-provider-videopress"><div class="wp-block-embed__wrapper">
https://videopress.com/<VIDEO_ID>
</div></figure>
<!-- /wp:video -->
  • Source can be validated with guid and id (these two parameters are the ones needed to generate a VideoPress video URL).

Post created in the app and saved on the web with videos (hosted in VideoPress)

<!-- wp:video {"id":1234} -->
<figure class="wp-block-video"><video controls src="https://videos.files.wordpress.com/<VIDEO_FILE>.mp4"></video></figure>
<!-- /wp:video -->

is converted to:

<!-- wp:video {"guid":"ABCD1234","id":1234,"videoPressClassNames":"wp-block-embed is-type-video is-provider-videopress"} -->
<figure class="wp-block-video wp-block-embed is-type-video is-provider-videopress"><div class="wp-block-embed__wrapper">
https://videopress.com/<VIDEO_ID>
</div></figure>
<!-- /wp:video -->

NOTE: It also transforms Video blocks that point to media URLs (e.g. https://<SITE>.files.wordpress.com/<VIDEO_FILE>.mp4).

  • Source can be validated with guid and id (these two parameters are the ones needed to generate a VideoPress video URL).

Conditions

  1. For most cases, we can simply check if attribute src is empty (current implementation).
  2. For other cases, specific to VideoPress videos, we need to check the presence of guid and if id is empty.

I'll open a PR with a temporary workaround, as the VideoPress case, should not be handled in the core block.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) [Priority] High Used to indicate top priority items that need quick attention [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
Status: Done
5 participants