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

fluiddot · 2024-03-25T15:51:30Z

Fixes wordpress-mobile/gutenberg-mobile#6696.

Note

Uses the changes from #22886.

To test

Delay when saving a post

Create a post.
Add a Gallery block and insert multiple images.
Wait until all uploads finish.
Close the editor and save the post.
Observe the editor closes quickly with no significant delay.
Re-open the post.
Add more images.
Repeat steps 3, 4.
Observe the editor closes quickly with no significant delay.

Block processors

Follow the testing instructions to cover the functionality related to the processors for each block:

Gallery block:
- Close/Re-open post with an ongoing image upload
- Close post with an ongoing image upload
Image Block:
- Close/Re-open post with an ongoing image upload
- Close post with an ongoing image upload

Regression Notes

Potential unintended areas of impact
The block processors are only used when saving a post. No other area should be impacted.
What I did to test those areas of impact (or what existing automated tests I relied on)
Manually tested saving a post with different blocks. Additionally, each processor has its own unit test that has been verified.
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-25T16:00:08Z

📲 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	pr22896-7694cb2
	Version	24.9
	Bundle ID	org.wordpress.alpha
	Commit	`7694cb2`
	App Center Build	WPiOS - One-Offs #9957

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

wpmobilebot · 2024-03-25T16:15:42Z

📲 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	pr22896-7694cb2
	Version	24.9
	Bundle ID	com.jetpack.alpha
	Commit	`7694cb2`
	App Center Build	jetpack-installable-builds #9007

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

kean · 2024-03-25T20:18:30Z

WordPress/Classes/Services/PostCoordinator.swift

@@ -188,26 +188,30 @@ class PostCoordinator: NSObject {
            }

            // Ensure that all synced media references are up to date
-            post.media.forEach { media in


Please be cautious about making changes to PostCoordinator because the large potion of it is or will be rewritten. prepareToSave is unchanged, so it's a safe change.

Thanks @kean for the call out 🙇 ! This PR mainly focuses on the logic related to processing blocks before saving a post, which requires touching up some bits of the PostCoordinator. I'm planning to modify only necessary here, so hopefully there won't be much conflict with ongoing changes. In any case, I'll be happy to contribute to the new version by applying the same updates.

…e-blocks-processor

fluiddot · 2024-03-26T10:20:23Z

WordPress/Classes/Services/PostCoordinator.swift

+        var gutenbergBlockProcessors: [GutenbergProcessor] = []
+        var gutenbergProcessors: [Processor] = []


The idea is to update gradually the processors to adopt the new GutenbergContentParser. In the meantime, we'll have two different processors.

fluiddot · 2024-03-26T10:21:58Z

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

-import Aztec
+import SwiftSoup


Aztec was imported in this file to use HTMLProcessor. Now the HTML processing is performed by using SwiftSoup.

fluiddot · 2024-03-26T10:56:49Z

WordPress/WordPressTest/Gutenberg/GutenbergGalleryUploadProcessorTests.swift

+                    <br />Asf fasd fas
+                    <br />A sdfasdf sadf
+                    <br /> Asdf</figcaption>


SwiftSoup always closes tags, so tags like br or hr will be converted.

…ove-file-block-processor

irfano · 2024-04-01T13:30:35Z

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

momo-ozawa · 2024-04-15T12:44:34Z

👋 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!

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

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:23:00Z

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.

twstokes · 2024-05-14T20:38:05Z

👋 Hey @fluiddot, I'd like to pull in trunk on this PR as some time has passed. I believe this patch resolves the conflict on PostCoordinator.swift but I'd like for you to verify if possible. Thanks!

diff --cc WordPress/Classes/Services/PostCoordinator.swift
index 7098c95cae,2ea04cc472..0000000000
--- a/WordPress/Classes/Services/PostCoordinator.swift
+++ b/WordPress/Classes/Services/PostCoordinator.swift
@@@ -203,15 -741,7 +736,16 @@@ class PostCoordinator: NSObject 
          completion(.success(post))
      }
  
 +    func updateMediaBlocksBeforeSave(in post: AbstractPost, with media: Set<Media>) {
 +        guard let postContent = post.content else {
 +            return
 +        }
 +        let contentParser = GutenbergContentParser(for: postContent)
 +        media.forEach { self.updateReferences(to: $0, in: contentParser.blocks, post: post) }
 +        post.content = contentParser.html()
 +    }
 +
+     // - warning: deprecated (kahu-offline-mode)
      func cancelAnyPendingSaveOf(post: AbstractPost) {
          removeObserver(for: post)
      }
@@@ -347,12 -879,18 +883,19 @@@
              switch state {
              case .ended:
                  let successHandler = {
 -                    self.updateReferences(to: media, in: post)
 +                    self.updateMediaBlocksBeforeSave(in: post, with: [media])
 +
-                     // Let's check if media uploading is still going, if all finished with success then we can upload the post
-                     if !self.mediaCoordinator.isUploadingMedia(for: post) && !post.hasFailedMedia {
-                         self.removeObserver(for: post)
-                         completion(.success(post))
+                     if self.isSyncPublishingEnabled {
+                         if post.media.allSatisfy({ $0.remoteStatus == .sync }) {
+                             self.removeObserver(for: post)
+                             completion(.success(post))
+                         }
+                     } else {
+                         // Let's check if media uploading is still going, if all finished with success then we can upload the post
+                         if !self.mediaCoordinator.isUploadingMedia(for: post) && !post.hasFailedMedia {
+                             self.removeObserver(for: post)
+                             completion(.success(post))
+                         }
                      }
                  }
                  switch media.mediaType {

# Conflicts: # WordPress/Classes/Services/PostCoordinator.swift

fluiddot · 2024-05-16T16:00:01Z

👋 Hey @fluiddot, I'd like to pull in trunk on this PR as some time has passed. I believe this patch resolves the conflict on PostCoordinator.swift but I'd like for you to verify if possible. Thanks!

Yes @twstokes, your patch contains the same conflict resolution I have in mind. I've solved it in ccffe5b.

…-blocks-processor

…essor' into update/improve-file-block-processor

twstokes · 2024-05-16T18:34:42Z

It looks like a unit test may be failing because the output changed. I'm only spotting a whitespace difference.

@fluiddot do you think we're good to just update the expected output?

fluiddot · 2024-05-20T09:12:36Z

It looks like a unit test may be failing because the output changed. I'm only spotting a whitespace difference.

@fluiddot do you think we're good to just update the expected output?

@twstokes Yep, the difference is the extra whitespace at the end of the figure tag. The whitespace is only necessary to conform the xhtml specs for self-closing tags, which is not the case here as we are processing HTML. However, as commented in the parser's code (SwiftSoup), seems the same formatting is used to for compatibility. Hence, this extra whitespace is expected in the output. I've pushed a fix in 3a22f64.

…ove-file-block-processor

twstokes

LGTM @fluiddot! Feel free to merge - I only shared some Swift styling thoughts. 🚀

twstokes · 2024-05-21T01:09:44Z

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

-        }
+            _ = try? imgTag.attr("src", self.remoteURLString)
+            _ = try? imgTag.attr("class", newImgClassAttributes)
+            _ = try? imgTag.attr(ImageKeys.dataID, "\(self.serverMediaID)")


Optional nit and more of a personal pref, but it may be more common to convert the Int -> String via String(self.serverMediaID) if you're not interpolating anything else.

I applied this suggestion in 7694cb2.

twstokes · 2024-05-21T01:10:26Z

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

-        return html
-    })
+            let dataLinkAttribute = try? imgTag.attr(ImageKeys.dataLink)
+            if dataLinkAttribute != nil {


Same comment about adding an if let or guarding to remove the optional.

I applied this suggestion in d60c1d1.

twstokes · 2024-05-21T01:15:46Z

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

-        let imgUploadID = Int32(imageIDString)
+    func processImgTags(_ block: GutenbergParsedBlock) {
+        let imgTags = try? block.elements.select("img")
+        imgTags?.forEach { img in


Same comment about adding an if let or guarding to remove the optional.

I applied this suggestion in d60c1d1.

twstokes · 2024-05-21T01:16:08Z

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

-            return nil
-        }
+    func processLinkPostMediaUpload(_ block: GutenbergParsedBlock) {
+        let aTags = try? block.elements.select(LinkKeys.name)


Same comment about adding an if let or guarding to remove the optional.

I applied this suggestion in d60c1d1.

twstokes · 2024-05-21T01:16:44Z

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

-        }
+    func processImgPostMediaUpload(_ element: Element) {
+        let imgTags = try? element.select(ImageKeys.name)
+        imgTags?.forEach {imgTag in


Optional nit: Could be in an if let, or could guard and return early to remove the optional.

I applied this suggestion in d60c1d1.

fluiddot · 2024-05-21T15:00:40Z

LGTM @fluiddot! Feel free to merge - I only shared some Swift styling thoughts. 🚀

Thanks @twstokes for reviewing and approving the PR 🙇 ! I've applied the suggestions you shared and enabled auto-merge.

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

fluiddot added this to the 24.6 milestone Mar 25, 2024

fluiddot added 3 commits March 25, 2024 17:03

Run new Gutenberg processors

14d9ddf

Use content parser in Image block processor

231f595

Use content parser in Gallery block processor

af3c5ad

fluiddot force-pushed the update/improve-image-blocks-processor branch from 6613f88 to af3c5ad Compare March 25, 2024 16:06

Use content parser in File block processor

8ba73eb

fluiddot mentioned this pull request Mar 25, 2024

Improve performance of File block processor #22897

Merged

14 tasks

fluiddot mentioned this pull request Mar 25, 2024

Improve performance of Cover block processor #22898

Draft

14 tasks

kean reviewed Mar 25, 2024

View reviewed changes

fluiddot marked this pull request as ready for review March 26, 2024 09:58

fluiddot added 3 commits March 26, 2024 12:05

Update Gallery block processor

e3e8eb9

Update release notes

a22f0a5

Merge branch 'add/gutenberg-content-parsing' into update/improve-imag…

f73df72

…e-blocks-processor

fluiddot commented Mar 26, 2024

View reviewed changes

fluiddot added 3 commits March 26, 2024 12:15

Update File block processor

cbbd09d

Update release notes

32f122b

Merge branch 'update/improve-image-blocks-processor' into update/impr…

f9eadad

…ove-file-block-processor

fluiddot linked an issue Mar 26, 2024 that may be closed by this pull request

Long delay when saving a post wordpress-mobile/gutenberg-mobile#6696

Closed

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

dvdchr removed this from the 24.8 milestone Apr 29, 2024

dvdchr added this to the 24.9 milestone Apr 29, 2024

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

Base automatically changed from add/gutenberg-content-parsing to trunk May 14, 2024 19:50

Merge branch 'trunk' into update/improve-image-blocks-processor

ccffe5b

# Conflicts: # WordPress/Classes/Services/PostCoordinator.swift

twstokes added 4 commits May 16, 2024 14:11

Merge remote-tracking branch 'origin/trunk' into update/improve-image…

cd1b26e

…-blocks-processor

Update release notes.

d3e4077

Merge remote-tracking branch 'origin/update/improve-image-blocks-proc…

527d98d

…essor' into update/improve-file-block-processor

Update release notes.

5612643

Update expected content in testSyncNewDraftWithImageBlock unit test

3a22f64

fluiddot and others added 2 commits May 20, 2024 11:13

Merge branch 'update/improve-image-blocks-processor' into update/impr…

a9c30fb

…ove-file-block-processor

Improve performance of File block processor (#22897)

5733b49

twstokes approved these changes May 21, 2024

View reviewed changes

fluiddot added 2 commits May 21, 2024 16:50

Use guard/if let statements to avoid using optionals

d60c1d1

Parse serverMediaID to string instead of using interpolation

7694cb2

fluiddot enabled auto-merge May 21, 2024 14:53

fluiddot merged commit f46c4d9 into trunk May 21, 2024
23 of 24 checks passed

fluiddot deleted the update/improve-image-blocks-processor branch May 21, 2024 15:13

fluiddot mentioned this pull request May 21, 2024

Media: Editor freezes when saving a post with several images WordPress/gutenberg#55386

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

fluiddot commented Mar 25, 2024 •

edited

wpmobilebot commented Mar 25, 2024 •

edited

wpmobilebot commented Mar 25, 2024 •

edited

kean Mar 25, 2024

fluiddot Mar 26, 2024

fluiddot Mar 26, 2024

fluiddot Mar 26, 2024

fluiddot Mar 26, 2024

irfano commented Apr 1, 2024

momo-ozawa commented Apr 15, 2024 •

edited

dvdchr commented Apr 29, 2024

salimbraksa commented May 13, 2024

twstokes commented May 14, 2024 •

edited

fluiddot commented May 16, 2024

twstokes commented May 16, 2024

fluiddot commented May 20, 2024

twstokes left a comment

twstokes May 21, 2024

fluiddot May 21, 2024

twstokes May 21, 2024

fluiddot May 21, 2024

twstokes May 21, 2024

fluiddot May 21, 2024

twstokes May 21, 2024

fluiddot May 21, 2024

twstokes May 21, 2024

fluiddot May 21, 2024

fluiddot commented May 21, 2024 •

edited

		var gutenbergBlockProcessors: [GutenbergProcessor] = []
		var gutenbergProcessors: [Processor] = []

		import Aztec
		import SwiftSoup

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

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

Conversation

fluiddot commented Mar 25, 2024 • edited

To test

Delay when saving a post

Block processors

Regression Notes

wpmobilebot commented Mar 25, 2024 • edited

wpmobilebot commented Mar 25, 2024 • edited

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

Choose a reason for hiding this comment

irfano commented Apr 1, 2024

momo-ozawa commented Apr 15, 2024 • edited

dvdchr commented Apr 29, 2024

salimbraksa commented May 13, 2024

twstokes commented May 14, 2024 • edited

fluiddot commented May 16, 2024

twstokes commented May 16, 2024

fluiddot commented May 20, 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

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

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 commented May 21, 2024 • edited

fluiddot commented Mar 25, 2024 •

edited

wpmobilebot commented Mar 25, 2024 •

edited

wpmobilebot commented Mar 25, 2024 •

edited

momo-ozawa commented Apr 15, 2024 •

edited

twstokes commented May 14, 2024 •

edited

fluiddot commented May 21, 2024 •

edited