fix: Stabilize Android media uploads #29
Conversation
Address errors originating from version conflicts with the WordPress-Android project.
Address version incompatibility by mirroring the WordPress-Android project.
It is not possible to use `evaluateJavascript` within the `shouldInterceptRequest` callback, as it occurs to early. This resulted in the absence of the `GBKit` global.
Defaulting this value to `false` should suffice.
The weak references resulted in unexpectedly null callback values, which meant that the media upload callback would occasionally fail.
This reverts commit f755623.
It is not possible to use `evaluateJavascript` within the `shouldInterceptRequest` callback, as it occurs to early. This resulted in the absence of the `GBKit` global.
| private var assetLoader = WebViewAssetLoader.Builder() | ||
| .addPathHandler("/assets/", AssetsPathHandler(this.context)) | ||
| .build() | ||
| private var initialTitle: String = "" | ||
| private var type: String = "" | ||
| private var id: Int? = null | ||
| private var themeStyles: Boolean? = null | ||
| private var themeStyles: Boolean = false |
There was a problem hiding this comment.
Defaulting to false felt simpler than a nullable value.
| var filePathCallback: ValueCallback<Array<Uri?>?>? = null | ||
| val pickImageRequestCode = 1 | ||
| private var onFileChooserRequested: WeakReference<((Intent, Int) -> Unit)?>? = null |
There was a problem hiding this comment.
The weak references seemingly led to unexpectedly discarded event handlers, which led to the "Upload" button sporadically failing to operate. Given we null out these values during view destruction, it feels safe to remove the weak reference.
| if (!hasSetEditorConfig && withConfig) { | ||
| handler.post { | ||
| var editorInitialConfig = getEditorConfiguration() | ||
| view?.evaluateJavascript(editorInitialConfig, null) | ||
|
|
||
| } | ||
| hasSetEditorConfig = true | ||
| } |
There was a problem hiding this comment.
There is a race condition that results in this approach not consistently load the global values before the HTML page loaded.
Unfortunately, Android does not provide a method similar to iOS' addUserScript. So, this "push" approach was replaced with a "pull" approach on Android to ensure the value is available before the first WebView requests are made.
| @@ -1,5 +1,5 @@ | |||
| [versions] | |||
| agp = "8.4.2" | |||
| agp = "8.5.1" | |||
There was a problem hiding this comment.
The AGP and Gradle versions were updated to mirror the WordPress-Android project.
Mitigate cryptic errors from unexpected editor configuration values.
| try { | ||
| return JSON.parse(window.editorDelegate.getEditorConfiguration()); | ||
| } catch (error) { | ||
| console.error('Failed parsing GBKit from editorDelegate:', error); |
There was a problem hiding this comment.
If an error occurs here, is there a way to let the user know something went wrong? And does the message sent to console.error get added to the app log? Apologies if those are basic questions, but I have little experience here.
There was a problem hiding this comment.
Good question. In the event his is triggered—given the fallback value of an empty object—the editor will display normally. However, most actions will undoubtedly fail, creating a confusing experience.
Unfortunately, it is unlikely a user would be recover from an error of this nature themselves, but we could communicate configuration failed and direct them to contact support/community forums. I captured this opportunity in https://github.com/Automattic/dotcom-forge/issues/9453.
!
Description
Relates to wordpress-mobile/WordPress-Android#21223.
Fix various instabilities in Android media uploads. Synchronize Gradle versions
with the WordPress-Android project.
Testing Instructions
See wordpress-mobile/WordPress-Android#21223.