-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat: Enable GutenbergKit asset caching #22035
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e6e956c
d7f1999
f013498
dd36e2c
20288d0
fd98574
3ef5c35
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -258,7 +258,6 @@ import org.wordpress.android.viewmodel.storage.StorageUtilsViewModel | |
| import org.wordpress.android.widgets.AppReviewManager.incrementInteractions | ||
| import org.wordpress.android.widgets.WPSnackbar.Companion.make | ||
| import org.wordpress.android.widgets.WPViewPager | ||
| import org.wordpress.gutenberg.GutenbergWebViewPool | ||
| import org.wordpress.aztec.AztecExceptionHandler | ||
| import org.wordpress.aztec.exceptions.DynamicLayoutGetBlockIndexOutOfBoundsException | ||
| import org.wordpress.aztec.util.AztecLog | ||
|
|
@@ -798,9 +797,6 @@ class EditPostActivity : BaseAppCompatActivity(), EditorFragmentActivity, Editor | |
| } | ||
|
|
||
| private fun setupEditor() { | ||
| if (isGutenbergKitEditor) { | ||
| GutenbergWebViewPool.getPreloadedWebView(this) | ||
| } | ||
| // Check whether to show the visual editor | ||
|
|
||
| // NOTE: Migrate to 'androidx.preference.PreferenceManager' and 'androidx.preference.Preference' | ||
|
|
@@ -2503,8 +2499,21 @@ class EditPostActivity : BaseAppCompatActivity(), EditorFragmentActivity, Editor | |
| onXpostsSettingsCapability(isXpostsCapable) | ||
| } | ||
|
|
||
| val isWpCom = site.isWPCom || siteModel.isPrivateWPComAtomic || siteModel.isWPComAtomic | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| val gutenbergWebViewAuthorizationData = GutenbergWebViewAuthorizationData( | ||
| val isWpCom = site.isWPCom || siteModel.isWPComAtomic | ||
| val gutenbergWebViewAuthorizationData = createGutenbergWebViewAuthorizationData(isWpCom) | ||
| val settings = createGutenbergKitSettings(isWpCom) | ||
|
|
||
| return GutenbergKitEditorFragment.newInstance( | ||
| getContext(), | ||
| isNewPost, | ||
| gutenbergWebViewAuthorizationData, | ||
| jetpackFeatureRemovalPhaseHelper.shouldShowJetpackPoweredEditorFeatures(), | ||
| settings | ||
| ) | ||
| } | ||
|
|
||
| private fun createGutenbergWebViewAuthorizationData(isWpCom: Boolean): GutenbergWebViewAuthorizationData { | ||
| return GutenbergWebViewAuthorizationData( | ||
| siteModel.url, | ||
| isWpCom, | ||
| accountStore.account.userId, | ||
|
|
@@ -2518,21 +2527,28 @@ class EditPostActivity : BaseAppCompatActivity(), EditorFragmentActivity, Editor | |
| userAgent.toString(), | ||
| isJetpackSsoEnabled | ||
| ) | ||
| } | ||
|
|
||
| private fun createGutenbergKitSettings(isWpCom: Boolean): MutableMap<String, Any?> { | ||
| val postType = if (editPostRepository.isPage) "page" else "post" | ||
| val siteApiRoot = if (isWpCom) "https://public-api.wordpress.com/" else "" | ||
| val authToken = accountStore.accessToken | ||
| val authHeader = "Bearer $authToken" | ||
| val siteApiNamespace = arrayOf("sites/${site.siteId}", "sites/${UrlUtils.removeScheme(siteModel.url)}") | ||
| val siteURL = siteModel.url | ||
| val siteApiRoot = if (isWpCom) "https://public-api.wordpress.com/" | ||
| else siteModel.wpApiRestUrl ?: "$siteURL/wp-json/" | ||
|
Comment on lines
+2535
to
+2536
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As a future improvement, I'd love for us to be a bit more clear on when the editor is fully supported and we know the logic is correct and when it's not or we are making a guess.
My main feedback - and it's not about this PR - is that I am having a hard time following the current code, mostly because it's only partially implemented. If it was fully implemented and I just needed to process it, than it'd be easy, but that's impossible right now without at least comments for each situation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree. Thank you for the note. Centralizing and documenting common calculations would definitely help communicate intent. A utility for converting |
||
| // Use the application password for self-hosted sites when available | ||
| val authHeader = if (isWpCom) "Bearer ${accountStore.accessToken}" else "Basic " | ||
| val siteApiNamespace = if (isWpCom) | ||
| arrayOf("sites/${site.siteId}/", "sites/${UrlUtils.removeScheme(siteURL)}/") | ||
| else arrayOf() | ||
|
|
||
| val languageString = perAppLocaleManager.getCurrentLocaleLanguageCode() | ||
| val wpcomLocaleSlug = languageString.replace("_", "-").lowercase() | ||
|
|
||
| val settings = mutableMapOf<String, Any?>( | ||
| return mutableMapOf( | ||
| "postId" to editPostRepository.getPost()?.remotePostId?.toInt(), | ||
| "postType" to postType, | ||
| "postTitle" to editPostRepository.getPost()?.title, | ||
| "postContent" to editPostRepository.getPost()?.content, | ||
| "siteURL" to siteURL, | ||
| "siteApiRoot" to siteApiRoot, | ||
| "namespaceExcludedPaths" to arrayOf("/wpcom/v2/following/recommendations", "/wpcom/v2/following/mine"), | ||
| "authHeader" to authHeader, | ||
|
|
@@ -2552,14 +2568,6 @@ class EditPostActivity : BaseAppCompatActivity(), EditorFragmentActivity, Editor | |
| ) | ||
| ) | ||
| ) | ||
|
|
||
| return GutenbergKitEditorFragment.newInstance( | ||
| getContext(), | ||
| isNewPost, | ||
| gutenbergWebViewAuthorizationData, | ||
| jetpackFeatureRemovalPhaseHelper.shouldShowJetpackPoweredEditorFeatures(), | ||
| settings | ||
| ) | ||
| } | ||
|
|
||
| private fun createGutenbergEditorFragment(): GutenbergEditorFragment { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,6 +41,7 @@ | |
| import org.wordpress.android.util.AppLog.T; | ||
| import org.wordpress.android.util.PermissionUtils; | ||
| import org.wordpress.android.util.ProfilingUtils; | ||
| import org.wordpress.android.util.UrlUtils; | ||
| import org.wordpress.android.util.helpers.MediaFile; | ||
| import org.wordpress.android.util.helpers.MediaGallery; | ||
| import org.wordpress.aztec.IHistoryListener; | ||
|
|
@@ -58,6 +59,7 @@ | |
| import java.util.ArrayList; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Set; | ||
| import java.util.concurrent.CountDownLatch; | ||
|
|
||
| import static org.wordpress.gutenberg.Media.createMediaUsingMimeType; | ||
|
|
@@ -72,6 +74,7 @@ public class GutenbergKitEditorFragment extends EditorFragmentAbstract implement | |
| @Nullable private GutenbergView mGutenbergView; | ||
| private static final String GUTENBERG_EDITOR_NAME = "gutenberg"; | ||
| private static final String KEY_HTML_MODE_ENABLED = "KEY_HTML_MODE_ENABLED"; | ||
| private static final String KEY_EDITOR_STARTED = "KEY_EDITOR_STARTED"; | ||
| private static final String KEY_EDITOR_DID_MOUNT = "KEY_EDITOR_DID_MOUNT"; | ||
| private static final String ARG_IS_NEW_POST = "param_is_new_post"; | ||
| private static final String ARG_GUTENBERG_WEB_VIEW_AUTH_DATA = "param_gutenberg_web_view_auth_data"; | ||
|
|
@@ -90,6 +93,7 @@ public class GutenbergKitEditorFragment extends EditorFragmentAbstract implement | |
| @Nullable private OpenMediaLibraryListener mOpenMediaLibraryListener = null; | ||
| @Nullable private LogJsExceptionListener mOnLogJsExceptionListener = null; | ||
|
|
||
| private boolean mEditorStarted; | ||
| private boolean mEditorDidMount; | ||
| @Nullable | ||
| private View mRootView; | ||
|
|
@@ -124,6 +128,7 @@ public void onCreate(@Nullable Bundle savedInstanceState) { | |
|
|
||
| if (savedInstanceState != null) { | ||
| mHtmlModeEnabled = savedInstanceState.getBoolean(KEY_HTML_MODE_ENABLED); | ||
| mEditorStarted = savedInstanceState.getBoolean(KEY_EDITOR_STARTED); | ||
| mEditorDidMount = savedInstanceState.getBoolean(KEY_EDITOR_DID_MOUNT); | ||
| mFeaturedImageId = savedInstanceState.getLong(ARG_FEATURED_IMAGE_ID); | ||
| } | ||
|
|
@@ -260,6 +265,7 @@ public void onAttach(Activity activity) { | |
| @Override | ||
| public void onSaveInstanceState(Bundle outState) { | ||
| outState.putBoolean(KEY_HTML_MODE_ENABLED, mHtmlModeEnabled); | ||
| outState.putBoolean(KEY_EDITOR_STARTED, mEditorStarted); | ||
| outState.putBoolean(KEY_EDITOR_DID_MOUNT, mEditorDidMount); | ||
| outState.putLong(ARG_FEATURED_IMAGE_ID, mFeaturedImageId); | ||
| } | ||
|
|
@@ -508,6 +514,7 @@ public void onDestroy() { | |
| mHistoryChangeListener = null; | ||
| mFeaturedImageChangeListener = null; | ||
| } | ||
| mEditorStarted = false; | ||
| super.onDestroy(); | ||
| } | ||
|
|
||
|
|
@@ -556,7 +563,7 @@ public void onEditorThemeUpdated(Bundle editorTheme) { | |
| } | ||
|
|
||
| public void startWithEditorSettings(@NonNull String editorSettings) { | ||
| if (mGutenbergView == null) { | ||
| if (mGutenbergView == null || mEditorStarted) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Due to the multiple FluxC subscription updates, the existing logic attempts to start the editor twice on open and once on close. This new state guards against unnecessary editor starts. Additional details in dd36e2c. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change has caused the regression mentioned here. |
||
| return; | ||
| } | ||
|
|
||
|
|
@@ -565,6 +572,12 @@ public void startWithEditorSettings(@NonNull String editorSettings) { | |
| postId = -1; | ||
| } | ||
|
|
||
| var siteURL = (String) mSettings.get("siteURL"); | ||
| var siteApiRoot = (String) mSettings.get("siteApiRoot"); | ||
| var siteApiNamespace = (String[]) mSettings.get("siteApiNamespace"); | ||
| var firstNamespace = siteApiNamespace != null && siteApiNamespace.length > 0 ? siteApiNamespace[0] : ""; | ||
| var editorAssetsEndpoint = siteApiRoot + "wpcom/v2/" + firstNamespace + "editor-assets"; | ||
|
|
||
| EditorConfiguration config = new EditorConfiguration.Builder() | ||
| .setTitle((String) mSettings.get("postTitle")) | ||
| .setContent((String) mSettings.get("postContent")) | ||
|
|
@@ -573,14 +586,18 @@ public void startWithEditorSettings(@NonNull String editorSettings) { | |
| .setThemeStyles((Boolean) mSettings.get("themeStyles")) | ||
| .setPlugins((Boolean) mSettings.get("plugins")) | ||
| .setSiteApiRoot((String) mSettings.get("siteApiRoot")) | ||
| .setSiteApiNamespace((String[]) mSettings.get("siteApiNamespace")) | ||
| .setSiteApiNamespace((String[]) siteApiNamespace) | ||
| .setNamespaceExcludedPaths((String[]) mSettings.get("namespaceExcludedPaths")) | ||
| .setAuthHeader((String) mSettings.get("authHeader")) | ||
| .setWebViewGlobals((List<WebViewGlobal>) mSettings.get("webViewGlobals")) | ||
| .setEditorSettings(editorSettings) | ||
| .setLocale((String) mSettings.get("locale")) | ||
| .setEditorAssetsEndpoint(editorAssetsEndpoint) | ||
| .setCachedAssetHosts(Set.of("s0.wp.com", UrlUtils.getHost(siteURL))) | ||
| .setEnableAssetCaching(true) | ||
| .build(); | ||
|
|
||
| mEditorStarted = true; | ||
| mGutenbergView.start(config); | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This performance optimization leads to duplicate "editor mounted" events, which results in invisible editor UI. We should reinstate this after refactoring the WebView initialization to avoid this issue.
Additional details capture in f013498.