Integrate Gutenberg Preloading#22579
Merged
Merged
Conversation
Collaborator
Generated by 🚫 Danger |
jkmassel
force-pushed
the
integrate/gutenberg-preloading
branch
2 times, most recently
from
7d72605 to
e221a2a
Compare
Contributor
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 (*) |
Contributor
|
|
Contributor
|
|
jkmassel
added
unit-tests-exemption
Gutenberg
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #22579 +/- ##
==========================================
+ Coverage 37.28% 37.31% +0.03%
==========================================
Files 2318 2319 +1
Lines 124520 124558 +38
Branches 16917 16920 +3
==========================================
+ Hits 46424 46484 +60
+ Misses 74342 74319 -23
- Partials 3754 3755 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
jkmassel
requested review from
adalpari and
dcalhoun
and removed request for
a team
dcalhoun
reviewed
Feb 13, 2026
Member
dcalhoun
left a comment
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, |
Member
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.
Comment on lines
+1045
to
+1047
| mUseThirdPartyBlocksPref = | ||
| (WPSwitchPreference) getChangePref(R.string.pref_key_use_third_party_blocks); | ||
| mUseThirdPartyBlocksPref.setChecked(mSiteSettings.getUseThirdPartyBlocks()); |
Member
There was a problem hiding this comment.
This was unexpectedly disabled for my WP.com Simple site.
Disabled third-party plugins toggle
Member
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.
Comment on lines
+441
to
+444
| if (isWPComSimpleSite()) { | ||
| return "https://public-api.wordpress.com/wp/v2/sites/" | ||
| + mSiteId; | ||
| } |
Member
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.
Comment on lines
+160
to
+168
| private fun getUseThirdPartyBlocks(site: SiteModel): Boolean { | ||
| if (!editorSettingsRepository | ||
| .getSupportsEditorAssetsForSite(site) | ||
| ) { | ||
| return false | ||
| } | ||
| return siteSettingsProvider | ||
| .getSettings(site)?.useThirdPartyBlocks ?: false | ||
| } |
Member
There was a problem hiding this comment.
Should we have this continue to respect the remote gutenberg_kit_plugins feature flag?
Comment on lines
+1110
to
+1114
| // 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 | ||
| ); |
Member
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> |
Member
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?
Suggested change
| <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> |
Comment on lines
+222
to
+229
| gutenbergView.setLatestContentProvider( | ||
| object : GutenbergView.LatestContentProvider { | ||
| override fun getLatestContent(): | ||
| GutenbergView.LatestContent? { | ||
| return null | ||
| } | ||
| } | ||
| } | ||
| ) |
Member
There was a problem hiding this comment.
Is this a non-functioning placeholder for integrating #22493?
jkmassel
added a commit
that referenced
this pull request
May 14, 2026
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.)
jkmassel
added a commit
that referenced
this pull request
May 19, 2026
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.)
jkmassel
force-pushed
the
jkmassel/builder-injectable
branch
2 times, most recently
from
e7e5d52 to
d851897
Compare
jkmassel
force-pushed
the
integrate/gutenberg-preloading
branch
2 times, most recently
from
99d4d3b to
2bd5339
Compare
jkmassel
added a commit
that referenced
this pull request
May 20, 2026
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.)
jkmassel
force-pushed
the
jkmassel/builder-injectable
branch
from
d851897 to
42901a7
Compare
jkmassel
added a commit
that referenced
this pull request
May 20, 2026
* Convert GutenbergKitSettingsBuilder to injectable class 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`. * Address adversarial-review findings on the injectable builder 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.) * Make buildAuthHeader internal
jkmassel
force-pushed
the
integrate/gutenberg-preloading
branch
from
2bd5339 to
b997b60
Compare
Contributor
|
@jkmassel On a self-hosted site, I very briefly see the progress bar, which is quickly replaced with a progress spinner. Should the bar be there? selfhosted.mp4On a private Atomic site, I see a "Accessing content of a private site" dialog, under which you can clearly see a "Loading editor" progress bar. atomic.mp4Also, I had Claude do a review and save it as a PDF. Does anything in it need to be addressed? |
Contributor
Author
Wow good eye – I think I know what's up there, and I can address it in a subsequent PR. tl;dr – that flash is the time it takes the preloader to read the data on-disk (which actually proves it's working!). There'll need to be a separate preloader invocation for the post list. I think it makes sense to do that as a followup PR – you should be able to get the "no progress bar at all" experience by tapping "New Post" on my "My Site" screen. (same comment for the "Accessing Private Site" stuff) I discovered some unrelated bugs while testing this that I'll also address separate. a893813 addresses some of the feedback from Claude, but some notes:
Will be addressed separately – there's plumbing in wordpress-mobile/GutenbergKit#493 for it, but it should be its own PR.
This is what we want – if the viewModel goes away, we want any in-flight work to be dropped. |
Squashes 43 in-flight commits from integrate/gutenberg-preloading. Pre-squash tip is preserved at backup/integrate-gutenberg-preloading-pre-rebase. Key changes: - GutenbergEditorPreloader: per-site background preload driven by MySiteViewModel, replacing the old GutenbergKitWarmupHelper. - EditorSettingsRepository + ThemeRepository: capability detection for editor-settings / editor-assets routes and block-theme status. - WpApiClientProvider + SiteModel: WP.com simple site support. - GutenbergKitSettingsBuilder: reads useThemeStyles / useThirdPartyBlocks via SiteSettingsProvider, with capability gating inlined (no EditorCapabilityResolver). - SiteSettingsFragment + SiteSettingsModel: "Use Third-Party Blocks" toggle and DB migration 70→71.
jkmassel
force-pushed
the
integrate/gutenberg-preloading
branch
from
a893813 to
3fb2015
Compare
SonarQube kotlin:S6517 — single-method interface is more idiomatic as fun interface; lets Hilt/Dagger and tests treat it as a SAM type.
Project lint rule DoNotMockDataClass flags mocking data classes. The stub doesn't care about the contents — anything returned will be handed to a stubbed editorServiceProvider.prepare — so build a minimal real EditorConfiguration instead.
Followup to 675e40c — that commit replaced the only remaining mock() call with a real EditorConfiguration, so the import is now unused. Detekt UnusedImports flagged it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds opportunistic background preloading of GutenbergKit
EditorDependenciesso the block editor opens with warm dependencies instead of loading them at launch.GutenbergEditorPreloaderis driven byMySiteViewModel— when a site becomes visible on the My Site dashboard it kicks off a per-site preload on a background thread. If the preload completes before the user opens the editor, the cachedEditorDependenciesare handed toGutenbergViewat construction time; otherwise the editor loads them itself. Caching is keyed bySiteModel.id, so switching between sites doesn't discard previously preloaded results, and pull-to-refresh forces a fresh fetch.Changes
WordPress/src/main/java/org/wordpress/android/ui/posts/GutenbergEditorPreloader.kt: new@SingletonexposingpreloadIfNeeded/refreshPreloading/getDependencies/clear. State is aConcurrentHashMap<Int, PreloadState>(per-siteLoading(Job)/Ready(EditorDependencies)). Failures evict the entry so the next visit retries.WordPress/src/main/java/org/wordpress/android/ui/posts/EditorServiceProvider.kt+EditorServiceProviderImpl.kt: thin abstraction overEditorService.create(...).prepare(null)so the preloader can be unit-tested without the real service.MySiteViewModel:buildDashboardOrSiteItemscallspreloadIfNeeded(orrefreshPreloadingon pull-to-refresh);onClearedcallsclear(). Replaces theGutenbergKitWarmupHelperflow, which is deleted. Sits alongside the existingfetchEditorCapabilitiesWithSnackbarfrom Add EditorCapabilityResolver to centralize editor-capability gating #22812 — they share the same repository under the hood, deduplicated at the cache layer.GutenbergKitEditorFragment.newInstance(configuration, site): now takes aSiteModelso the fragment can resolve its preloadedEditorDependenciesby local ID and pass them toGutenbergView's constructor.GutenbergKitActivityis updated for the new signature.Drive-by:
SiteUtils.isBlockEditorDefaultForNewPostandSiteUtils.alwaysDefaultToGutenbergare marked@Deprecatedin favour ofSiteSettingsProvider.isBlockEditorDefault(added in #22812); three Kotlin callers add@Suppress("DEPRECATION")rather than migrate inline, since the migration is mechanical and can land separately.Related
EditorCapabilityResolver+EditorSettingsRepository+ per-site capability prefs) and Convert GutenbergKitSettingsBuilder to injectable class #22836 (GutenbergKitSettingsBuilderinjectable refactor, WP.com simple site support inWpApiClientProvider/SiteModel). Both merged into trunk on 2026-05-20.Test plan
The user-visible signal is the dependency-loading progress bar that appears inside
GutenbergViewwhile it fetches its dependencies. With preloading working, that bar should not appear when the user opens the editor from a site they've been browsing. A transient spinner while the editor JS loads is a separate thing and is expected.refreshPreloadingpath)./gradlew :WordPress:testJetpackDebugUnitTest --tests "org.wordpress.android.ui.posts.GutenbergEditorPreloaderTest"passes (CI:unit-tests)./gradlew :WordPress:testJetpackDebugUnitTest --tests "org.wordpress.android.ui.mysite.MySiteViewModelTest"passes (CI:unit-tests)