Integrate Gutenberg Preloading#22579
Conversation
Generated by 🚫 Danger |
7d72605 to
e221a2a
Compare
Project dependencies changeslist+ New Dependencies
org.jetbrains.kotlinx:kotlinx-serialization-json:1.7.3
org.jetbrains.kotlinx:kotlinx-serialization-json-jvm:1.7.3
! Upgraded Dependencies
org.wordpress.gutenbergkit:android:339-913043cb0ba486850c9cb90c72059b92c1900577, (changed from v0.11.1)tree++--- androidx.navigation:navigation-compose:2.9.7
+| \--- androidx.navigation:navigation-compose-android:2.9.7
+| \--- androidx.activity:activity:1.8.0 -> 1.10.1
+| \--- androidx.lifecycle:lifecycle-viewmodel-savedstate:2.6.1 -> 2.10.0
+| \--- androidx.lifecycle:lifecycle-viewmodel-savedstate-android:2.10.0
+| \--- androidx.savedstate:savedstate:1.4.0
+| \--- androidx.savedstate:savedstate-android:1.4.0
+| \--- org.jetbrains.kotlinx:kotlinx-serialization-core:1.7.3
+| \--- org.jetbrains.kotlinx:kotlinx-serialization-core-jvm:1.7.3
+| \--- org.jetbrains.kotlinx:kotlinx-serialization-bom:1.7.3
+| +--- org.jetbrains.kotlinx:kotlinx-serialization-json-jvm:1.7.3 (c)
+| \--- org.jetbrains.kotlinx:kotlinx-serialization-json:1.7.3 (c)
+--- project :libs:editor
-| \--- org.wordpress.gutenbergkit:android:v0.11.1
-| +--- org.jetbrains.kotlin:kotlin-parcelize-runtime:2.0.21 -> 2.3.20 (*)
-| +--- androidx.core:core-ktx:1.13.1 -> 1.16.0 (*)
-| +--- androidx.appcompat:appcompat:1.7.0 -> 1.7.1 (*)
-| +--- com.google.android.material:material:1.12.0 (*)
-| +--- androidx.webkit:webkit:1.11.0 -> 1.15.0 (*)
-| +--- com.google.code.gson:gson:2.8.9 -> 2.13.2
-| | \--- com.google.errorprone:error_prone_annotations:2.41.0
-| +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.10.2 (*)
-| \--- org.jetbrains.kotlin:kotlin-stdlib:2.0.21 -> 2.3.20 (*)
+| \--- org.wordpress.gutenbergkit:android:339-913043cb0ba486850c9cb90c72059b92c1900577
+| +--- org.jetbrains.kotlin:kotlin-parcelize-runtime:2.1.21 -> 2.3.20 (*)
+| +--- androidx.core:core-ktx:1.13.1 -> 1.16.0 (*)
+| +--- androidx.appcompat:appcompat:1.7.0 -> 1.7.1 (*)
+| +--- com.google.android.material:material:1.12.0 (*)
+| +--- androidx.webkit:webkit:1.11.0 -> 1.15.0 (*)
+| +--- com.google.code.gson:gson:2.8.9 -> 2.13.2
+| | \--- com.google.errorprone:error_prone_annotations:2.41.0
+| +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.10.2 (*)
+| +--- org.jetbrains.kotlinx:kotlinx-serialization-json:1.7.3
+| | \--- org.jetbrains.kotlinx:kotlinx-serialization-json-jvm:1.7.3
+| | +--- org.jetbrains.kotlinx:kotlinx-serialization-bom:1.7.3 (*)
+| | +--- org.jetbrains.kotlin:kotlin-stdlib:2.0.20 -> 2.3.20 (*)
+| | \--- org.jetbrains.kotlinx:kotlinx-serialization-core:1.7.3 (*)
+| +--- org.jsoup:jsoup:1.18.1 -> 1.22.1
+| +--- com.squareup.okhttp3:okhttp:4.12.0 -> 5.3.2 (*)
+| \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.3.20 (*)
-\--- org.wordpress.gutenbergkit:android:v0.11.1 (*)
+\--- org.wordpress.gutenbergkit:android:339-913043cb0ba486850c9cb90c72059b92c1900577 (*) |
|
|
|
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## jkmassel/builder-injectable #22579 +/- ##
===============================================================
+ Coverage 37.22% 37.25% +0.03%
===============================================================
Files 2317 2318 +1
Lines 124420 124455 +35
Branches 16890 16893 +3
===============================================================
+ Hits 46316 46371 +55
+ Misses 74356 74334 -22
- Partials 3748 3750 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
dcalhoun
left a comment
There was a problem hiding this comment.
Thank you for implementing this! 🙇🏻♂️
I was unable to fully test the implementation due to the issues noted in inline comments and in wordpress-mobile/GutenbergKit#316 (review). They kept me from truly experiencing the intended functionality.
In addition to the inline comments here, I was unable to upload new media or attach Media Library items to block. When I attempted to do so, the media never displayed in the block editor canvas, the block remained as a placeholder. There were no logs in the Android Studio or Chrome console. 😕
Hopefully we can identify the root cause for some of these foundational problems and I can perform deeper testing afterwards.
| ) | ||
| } else { | ||
| WpAuthenticationProvider.staticWithUsernameAndPassword( | ||
| username = site.apiRestUsernamePlain, |
There was a problem hiding this comment.
I encountered a crash when opening the editor for the time with a Wow/Atomic site. GBK and plugins features were enabled. Before opening the editor, I created an application password for the site via tapping the prompt atop the My Site view.
Stack trace
FATAL EXCEPTION: DefaultDispatcher-worker-6 (Fix with AI)
Process: com.jetpack.android.beta, PID: 3797
java.lang.NullPointerException: getApiRestUsernamePlain(...) must not be null
at org.wordpress.android.fluxc.network.rest.wpapi.rs.WpApiClientProvider.getWpApiClient(WpApiClientProvider.kt:43)
at org.wordpress.android.fluxc.network.rest.wpapi.rs.WpApiClientProvider.getWpApiClient$default(WpApiClientProvider.kt:32)
at org.wordpress.android.repositories.ThemeRepository$fetchCurrentTheme$2.invokeSuspend(ThemeRepository.kt:27)
at org.wordpress.android.repositories.ThemeRepository$fetchCurrentTheme$2.invoke(Unknown Source:8)
at org.wordpress.android.repositories.ThemeRepository$fetchCurrentTheme$2.invoke(Unknown Source:4)
at kotlinx.coroutines.intrinsics.UndispatchedKt.startUndspatched(Undispatched.kt:66)
at kotlinx.coroutines.intrinsics.UndispatchedKt.startUndispatchedOrReturn(Undispatched.kt:43)
at kotlinx.coroutines.BuildersKt__Builders_commonKt.withContext(Builders.common.kt:165)
at kotlinx.coroutines.BuildersKt.withContext(Unknown Source:1)
at org.wordpress.android.repositories.ThemeRepository.fetchCurrentTheme(ThemeRepository.kt:26)
at org.wordpress.android.repositories.EditorSettingsRepository.fetchThemeBlockStyleSupport(EditorSettingsRepository.kt:162)
at org.wordpress.android.repositories.EditorSettingsRepository.access$fetchThemeBlockStyleSupport(EditorSettingsRepository.kt:19)
at org.wordpress.android.repositories.EditorSettingsRepository$fetchEditorCapabilitiesForSite$2$1$2.invokeSuspend(EditorSettingsRepository.kt:110)
at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:34)
at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:100)
at kotlinx.coroutines.internal.LimitedDispatcher$Worker.run(LimitedDispatcher.kt:124)
at kotlinx.coroutines.scheduling.TaskImpl.run(Tasks.kt:89)
at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:586)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:820)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:717)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:704)
Suppressed: kotlinx.coroutines.internal.DiagnosticCoroutineContextException: [StandaloneCoroutine{Cancelling}@4908726, Dispatchers.IO]
This did not occur in the prototype build, only a local build. After it occurred, I app continued to crash on launch, presumably the same site remains selected on My Site.
| mUseThirdPartyBlocksPref = | ||
| (WPSwitchPreference) getChangePref(R.string.pref_key_use_third_party_blocks); | ||
| mUseThirdPartyBlocksPref.setChecked(mSiteSettings.getUseThirdPartyBlocks()); |
There was a problem hiding this comment.
Additionally, when I enabled this option for a newly added jetpack.wpmt.co site, I encountered a crash.
Stack trace
FATAL EXCEPTION: main (Fix with AI)
Process: com.jetpack.android.beta, PID: 30058
java.lang.AssertionError: Cannot get asset path from empty bundle
at org.wordpress.gutenberg.model.EditorAssetBundle.assetDataPath(EditorAssetBundle.kt:137)
at org.wordpress.gutenberg.model.EditorAssetBundle.hasAssetData(EditorAssetBundle.kt:120)
at org.wordpress.gutenberg.CachedAssetRequestInterceptor.handleRequest(CachedAssetRequestInterceptor.kt:48)
at org.wordpress.gutenberg.GutenbergView$initializeWebView$1.shouldInterceptRequest(GutenbergView.kt:395)
at WV.og.a(chromium-SystemWebViewGoogle6432.aab-stable-755913303:101)
at org.chromium.android_webview.ShouldInterceptRequestMediator.shouldInterceptRequestFromNative(chromium-SystemWebViewGoogle6432.aab-stable-755913303:18)
Using the pull-to-refresh gesture on My Site seems to address it. I was able to then open the editor and see Jetpack blocks.
| if (isWPComSimpleSite()) { | ||
| return "https://public-api.wordpress.com/wp/v2/sites/" | ||
| + mSiteId; | ||
| } |
There was a problem hiding this comment.
I asked Claud to assess if this might introduce unexpected behavior elsewhere if existing callers expect a null value. It seems to think it is OK.
Claude's findings
Summary
┌──────────┬────────────────────────────────┬─────────────────────────────────────────────────────────────────────────────┐
│ Severity │ File │ Issue │
├──────────┼────────────────────────────────┼─────────────────────────────────────────────────────────────────────────────┤
│ Medium │ CookieNonceAuthenticator │ Null-check-based discovery/retry logic broken for WP.com simple sites │
├──────────┼────────────────────────────────┼─────────────────────────────────────────────────────────────────────────────┤
│ Medium │ ReactNativeStore │ Same null-check-based discovery/retry logic broken │
├──────────┼────────────────────────────────┼─────────────────────────────────────────────────────────────────────────────┤
│ Low │ WpApiClientProvider.buildUrl() │ Intentional change, but affects getWpApiClientCookiesNonceAuthentication() │
├──────────┼────────────────────────────────┼─────────────────────────────────────────────────────────────────────────────┤
│ None │ All other callers │ Either write-only, guarded by isUsingSelfHostedRestApi, or in the PR itself │
└──────────┴────────────────────────────────┴─────────────────────────────────────────────────────────────────────────────┘
Recommendation
The practical risk is low because CookieNonceAuthenticator and ReactNativeStore are only used for self-hosted/cookie-nonce auth flows, and WP.com simple
sites shouldn't reach those code paths. However, the semantic change to the getter is a landmine for future code — anyone writing if (site.wpApiRestUrl !=
null) as a guard for "has a real persisted REST URL" will silently get wrong behavior for WP.com simple sites.
Consider either:
1. Adding a separate method like getEffectiveWpApiRestUrl() for the synthesized URL and leaving getWpApiRestUrl() as a pure field accessor, or
2. Adding a hasPersistedWpApiRestUrl() method that checks the raw field, so callers that need to distinguish "synthesized" from "persisted" can do so
explicitly.
| private fun getUseThirdPartyBlocks(site: SiteModel): Boolean { | ||
| if (!editorSettingsRepository | ||
| .getSupportsEditorAssetsForSite(site) | ||
| ) { | ||
| return false | ||
| } | ||
| return siteSettingsProvider | ||
| .getSettings(site)?.useThirdPartyBlocks ?: false | ||
| } |
There was a problem hiding this comment.
Should we have this continue to respect the remote gutenberg_kit_plugins feature flag?
| // hide third-party blocks preference if GutenbergKit is not enabled | ||
| if (!mGutenbergKitFeatureChecker.isGutenbergKitEnabled()) { | ||
| WPPrefUtils.removePreference( | ||
| this, R.string.pref_key_site_editor, R.string.pref_key_use_third_party_blocks | ||
| ); |
There was a problem hiding this comment.
Should we also hide this if the remote gutenberg_kit_plugins feature flag is disabled?
| <string name="site_settings_use_theme_styles_unsupported">Install the Gutenberg Plugin on your site to activate theme style support.</string> | ||
| <string name="site_settings_use_theme_styles_not_block_theme">Your site isn\'t using a Block Theme, so the editor might not match your content correctly. If things aren\'t looking right, you can disable editor styles.</string> | ||
| <string name="site_settings_use_third_party_blocks">Use Third-Party Blocks (Beta)</string> | ||
| <string name="site_settings_use_third_party_blocks_summary">Load third-party blocks and styles from plugins installed on your site.</string> |
There was a problem hiding this comment.
Maybe styles is superfluous? Blocks include the styles. Are there editor plugins that load styles without blocks? Does it impact GBK?
| <string name="site_settings_use_third_party_blocks_summary">Load third-party blocks and styles from plugins installed on your site.</string> | |
| <string name="site_settings_use_third_party_blocks_summary">Load third-party blocks from plugins installed on your site.</string> |
| gutenbergView.setLatestContentProvider( | ||
| object : GutenbergView.LatestContentProvider { | ||
| override fun getLatestContent(): | ||
| GutenbergView.LatestContent? { | ||
| return null | ||
| } | ||
| } | ||
| } | ||
| ) |
There was a problem hiding this comment.
Is this a non-functioning placeholder for integrating #22493?
Gate the "Use Third-Party Blocks" toggle behind the remote gutenberg_kit_plugins feature flag in addition to the existing GutenbergKit and editor assets checks. Also simplify the summary string by removing "and styles" per reviewer feedback. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tion Add ThemeRepository to fetch the active theme via WP API and determine if it is a block theme. Add EditorSettingsRepository to discover editor settings and editor assets route support via manifest/API root queries. Wire SiteSettingsFragment to use EditorSettingsRepository for gating theme styles and third-party blocks toggles. Also adds manifest route fetching methods to WpApiClientProvider for discovering available REST routes on WP.com and self-hosted sites. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove fetchWpComManifestRoutes and fetchSiteManifestRoutes from
WpApiClientProvider. EditorSettingsRepository now uses the standard
WpApiClient.request { it.apiRoot().get() } for all site types,
which already handles WP.com vs self-hosted URL resolution.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Call `EditorSettingsRepository.fetchEditorCapabilitiesForSite` when the user lands on My Site so the route-discovery and theme-detection results are available by the time they open Site Settings. On failure, a snackbar is shown and cached prefs retain their previous values so settings degrade to disabled rather than crashing.
12 tests covering pref delegation, route discovery persistence, theme detection, API errors, transport-level error isolation (one fetch failing doesn't block the other), and the both-fail case. Also fix MySiteViewModelTest for new constructor param.
Points at Automattic/wordpress-rs#1285 which fixes route discovery on WP.com simple sites so editor-assets and editor-settings routes appear in the API root response. Also adapts StatsDataSourceImpl to the new StatsUtmKeys API (String replaced with List<StatsUtmKey>).
Switch from hasRoute (exact path match) to hasRouteForEndpoint
(namespace + endpoint) so that WP.com sites — whose route keys
include a sites/{site_id} prefix — are correctly detected.
Requires wordpress-rs PR #1285 which adds hasRouteForEndpoint
to WpApiDetails and the ApiUrlResolver parameter.
Call fetchEditorCapabilitiesForSite during pull-to-refresh so editor settings prefs stay current without requiring a full screen resume. Show snackbar on failure in both paths.
cd579db to
6b4d402
Compare
c726f7f to
25a9cd5
Compare
6968ec2 to
7af2cc5
Compare
`SiteSettingsFragment` combined the top-level GutenbergKit feature flag, the remote `gutenberg_kit_plugins` flag, the per-site capability cache, and the user's toggle inline — a long conditional chain per capability that's hard to share with other consumers and easy to drift from when those consumers get added. Introduces `EditorCapabilityResolver` as a single source of truth for both theme styles and third-party blocks. Returns a `Resolved` sealed result — `Hidden` / `Unsupported(reason)` / `Available(userEnabled, advisory?)` — that UI callers branch on directly, and non-UI callers collapse via `shouldApplyInEditor`. The theme-not-a-block-theme case is modelled as an advisory, not a gate, matching today's behaviour where the pref stays enabled with an informational summary. Wires `SiteSettingsFragment` through the resolver. The editor-side consumer (the GutenbergKit config builder) will land in a follow-up alongside its refactor to an injectable class.
7af2cc5 to
34887ae
Compare
4c345ed to
86cab95
Compare
Promotes `GutenbergKitSettingsBuilder` from a Kotlin `object` with `buildSettings` returning a `Map<String, Any>` to a `@Singleton` class that returns `EditorConfiguration` directly via `buildPostConfiguration(site, post, accessToken)`. The injectable form is needed so the builder can constructor-inject `EditorCapabilityResolver` and consult per-site theme-styles and third-party-blocks state itself, instead of relying on each call site to assemble a `FeatureConfig`. `EditorConfigurationBuilder` (the intermediate Map → `EditorConfiguration` adapter) is now redundant and removed. `GutenbergKitActivity` adopts the injected builder via a small `buildEditorConfiguration` helper that overlays the per-call locale, cookies, and account fields the builder cannot know about, and drops its now-unused `GutenbergKitPluginsFeature` injection. Settings-builder tests are rewritten against the new ctor and API; capability-gating coverage already lives in `EditorCapabilityResolverTest`.
Introduces `GutenbergEditorPreloader`, a `@Singleton` that prepares `EditorDependencies` per site on dashboard refresh so the editor can open with warm dependencies instead of loading them at launch. - Wires preloading into `MySiteViewModel.buildDashboardOrSiteItems`, replacing the previous `GutenbergKitWarmupHelper` warmup flow. The separate `fetchEditorCapabilitiesWithSnackbar` path is unchanged — the preloader and the snackbar flow share the same repository call, deduplicated at the cache layer. - `GutenbergKitActivity` passes the site to `GutenbergKitEditorFragment`; the fragment resolves preloaded `EditorDependencies` from the preloader by site local ID instead of building them inline. - Removes `GutenbergKitWarmupHelper`.
86cab95 to
12084a6
Compare
b984d31 to
3538ea9
Compare
Three independent fixes surfaced while validating the converted builder; each is small but distinct. ## Handle schemeless URLs in `extractHost` `URI(url).host` returns null for hosts without a scheme (e.g. `example.wordpress.com`), but the OLD `UrlUtils.removeScheme` path that this PR replaced was lenient. `SiteModel.url` can be schemeless (FluxC's `SiteStoreUnitTest.java:472,494` writes `shieldeyesfromlight.wordpress.com` directly), which regressed `siteApiNamespace` (missing the `sites/$host/` alias) and `cachedAssetHosts` (missing the site host). Normalize by prepending `https://` when no scheme is present, then let `URI` parse. Pure JDK, no Android dependency. Added regression tests covering schemeless hosts, ports, userinfo, and integration through `buildSiteApiNamespace` and `buildCachedHosts` (the latter uses the exact value `shieldeyesfromlight.wordpress.com` to lock in the regression). ## Require per-call values explicitly in `buildPostConfiguration` The builder previously hardcoded `locale="en"`, `cookies=emptyMap()`, and `enableNetworkLogging=false`, relying on `GutenbergKitActivity.buildEditorConfiguration` to overlay them via `toBuilder()`. The stacked preloader (#22579) calls `buildPostConfiguration` directly with no overlay, baking the placeholder values into the preloaded `EditorDependencies`. Promoted `locale`, `cookies`, and `isNetworkLoggingEnabled` to required non-default parameters. The activity now passes them at the call site; the `.toBuilder()` overlay is gone. The preloader will fail to compile on rebase until it supplies the values — exactly the failure mode we want. ## Gate `editorAssetsEndpoint` on capability and correct the route check Two related bugs: - `EditorSettingsRepository.fetchRouteSupport` was populating `SiteSupportsEditorAssets` from `/wp-block-editor/v1/assets` — a copy-paste from the line above. The URL we actually hit is `wpcom/v2/editor-assets`, which a vanilla self-hosted WP 6.7+ install doesn't expose. So `getSupportsEditorAssetsForSite` could be true on sites where the URL 404s. - This PR set `editorAssetsEndpoint` unconditionally; the safety net was GutenbergKit's `plugins == false` short-circuit, which fails to protect when `plugins == true` on a site that lacks `wpcom/v2/editor-assets`. Fixed both: the route check now queries `/wpcom/v2, editor-assets`, and the URL is only set when `resolveThirdPartyBlocks(site).isAvailable`. Both `plugins` and `editorAssetsEndpoint` flow from the same resolver call and cannot drift. (Considered using `resolver.resolve(...)` to canonicalise the URL, but the concrete uniffi resolver classes load JNA on instantiation and the unit-test classpath doesn't include the native lib. The string-concat output is byte-identical to the resolver output for both WP.com and self-hosted shapes, so the resolver was purely cosmetic here.)






Description
Adds background preloading of GutenbergKit editor dependencies so the block editor opens faster. When the My Site dashboard loads,
GutenbergEditorPreloaderfetches editor capabilities, theme styles, and third-party block settings on a background thread. If preloading completes before the editor is opened, the cached result is used immediately; otherwise the editor loads dependencies itself.Key changes:
GutenbergEditorPreloadersingleton driven byMySiteViewModelthat opportunistically warmsEditorDependencyStorein the background. Supports idempotent preload, forced refresh (pull-to-refresh), and graceful cancellation.EditorSettingsRepositoryandThemeRepositoryfor fetching editor capabilities and active theme data via WP API. Per-site preferences track whether the site supports editor settings and editor assets.objectto an injectable@Singletonclass. Now reads "Use Theme Styles" and "Use Third-Party Blocks" from site settings to configure the editor. Builds an editor assets endpoint for both WP.com and self-hosted sites.WpApiClientProviderandSiteModel.getWpApiRestUrl()now handle WP.com simple sites that lack a self-hosted REST URL.Testing instructions
Editor preloading on WP.com site:
Pull-to-refresh clears preloaded dependencies:
Editor configuration for self-hosted site with application password:
Theme styles and third-party blocks settings:
Local draft post ID handling:
I tested against the following sites: