WebKit Reader Comments (Groundwork) #24022

kean · 2025-01-28T20:49:57Z

This is the initial PR that enabled WebKit-based rendering for comments in the "Comments" screen.

The app already had WebCommentContentRenderer. It was used only in a subset of screens, for example, for a single comment displayed right under the article. This PR introduces a few performance and other improvements that made it possible to use in the lists.

This PR also introduces a new WordPressReader module. I moved WebCommentContentRenderer and ReaderDisplaySettings to this new module and added Xcode previews to make it easy to iterate on it.

The feature is not complete, so it's currently hidden behind a feature flag, so it should be safe to merge now and continue iterating in trunk. I'm planning more performance optimizations, better styles for some Gutenberg blocks, and better handling for URLs.

Future Plans

The main issues is that rendering in WKWebView is inherently asynchronous and depending on how fast you scroll and how slow your device is, you might end up seeing empty cells until the comment loads. I'm planning to work this around by adding prefetching to this screen. It will ensure that in the vast majority of cases, you are never going to see the empty cells. I will also add loading state (skeletons?) for empty cells.

I also haven't measured the impact on the battery life yet. It might be worth adding a simple UITextView-based implementation for plain text cells. It will also free the resources for other WKWebView instances that do need WebKit to render the content.

Testing

Enable the new "Render Comments using WebKit" feature flag
Verify that Gutenberg comments are displayed (well) and the cell heights are computed correctly

Future

Regression Notes

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

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:

dangermattic · 2025-01-28T20:51:01Z

	2 Warnings
⚠️	`Modules/Package.swift` was changed without updating its corresponding `Package.resolved`. Please resolve the Swift packages as appropriate to your project setup (e.g. in Xcode or by running `swift package resolve`).
⚠️	This PR is larger than 500 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

wpmobilebot · 2025-01-28T21:00:36Z

📲 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	pr24022-075261c
	Version	25.7
	Bundle ID	com.jetpack.alpha
	Commit	`075261c`
	App Center Build	jetpack-installable-builds #10461

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

wpmobilebot · 2025-01-28T21:02:45Z

📲 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	pr24022-075261c
	Version	25.7
	Bundle ID	org.wordpress.alpha
	Commit	`075261c`
	App Center Build	WPiOS - One-Offs #11428

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

kean · 2025-01-30T21:25:26Z

Modules/Sources/WordPressReader/Comments/Views/WebCommentContentRenderer.swift

+    }
+
+    public func render(comment: String) -> UIView {
+        guard self.comment != comment else {


It was part of the existing CommentContentRenderer protocol, and I don't want to change it too much just yet.

kean · 2025-01-30T21:26:33Z

Modules/Sources/WordPressReader/Comments/Views/WebCommentContentRenderer.swift

+    public func webView(_ webView: WKWebView, didFinish navigation: WKNavigation!) {
+        // Wait until the HTML document finished loading.
+        // This also waits for all of resources within the HTML (images, video thumbnail images) to be fully loaded.
+        webView.evaluateJavaScript("document.readyState") { complete, _ in


This was the existing code. I just moved it. Not sure if it works the way it says it does.

kean · 2025-01-30T21:26:58Z

Modules/Sources/WordPressReader/Comments/Views/WebCommentContentRenderer.swift

+        // remove empty HTML elements from the `content`, as the content often contains empty paragraph elements which adds unnecessary padding/margin.
+        // `rawContent` does not have this problem, but it's not used because `rawContent` gets rid of links (<a> tags) for mentions.
+        let comment = comment
+            .replacingOccurrences(of: Self.emptyElementRegexPattern, with: "", options: [.regularExpression])


Ideally, I want to preprocess this in the background and store in Core Data in this way.

kean · 2025-01-30T21:28:02Z

Modules/Sources/WordPressReader/Settings/ReaderDisplaySettings+WebKit.swift

+
+// MARK: - ReaderDisplaySetting (CSS)
+
+extension ReaderDisplaySettings {


There is an opportunity to reuse this with ReaderWebView. I'm going to look into it.

kean · 2025-01-30T21:28:23Z

Modules/Sources/WordPressReader/Settings/ReaderDisplaySettings.swift

 import WordPressShared

-struct ReaderDisplaySetting: Codable, Equatable {
+public struct ReaderDisplaySettings: Codable, Equatable, Hashable, Sendable {


I renamed it because I kept typing ReaderDisplaySettings.

kean · 2025-01-30T21:29:38Z

WordPress/Classes/ViewRelated/Reader/Comments/ReaderCommentsHelper.swift

+        return renderer
+    }
+
+    func getCachedContentHeight(for commentID: TaggedManagedObjectID<Comment>) -> CGFloat? {


I'll probably change the keys to use comment's content (String). I'm not sure yet because comparing long strings isn't ideal.

kean · 2025-01-30T21:30:23Z

WordPress/Classes/ViewRelated/Reader/Comments/ReaderCommentsHelper.swift

+    }
+
+    func getRenderer(for comment: Comment) -> WebCommentContentRenderer {
+        if let renderer = renderers.object(forKey: comment) {


I'm not sure it's a good idea to reuse web view across different cells this way, but it helps with cell reload. UITableView does little to nothing to ensure the same cells are used for the same comments on reload.

kean added the Reader label Jan 28, 2025

kean added this to the 25.8 milestone Jan 28, 2025

kean added 25 commits January 30, 2025 10:42

Enable web-based rendering

7748e35

Disable animations when changing height for comments

ac8beb4

Add ReaderCommentsHelper for caching height for comment cells

5f43d37

Add reuse of WKWebView instances

1f2587a

Add NSCache-based cache for comments

d65418f

Mask the initial load

b0278a5

Fix text-based rendering

258d11b

Add a note about self.tableView.alpha = 0.0;

ea2ec03

Add WordPressReader target

fc34aad

Fix retain cycle

78ec774

Move templates to the new module

9efed5f

Add a preview

8f7d054

Add preview

33870f4

Extract CommentWebView to a separate file

bdf7b8b

Add Gutenberg preview

4f25334

Add tint color for web view

342c5fe

Log URLs

f5a87c9

Disable JavaScript just in case

37ee2ca

Cache html template and stylesheet

8e83414

Cleanup stylesheet creation a bit

f59c176

Extract ReaderDisplaySettings+WebKit

f8023c1

Cache preprocessed <head>

1d8cf53

Rename ReaderDisplaySettings

81468e6

Fix compilation

b839e97

Disable FF

108380e

kean force-pushed the task/better-comments-rendering branch from 292a91d to 108380e Compare January 30, 2025 21:13

kean commented Jan 30, 2025

View reviewed changes

Fix compilation

075261c

kean force-pushed the task/better-comments-rendering branch from 6c43b39 to 075261c Compare January 30, 2025 21:38

kean changed the title ~~WebKit Reader Comments~~ WebKit Reader Comments (Ground Work) Jan 30, 2025

kean marked this pull request as ready for review January 30, 2025 21:38

kean changed the title ~~WebKit Reader Comments (Ground Work)~~ WebKit Reader Comments (Groundwork) Jan 30, 2025

kean requested a review from crazytonyli January 30, 2025 21:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WebKit Reader Comments (Groundwork) #24022

WebKit Reader Comments (Groundwork) #24022

kean commented Jan 28, 2025 •

edited

Loading

dangermattic commented Jan 28, 2025 •

edited

Loading

wpmobilebot commented Jan 28, 2025 •

edited

Loading

wpmobilebot commented Jan 28, 2025 •

edited

Loading

kean Jan 30, 2025

kean Jan 30, 2025

kean Jan 30, 2025

kean Jan 30, 2025

kean Jan 30, 2025

kean Jan 30, 2025

kean Jan 30, 2025


		// MARK: - ReaderDisplaySetting (CSS)

		extension ReaderDisplaySettings {

WebKit Reader Comments (Groundwork) #24022

Are you sure you want to change the base?

WebKit Reader Comments (Groundwork) #24022

Conversation

kean commented Jan 28, 2025 • edited Loading

Future Plans

Testing

Future

Regression Notes

dangermattic commented Jan 28, 2025 • edited Loading

wpmobilebot commented Jan 28, 2025 • edited Loading

wpmobilebot commented Jan 28, 2025 • edited Loading

kean Jan 30, 2025

Choose a reason for hiding this comment

kean Jan 30, 2025

Choose a reason for hiding this comment

kean Jan 30, 2025

Choose a reason for hiding this comment

kean Jan 30, 2025

Choose a reason for hiding this comment

kean Jan 30, 2025

Choose a reason for hiding this comment

kean Jan 30, 2025

Choose a reason for hiding this comment

kean Jan 30, 2025

Choose a reason for hiding this comment

kean commented Jan 28, 2025 •

edited

Loading

dangermattic commented Jan 28, 2025 •

edited

Loading

wpmobilebot commented Jan 28, 2025 •

edited

Loading

wpmobilebot commented Jan 28, 2025 •

edited

Loading