-
-
Notifications
You must be signed in to change notification settings - Fork 103
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
chore(android): enables debugging and inspection of mobile app internal webviews #11215
Conversation
User Test ResultsTest specification and instructions User tests are not required |
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.
Looks good, but I think we should frame this with alpha+beta checks so it doesn't slip into stable
|
||
if ( | ||
0 != (context.getApplicationInfo().flags & ApplicationInfo.FLAG_DEBUGGABLE) | ||
|| KMManager.getTier(null) != KMManager.Tier.STABLE |
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.
keyman/android/KMEA/app/src/main/java/com/keyman/engine/KMManager.java
Lines 339 to 348 in 93dc41e
/** | |
* Extract KMEA tier from versionName. Uses parameter so we can unit test. | |
* @param versionName String - If not provided, determine tier from | |
* com.keyman.engine.BuildConfig.KEYMAN_ENGINE_VERSION_NAME | |
* @return Tier (ALPHA, BETA, STABLE) | |
*/ | |
public static Tier getTier(String versionName) { | |
if (versionName == null || versionName.isEmpty()) { | |
versionName = com.keyman.engine.BuildConfig.KEYMAN_ENGINE_VERSION_NAME; | |
} |
When left null
, .getTier
uses the current engine version.
Just peachy. For the iOS build:
For the Android build:
So... apparently that method isn't available when unit testing. Locally testing, simply blocking the method from being reached during such scenarios is enough, so... I just added an appropriate check on the latest commit. |
@@ -130,6 +130,12 @@ class KeymanWebViewController: UIViewController { | |||
webView!.backgroundColor = UIColor.clear | |||
webView!.navigationDelegate = self | |||
webView!.scrollView.isScrollEnabled = false | |||
|
|||
if #available(iOSApplicationExtension 16.4, *) { | |||
if(Version.current.tier != .stable) { |
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.
Potential improvement: also add check for if local
or test
; those should be allowed to override .stable, since they're inherently less stable by virtue of being development versions.
I don't think our iOS engine's Version
class yet exposes those as flags, though...
iOS build failure:
|
I've gone ahead and linked this detail to #10671. For the sake of moving ahead with this task for Android, I think I'll do a quick rebase to split the iOS changes off into their own PR that can await it without blocking the Android changes. |
16baa14
to
6f3c5e0
Compare
android/KMEA/app/src/main/java/com/keyman/engine/KMKeyboard.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Marc Durdin <marc@durdin.net>
Changes in this pull request will be available for download in Keyman version 17.0.309-beta |
This PR exists to facilitate diagnosis of #11186 when it occurs.
@keymanapp-test-bot skip