Skip to content
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

fix(android/app): Temporarily disable Keyman browser #8428

Closed
wants to merge 1 commit into from

Conversation

darcywong00
Copy link
Contributor

@darcywong00 darcywong00 commented Mar 14, 2023

Intermediary step towards #2159

This PR temporarily disable access to the in-app browser (Keeps the Keyman app account agonistic)

User Testing

Setup
Install the PR build of Keyman for Android

  • TEST_BROWSER_DISABLED - Verifies Keyman browser can't be accessed
  1. Launch Keyman for Android in portrait orientation
  2. Observe the menu icons and verify the browser icon is not visible
  3. Rotate the device to landscape orientation
  4. Observe the menu icons and verify the browser icon is not visible

@keymanapp-test-bot keymanapp-test-bot bot added has-user-test user-test-required User tests have not been completed labels Mar 14, 2023
@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Mar 14, 2023

User Test Results

Test specification and instructions

  • TEST_BROWSER_DISABLED (PASSED): Tested with the attached PR build (Keyman 16.0.139-test-8428) in Android 12 / API 31 emulator and here is my observation: 1. Verified that the browser icon is not visible on the menu icons in both (Potrait and Landscape) orientations. Seems to be working fine. (notes)

Copy link
Member

@mcdurdin mcdurdin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM I guess?

@mcdurdin
Copy link
Member

Here's how I setup Keyman for Android to allow for a custom CA for use with the Fiddler proxy:

diff --git a/android/KMAPro/kMAPro/src/main/AndroidManifest.xml b/android/KMAPro/kMAPro/src/main/AndroidManifest.xml
index 6acb4d9da..e5e1393ed 100644
--- a/android/KMAPro/kMAPro/src/main/AndroidManifest.xml
+++ b/android/KMAPro/kMAPro/src/main/AndroidManifest.xml
@@ -19,6 +19,7 @@
     android:icon="@mipmap/ic_launcher"
     android:label="@string/app_name"
     android:usesCleartextTraffic="true"
+    android:networkSecurityConfig="@xml/network_security_config"
     android:theme="@style/AppTheme"
     android:supportsRtl="true">
     <receiver android:name=".NetworkStateReceiver"
diff --git a/android/KMAPro/kMAPro/src/main/res/xml/network_security_config.xml b/android/KMAPro/kMAPro/src/main/res/xml/network_security_config.xml
new file mode 100644
index 000000000..ad83b21d7
--- /dev/null
+++ b/android/KMAPro/kMAPro/src/main/res/xml/network_security_config.xml
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="utf-8"?>
+<network-security-config>
+  <base-config>
+    <trust-anchors>
+      <!-- Trust preinstalled CAs -->
+      <certificates src="system" />
+      <!-- HERE: Additionaly trus user added CAs -->
+      <certificates src="user"/>
+    </trust-anchors>
+  </base-config>
+</network-security-config>
\ No newline at end of file

@darcywong00 darcywong00 changed the base branch from stable-16.0 to master March 14, 2023 04:50
@darcywong00 darcywong00 changed the base branch from master to stable-16.0 March 14, 2023 04:50
@darcywong00
Copy link
Contributor Author

Will redo towards 17.0 alpha

@bharanidharanj
Copy link

Test Results

  • TEST_BROWSER_DISABLED (PASSED): Tested with the attached PR build (Keyman 16.0.139-test-8428) in Android 12 / API 31 emulator and here is my observation: 1. Verified that the browser icon is not visible on the menu icons in both (Potrait and Landscape) orientations. Seems to be working fine.

..in Portrait view

..in Landscape view

@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-required User tests have not been completed label Mar 14, 2023
@mcdurdin mcdurdin added this to the A17S10 milestone Apr 4, 2023
@mcdurdin mcdurdin modified the milestones: A17S10, A17S11 Apr 15, 2023
@jahorton
Copy link
Contributor

An idea toward replacing / reworking the functionality, longterm: #2159 (comment)

@mcdurdin mcdurdin modified the milestones: A17S11, A17S12 Apr 29, 2023
@mcdurdin mcdurdin modified the milestones: A17S12, A17S13 May 14, 2023
@mcdurdin mcdurdin modified the milestones: A17S13, A17S14 May 28, 2023
@mcdurdin mcdurdin modified the milestones: A17S14, A17S15 Jun 10, 2023
@mcdurdin mcdurdin modified the milestones: A17S15, A17S16 Jun 26, 2023
@mcdurdin mcdurdin modified the milestones: A17S16, A17S17 Jul 10, 2023
@mcdurdin mcdurdin modified the milestones: A17S17, A17S18 Jul 24, 2023
@mcdurdin mcdurdin modified the milestones: A17S18, A17S19 Aug 6, 2023
@darcywong00
Copy link
Contributor Author

From A17S20 planning meeting, we'll go ahead and close this.
Can re-implement if Google Play Store flags a future build.

@darcywong00 darcywong00 modified the milestones: A17S19, A17S20 Aug 18, 2023
@darcywong00 darcywong00 deleted the fix/android/disable-km-browser branch August 18, 2023 04:28
@darcywong00
Copy link
Contributor Author

and also replace with https://github.com/keymanapp/keyman-browser

@darcywong00
Copy link
Contributor Author

We may need to re-implement this as Play Store has been blocking stable builds since
Jan 2023 (last version 16.0.138)

darcywong00 added a commit that referenced this pull request Feb 8, 2024
darcywong00 added a commit that referenced this pull request Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants