[Gutenberg] Update VideoPress upload processor (support self-closing block tag) #18287

fluiddot · 2023-04-19T16:22:17Z

Fixes wordpress-mobile/gutenberg-mobile#5678.

This PR updates the VideoPress upload processor following a recent change in the block format. Additionally, the upload processor logic has been updated to support the self-closing block tag (e.g. ), as the VideoPress block uses this format. Note that most of the changes are reverting the modifications introduced in #18187 (1419183).

To test

Testing instructions copied from #18187.

Add a VideoPress block to an existing or new post.
Select the choose from device option and upload a video.
While there’s an ongoing upload, close the post with publishing changes.
Verify that you see the upload progress in the post summary.
Re-open the post and confirm the upload has completed.
Preview the post and observe that the video can be played.

Default markup examples

Example default markup before upload:

<!-- wp:videopress/video {"id":"1234" /-->

Example default markup after upload:

<!-- wp:videopress/video {"id":"1234","guid":"AbCdE" /-->

Markup with attributes examples

Example markup before upload:

<!-- wp:videopress/video {"autoplay":true,"controls":false,"description":"","id":${localMediaId},"loop":true,"muted":true,"playsinline":true,"poster":"https://test.files.wordpress.com/2022/02/265-5000x5000-1.jpeg","preload":"none","seekbarColor":"#abb8c3","seekbarLoadingColor":"#cf2e2e","seekbarPlayedColor":"#9b51e0","title":"Demo title","useAverageColor":false} /-->

Example markup after upload:

<!-- wp:videopress/video {"autoplay":true,"controls":false,"description":"","id":${remoteMediaId},"loop":true,"muted":true,"playsinline":true,"poster":"https://test.files.wordpress.com/2022/02/265-5000x5000-1.jpeg","preload":"none","seekbarColor":"#abb8c3","seekbarLoadingColor":"#cf2e2e","seekbarPlayedColor":"#9b51e0","title":"Demo title","useAverageColor":false,"guid":"${videoPressGuid}"} /-->

Regression Notes

Potential unintended areas of impact

This change could impact other upload processors due to the change related to supporting the self-closing tag.

What I did to test those areas of impact (or what existing automated tests I relied on)

Manually tested the VideoPress upload processor and checked the unit tests of other upload processors.

What automated tests I added (or what prevented me from doing so)

I added a test in 0d1ce8e and updated other unit tests.

PR submission checklist:

I have completed the Regression Notes.
I have considered adding accessibility improvements for my changes.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

UI Changes testing checklist:

Portrait and landscape orientations.
Light and dark modes.
Fonts: Larger, smaller and bold text.
High contrast.
Talkback.
Languages with large words or with letters/accents not frequently used in English.
Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
Large and small screen sizes. (Tablet and smaller phones)
Multi-tasking: Split screen and Pop-up view. (Android 10 or higher)

wpmobilebot · 2023-04-19T16:39:08Z

📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.

	App Name	WordPress
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr18287-47a25ca
	Commit	`47a25ca`
	Direct Download	`wordpress-prototype-build-pr18287-47a25ca.apk`

Note: Google Login is not supported on these builds.

wpmobilebot · 2023-04-19T16:40:23Z

📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.

	App Name	Jetpack
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr18287-47a25ca
	Commit	`47a25ca`
	Direct Download	`jetpack-prototype-build-pr18287-47a25ca.apk`

Note: Google Login is not supported on these builds.

fluiddot · 2023-04-19T17:05:51Z

...main/java/org/wordpress/android/ui/posts/mediauploadcompletionprocessors/BlockProcessor.java

+            mBlockContentDocument = isSelfClosingTag ? null : parseHTML(captures.group(3));
+            mClosingComment = isSelfClosingTag ? null : captures.group(4);


Blocks with the self-closing tag won't have content so these properties are empty.

fluiddot · 2023-04-19T17:09:56Z

...main/java/org/wordpress/android/ui/posts/mediauploadcompletionprocessors/BlockProcessor.java

+    String processBlock(String block) {
+        return processBlock(block, false);
+    }


This function is also used in unit tests. So, in order to avoid adding the isSelfClosingTag argument to all of them, I overloaded the function for convenience.

fluiddot · 2023-04-19T17:11:59Z

...rdpress/android/ui/posts/mediauploadcompletionprocessors/MediaUploadCompletionProcessor.java

+            String blockTagSuffix = headerMatcher.group(2);
+            Boolean isSelfClosingTag = blockTagSuffix.equals("/-->");
+            if (isSelfClosingTag) {
+                positionBlockEnd = headerMatcher.end();


When the block uses the self-closing tag, we don't need to loop over the content to find the end position.

fluiddot · 2023-04-19T17:12:32Z

...android/ui/posts/mediauploadcompletionprocessors/MediaUploadCompletionProcessorPatterns.java

@@ -11,7 +11,7 @@ public class MediaUploadCompletionProcessorPatterns {
    public static final Pattern PATTERN_BLOCK_HEADER = Pattern.compile(new StringBuilder()
            .append(PATTERN_BLOCK_PREFIX)
            .append(MediaBlockType.getMatchingGroup())
-            .append(").*? -->\n?")
+            .append(").*? (/?-->)\n?")


This new capture group is used to determine if the block uses the self-closing tag.

fluiddot · 2023-04-19T17:15:51Z

...a/org/wordpress/android/ui/posts/mediauploadcompletionprocessors/VideoPressBlockProcessor.kt

+                if (src?.startsWith("file:") == true) {
+                    remove(SRC_ATTRIBUTE)
+                }


The src is removed when pointing to a local file, similar to how we do in the block's logic.

fluiddot · 2023-04-19T17:16:37Z

...press/android/ui/posts/mediauploadcompletionprocessors/MediaUploadCompletionProcessorTest.kt

@@ -37,6 +37,14 @@ class MediaUploadCompletionProcessorTest {
        Assertions.assertThat(blocks).isEqualTo(TestContent.newPostVideo)
    }

+    @Test
+    fun `processPost splices id and guid for a VideoPress block`() {


This test was added because the VideoPress block wasn't being tested against the processContent function.

Remove unused import Remove unused import Update test: processBlock leaves VideoPress block unchanged

SiobhyB

I confirmed uploads works as expected after exiting a post with the changes from this PR. Thank you @fluiddot for the updates! 🙌 🚀

Update Media upload processor to support self-closing block tags

f9a686c

fluiddot added the gutenberg-mobile label Apr 19, 2023

fluiddot added this to the 22.3 milestone Apr 19, 2023

fluiddot self-assigned this Apr 19, 2023

fluiddot commented Apr 19, 2023

View reviewed changes

fluiddot marked this pull request as ready for review April 19, 2023 17:19

fluiddot requested review from jhnstn and SiobhyB April 19, 2023 17:20

fluiddot force-pushed the gutenberg/update/videopress-upload-processor branch from a07475c to 63f6297 Compare April 20, 2023 09:01

fluiddot added 3 commits April 20, 2023 11:06

Remove processing HTML in VideoPress upload processor

71c7ef0

Remove unused import Remove unused import Update test: processBlock leaves VideoPress block unchanged

Add VideoPress upload test case

f62ed4b

Remove src attribute in VideoPress upload processor

47a25ca

fluiddot force-pushed the gutenberg/update/videopress-upload-processor branch from 63f6297 to 47a25ca Compare April 20, 2023 09:06

SiobhyB approved these changes Apr 21, 2023

View reviewed changes

fluiddot merged commit bd46b53 into trunk Apr 24, 2023

fluiddot deleted the gutenberg/update/videopress-upload-processor branch April 24, 2023 07:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Gutenberg] Update VideoPress upload processor (support self-closing block tag) #18287

[Gutenberg] Update VideoPress upload processor (support self-closing block tag) #18287

fluiddot commented Apr 19, 2023 •

edited

Loading

wpmobilebot commented Apr 19, 2023 •

edited

Loading

wpmobilebot commented Apr 19, 2023 •

edited

Loading

fluiddot Apr 19, 2023

fluiddot Apr 19, 2023

fluiddot Apr 19, 2023

fluiddot Apr 19, 2023

fluiddot Apr 19, 2023

fluiddot Apr 19, 2023

SiobhyB left a comment

		mBlockContentDocument = isSelfClosingTag ? null : parseHTML(captures.group(3));
		mClosingComment = isSelfClosingTag ? null : captures.group(4);

[Gutenberg] Update VideoPress upload processor (support self-closing block tag) #18287

[Gutenberg] Update VideoPress upload processor (support self-closing block tag) #18287

Conversation

fluiddot commented Apr 19, 2023 • edited Loading

To test

Default markup examples

Markup with attributes examples

Regression Notes

wpmobilebot commented Apr 19, 2023 • edited Loading

wpmobilebot commented Apr 19, 2023 • edited Loading

fluiddot Apr 19, 2023

Choose a reason for hiding this comment

fluiddot Apr 19, 2023

Choose a reason for hiding this comment

fluiddot Apr 19, 2023

Choose a reason for hiding this comment

fluiddot Apr 19, 2023

Choose a reason for hiding this comment

fluiddot Apr 19, 2023

Choose a reason for hiding this comment

fluiddot Apr 19, 2023

Choose a reason for hiding this comment

SiobhyB left a comment

Choose a reason for hiding this comment

fluiddot commented Apr 19, 2023 •

edited

Loading

wpmobilebot commented Apr 19, 2023 •

edited

Loading

wpmobilebot commented Apr 19, 2023 •

edited

Loading