Add Gutenberg content parser to use in block processors #22886

fluiddot · 2024-03-21T18:27:10Z

Related to wordpress-mobile/gutenberg-mobile#6696.

To test:

Note

This functionality will be tested on separate PRs where some block processors are using it.

Observe that all CI jobs succeed.

Regression Notes

Potential unintended areas of impact
N/A
What I did to test those areas of impact (or what existing automated tests I relied on)
N/A
What automated tests I added (or what prevented me from doing so)
N/A

PR submission checklist:

I have completed the Regression Notes.
I have considered adding unit tests for my changes.
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.

Testing checklist:

wpmobilebot · 2024-03-21T18:35:57Z

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

	App Name	WordPress Alpha
	Configuration	Release-Alpha
	Build Number	pr22886-3a0894c
	Version	24.9
	Bundle ID	org.wordpress.alpha
	Commit	`3a0894c`
	App Center Build	WPiOS - One-Offs #9898

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

wpmobilebot · 2024-03-21T18:36:21Z

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

	App Name	Jetpack Alpha
	Configuration	Release-Alpha
	Build Number	pr22886-3a0894c
	Version	24.9
	Bundle ID	com.jetpack.alpha
	Commit	`3a0894c`
	App Center Build	jetpack-installable-builds #8947

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

fluiddot · 2024-03-25T11:36:17Z

WordPress/Classes/ViewRelated/Gutenberg/Processors/GutenbergContentParser.swift

+    public var comment: SwiftSoup.Comment
+    public var elements: Elements
+    public var blocks: [GutenbergParsedBlock]
+    public weak var parentBlock: GutenbergParsedBlock?


parentBlock is treated as a weak reference because the parent block also references this block via its blocks property. This will facilitate the automated deallocation.

fluiddot · 2024-03-26T09:29:57Z

WordPress.xcworkspace/xcshareddata/swiftpm/Package.resolved

The library SwiftSoup is added via the Swift Package Manager. But we could also include it as a Pod.

fluiddot · 2024-03-26T09:30:16Z

WordPress/Classes/ViewRelated/Gutenberg/Processors/GutenbergContentParser.swift

+        }
+    }
+
+    private var comment: SwiftSoup.Comment


This type includes SwiftSoup to avoid conflicts with the CoreData model Comment.

irfano · 2024-04-01T13:31:26Z

I'm moving this to the next milestone since this is not a blocker, and the code freeze will be completed today.

twstokes · 2024-04-09T13:45:17Z

👋 @fluiddot - No rush on this, but I wanted to ask:

Did you evaluate what it'd take to put the processor(s) in the Mobile Gutenberg package versus the iOS app codebase? Is there something that makes the content parsers unique to the WordPress app, versus the Gutenberg project? Thanks!

fluiddot · 2024-04-10T08:48:01Z

Did you evaluate what it'd take to put the processor(s) in the Mobile Gutenberg package versus the iOS app codebase?

Nope, I haven't explored what would be needed for moving these files to the Gutenberg Mobile package. There are several parts of the editor implemented in the client like this one that I agree seem to have a better fit in the Gutenberg Mobile. Ideally, the Gutenberg Mobile package should provide all the functionality to use the editor in any app, but the reality is that Gutenberg Mobile is quite coupled to the WP/JP app code.

Is there something that makes the content parsers unique to the WordPress app, versus the Gutenberg project?

Part of the logic is related to Gutenberg, as the structure of blocks is defined by the editor. However, there's a bit of overlap in the processing because the need of the processors is due to updating media attributes that are defined by the client. As an example, an Image block needs to be updated with the server media ID, and the media structure is defined in the WP/JP apps.

In the future, it would be great if we identify which parts should be provided by Gutenberg Mobile and which ones should be delegated to the client. However, currently, this separation is a bit blurry.

twstokes · 2024-04-10T21:47:14Z

Makes sense @fluiddot, and certainly something we could defer to the future. 👍

momo-ozawa · 2024-04-15T12:49:14Z

👋 Hey @fluiddot, I'm bumping this PR to 24.8 since it's code freeze day. If this PR needs to target 24.7, please target the release branch once it's been cut. Thanks!

kean · 2024-04-25T14:48:23Z

Did you evaluate what it'd take to put the processor(s) in the Mobile Gutenberg package versus the iOS app codebase?

There is a good reason for consolidating this code in Gutenberg. The app and the editor produce different results when handling uploads.

Here's an example of PostCoordinator using its logic:

<!-- wp:image {\"id\":1431,\"sizeSlug\":\"large\"} -->
  <figure class=\"wp-block-image size-large\">
    <img src=\"https://alextest41234.files.wordpress.com/2024/04/img_0005-2-1.jpg?w=1024\"  class=\"wp-image-1431\"/>
  </figure>
<!-- /wp:image -->

And here's Gutenberg:

<!-- wp:image {\"id\":1431,\"sizeSlug\":\"large\"} -->
  <figure class=\"wp-block-image size-large\">
    <img src=\"https://alextest41234.files.wordpress.com/2024/04/img_0005-2-1.jpg?w=1024\" alt=\"\"
class=\"wp-image-1431\"/>
  </figure>
<!-- /wp:image -->

Notice an alt tag. It leads to situation where the app thinks there are changes to the post when the are none. Discussion here: p1714049179672299/1714047410.810349-slack-C06GRKUGDNX.

re: performance

To address the performance issues, I recommend moving this code to the background. I can make this change in the scope of the current project since I'm working in this area.

fluiddot · 2024-04-25T16:09:17Z

There is a good reason for consolidating this code in Gutenberg. The app and the editor produce different results when handling uploads.

Here's an example of PostCoordinator using its logic:

  <figure class=\"wp-block-image size-large\">
    <img src=\"https://alextest41234.files.wordpress.com/2024/04/img_0005-2-1.jpg?w=1024\"  class=\"wp-image-1431\"/>
  </figure>

And here's Gutenberg:

  <figure class=\"wp-block-image size-large\">
    <img src=\"https://alextest41234.files.wordpress.com/2024/04/img_0005-2-1.jpg?w=1024\" alt=\"\"
class=\"wp-image-1431\"/>
  </figure>

Notice an alt tag. It leads to situation where the app thinks there are changes to the post when the are none. Discussion here: p1714049179672299/1714047410.810349-slack-C06GRKUGDNX.

I agree that consolidating the block processors within the Gutenberg Mobile project could be beneficial. However, I don't believe it will directly address the core issue. The main challenge stems from the fact that the logic for generating the block's markup resides on the JavaScript side (Gutenberg). Currently, the block processors are detached from Gutenberg, requiring them to stay updated with the logic of each block. Given that this is a manual process, discrepancies, as you highlighted @kean, can arise. While finding a method to synchronize the block processors with Gutenberg's logic would be ideal, I anticipate it would introduce considerable complexity.

To address the performance issues, I recommend moving this code to the background. I can make this change in the scope of the current project since I'm working in this area.

Please correct me if I'm mistaken, but even if we move the code to the background, the post-saving process will still be time-consuming due to the required processing. Is that correct?

kean · 2024-04-25T16:11:29Z

Please correct me if I'm mistaken, but even if we move the code to the background, the post-saving process will still be time-consuming due to the required processing. Is that correct?

"30 media items: 5.250 seconds" – oh, I didn't know we we talking seconds 😨
I assumed it was just micro main thread hangs.

kean · 2024-04-25T16:13:15Z

While finding a method to synchronize the block processors with Gutenberg's logic would be ideal, I anticipate it would introduce considerable complexity.

There is one more variable: the server. I noticed that it modifies the content before saving. For example, it might end up removing redundant spaces between tag attributes. So, it's really hard to get a canonical representation of a post.

generating the block's markup resides on the JavaScript side (Gutenberg)

An app can call JavaScript directly with little overhead.

fluiddot · 2024-04-25T16:41:16Z

There is one more variable: the server. I noticed that it modifies the content before saving. For example, it might end up removing redundant spaces between tag attributes. So, it's really hard to get a canonical representation of a post.

Good point. That's definitely another factor to take into account. Usually, when changes are made to a block (either in the frontend or the backend) they are updated in the same Gutenberg version. In most cases, those changes are also made in the mobile native version (i.e. Gutenberg Mobile). However, the web version doesn't have the same concept of block processors that are run outside the context of the editor, like the case of the app. Hence, in case we consolidate them in the Gutenberg repo, it will take time for Gutenberg contributors to be aware of the need to update them.

An app can call JavaScript directly with little overhead.

True. We could consider a potential workaround where the block processor resides on the JavaScript side and imports the necessary code from Gutenberg to process the block. It's worth noting that, as far as I know, such an approach doesn't currently exist, so it would require building it from scratch.

kean · 2024-04-25T18:38:49Z

@fluiddot, I extracted the processors and moved the work to the background thread #23093. This code will only run for a small fraction of the users eligible to test the "Offline Mode" changes.

kean · 2024-04-26T18:28:33Z

WordPress.xcworkspace/xcshareddata/swiftpm/Package.resolved

@@ -64,6 +64,15 @@
          "version": "0.2.3"
        }
      },
+      {
+        "package": "SwiftSoup",
+        "repositoryURL": "https://github.com/scinfu/SwiftSoup.git",


The size of the dependency is a bit concerning:

------------------------------------------------------------------------------- Language files blank comment code ------------------------------------------------------------------------------- Swift 58 1709 3707 10315

It should not affect incremental builds, I think it's something to be considered.

I would suggest considering wrapping https://developer.wordpress.org/block-editor/reference-guides/packages/packages-blocks/ and other related WordPress JS APIs in Objective-C and invoking them using JavaScriptCore. This way, we'll be able to reuse the existing parser. You can run Javascript in background on iOS to ensure that processing doesn't take a lot of time.

A native package that would wrap the existing JS packages could be useful for more scenarios than this. For example, the app could use it for addressing the "dangling" media uploads issue.

The size of the dependency is a bit concerning:
[...]
It should not affect incremental builds, I think it's something to be considered.

I haven't checked how much this library increases the binary. If it's significant we might consider other libraries.

I would suggest considering wrapping https://developer.wordpress.org/block-editor/reference-guides/packages/packages-blocks/ and other related WordPress JS APIs in Objective-C and invoking them using JavaScriptCore. This way, we'll be able to reuse the existing parser. You can run Javascript in background on iOS to ensure that processing doesn't take a lot of time.

A native package that would wrap the existing JS packages could be useful for more scenarios than this. For example, the app could use it for addressing the "dangling" media uploads issue.

This approach is intriguing and would certainly ensure consistent results when processing a block compared to Gutenberg. As a long-term solution, I'd advocate exploring this approach. However, I believe it would entail significant complexity in implementation, particularly regarding how to bundle the necessary code and potential tweaks to the Gutenberg code. @kean not sure if with your comment you're proposing following this instead of the refactor implemented in this PR. WDYT?

your comment you're proposing following this instead of the refactor implemented in this PR. WDYT?

It's more long-term. I worked with JS-based packages before, so I know it should be feasible, but I don't know how much effort it would take.

particularly regarding how to bundle the necessary code

These packages are already part of the Gutenberg-mobile project, aren't they?

These packages are already part of the Gutenberg-mobile project, aren't they?

Yes, but we bundled them with Metro and files are compiled with Hermes (the setup for React Native). I presume the output of this setup won't be supported natively. We'd need to generate a new bundle compatible with JavaScriptCore.

The app doesn't need to use these JS files directly. It could part of the existing Gutenberg framework.

dvdchr · 2024-04-29T08:48:50Z

Hi @fluiddot 👋 , I'm bumping this PR's milestone to 24.9 since I'm starting code freeze. Feel free to re-target this to the release branch if this is a blocker or intended for 24.8.

salimbraksa · 2024-05-13T20:17:49Z

Hey @fluiddot, I'm bumping the milestone for this PR to 25.0 as I'm starting the code freeze. If this PR needs to go into 24.9, please go ahead and re-target it to the release branch.

…t-parsing

twstokes

LGTM @fluiddot! Sorry for the delay. I'm getting the branch caught up with trunk and letting CI run. If we're all green then I'll merge. Thanks for the hard work! 🚀

fluiddot · 2024-05-16T08:08:14Z

RELEASE-NOTES.txt

@@ -48,6 +48,7 @@
 * [**] Block editor: Highlight text fixes [https://github.com/WordPress/gutenberg/pull/57650]
 * [*] [Jetpack-only] Stats: Eliminated common error causes in the Insights tab. [#22890]
 * [*] [Jetpack-only] Reader: Fix displaying stale site information [#22885]
+* [*] [internal] Incorporate a parser to handle Gutenberg blocks more efficiently for improved performance [#22886]


Seems this entry landed in the wrong section, I'll update it in another PR.

Here's the PR with the fix.

fluiddot added 3 commits March 21, 2024 18:06

Add SwiftSoap library

eafb0a8

Add Gutenberg content parser

101ab41

Add Gutenberg processor

e1ba476

fluiddot added [Type] Enhancement Gutenberg Editing and display of Gutenberg blocks. labels Mar 21, 2024

fluiddot added this to the 24.6 milestone Mar 21, 2024

fluiddot mentioned this pull request Mar 21, 2024

Improve Gutenberg block processors performance to avoid long delay when saving a post #22845

Closed

14 tasks

fluiddot added 3 commits March 25, 2024 12:17

Rename Gutenberg content parser function for generating HTML output

f0ad71d

Update Gutenberg content parser unit tests

f41873c

Update and document Gutenberg content parser

ab607ec

fluiddot commented Mar 25, 2024

View reviewed changes

fluiddot marked this pull request as ready for review March 25, 2024 12:27

fluiddot mentioned this pull request Mar 25, 2024

Improve performance of Image and Gallery block processors to avoid long delay when saving a post #22896

Merged

14 tasks

Merge branch 'trunk' into add/gutenberg-content-parsing

2c56558

fluiddot commented Mar 26, 2024

View reviewed changes

fluiddot added 2 commits March 26, 2024 11:13

Update documentation of GutenbergContentParser

15b2bc2

Update release notes

6ef5cfa

irfano modified the milestones: 24.6, 24.7 Apr 1, 2024

twstokes self-requested a review April 3, 2024 18:27

momo-ozawa modified the milestones: 24.7, 24.8 Apr 15, 2024

kean reviewed Apr 26, 2024

View reviewed changes

dvdchr modified the milestones: 24.8, 24.9 Apr 29, 2024

salimbraksa modified the milestones: 24.9, 25.0 May 13, 2024

twstokes mentioned this pull request May 14, 2024

Offline Mode: Perform media block processing in the background #23093

Closed

14 tasks

Merge remote-tracking branch 'origin/trunk' into add/gutenberg-conten…

3a0894c

…t-parsing

twstokes approved these changes May 14, 2024

View reviewed changes

twstokes merged commit 20d87aa into trunk May 14, 2024
24 checks passed

twstokes deleted the add/gutenberg-content-parsing branch May 14, 2024 19:50

fluiddot commented May 16, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Gutenberg content parser to use in block processors #22886

Add Gutenberg content parser to use in block processors #22886

fluiddot commented Mar 21, 2024 •

edited

wpmobilebot commented Mar 21, 2024 •

edited

wpmobilebot commented Mar 21, 2024 •

edited

fluiddot Mar 25, 2024

fluiddot Mar 26, 2024

fluiddot Mar 26, 2024

irfano commented Apr 1, 2024

twstokes commented Apr 9, 2024

fluiddot commented Apr 10, 2024

twstokes commented Apr 10, 2024

momo-ozawa commented Apr 15, 2024 •

edited

kean commented Apr 25, 2024 •

edited

fluiddot commented Apr 25, 2024

kean commented Apr 25, 2024 •

edited

kean commented Apr 25, 2024 •

edited

fluiddot commented Apr 25, 2024

kean commented Apr 25, 2024

kean Apr 26, 2024

kean Apr 26, 2024

fluiddot Apr 29, 2024

kean Apr 29, 2024

fluiddot Apr 29, 2024 •

edited

kean Apr 29, 2024

dvdchr commented Apr 29, 2024

salimbraksa commented May 13, 2024

twstokes left a comment

fluiddot May 16, 2024

fluiddot May 16, 2024

Add Gutenberg content parser to use in block processors #22886

Add Gutenberg content parser to use in block processors #22886

Conversation

fluiddot commented Mar 21, 2024 • edited

Regression Notes

wpmobilebot commented Mar 21, 2024 • edited

wpmobilebot commented Mar 21, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

irfano commented Apr 1, 2024

twstokes commented Apr 9, 2024

fluiddot commented Apr 10, 2024

twstokes commented Apr 10, 2024

momo-ozawa commented Apr 15, 2024 • edited

kean commented Apr 25, 2024 • edited

fluiddot commented Apr 25, 2024

kean commented Apr 25, 2024 • edited

kean commented Apr 25, 2024 • edited

fluiddot commented Apr 25, 2024

kean commented Apr 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fluiddot Apr 29, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dvdchr commented Apr 29, 2024

salimbraksa commented May 13, 2024

twstokes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fluiddot commented Mar 21, 2024 •

edited

wpmobilebot commented Mar 21, 2024 •

edited

wpmobilebot commented Mar 21, 2024 •

edited

momo-ozawa commented Apr 15, 2024 •

edited

kean commented Apr 25, 2024 •

edited

kean commented Apr 25, 2024 •

edited

kean commented Apr 25, 2024 •

edited

fluiddot Apr 29, 2024 •

edited