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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 1 commit into from
Mar 16, 2023
Merged

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

merged 1 commit into from
Mar 16, 2023

Conversation

darcywong00
Copy link
Contributor

🍒 of #8428 to 17.0 alpha

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

@darcywong00 darcywong00 requested a review from sgschantz as a code owner March 14, 2023 04:56
@darcywong00 darcywong00 added this to the A17S8 milestone Mar 14, 2023
@keymanapp-test-bot keymanapp-test-bot bot added the user-test-required User tests have not been completed label Mar 14, 2023
@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Mar 14, 2023 •
edited
Loading

User Test Results

Test specification and instructions

  • ✅ TEST_BROWSER_DISABLED (PASSED): Retested with the updated PR build (keyman 17.0.67-alpha-test-8430) in Android 12 / API 31 emulator and here is my observation: 1. Verified the browser icon is not visible in both Portrait and Landscape view. (notes)

Test Artifacts

@github-actions github-actions bot added android/ android/app/ cherry-pick Change already merged into another (stable) branch fix labels Mar 14, 2023
@darcywong00
Copy link
Contributor Author

@bharanidharanj - you can use this test link

@darcywong00 darcywong00 mentioned this pull request Mar 14, 2023
Closed
@bharanidharanj
Copy link

@bharanidharanj - you can use this test link

@darcywong00 Okay, Thanks.

@bharanidharanj
Copy link

Test Results

  • TEST_BROWSER_DISABLED (FAILED): Tested with the PR build (Keyman 17.0.67-alpha-test-8430) in my Android mobile (Redmi 8 Note Pro (11.0 Ver)) and it throws an error message during installation.

@keymanapp-test-bot keymanapp-test-bot bot added user-test-failed and removed user-test-required User tests have not been completed labels Mar 15, 2023
@darcywong00
Copy link
Contributor Author

Ah, the build fail should be fixed in #8442. Will retest after that gets merged

@darcywong00
Copy link
Contributor Author

darcywong00 commented Mar 16, 2023 •
edited
Loading

Added configure:engine to CI step which should fix the test build

./build.sh configure:engine build:engine --ci

@keymanapp-test-bot retest

@keymanapp-test-bot keymanapp-test-bot bot added user-test-required User tests have not been completed and removed user-test-failed labels Mar 16, 2023
@bharanidharanj
Copy link

bharanidharanj commented Mar 16, 2023 •
edited
Loading

Test Results

  • TEST_BROWSER_DISABLED (PASSED): Retested with the updated PR build (keyman 17.0.67-alpha-test-8430) in Android 12 / API 31 emulator and here is my observation: 1. Verified the browser icon is not visible in both Portrait and Landscape view.

..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 16, 2023
@darcywong00 darcywong00 merged commit b3cff7e into master Mar 16, 2023
@darcywong00 darcywong00 deleted the cherrypick/android/disable-browser branch March 16, 2023 07:04
@keyman-server
Copy link
Collaborator

Changes in this pull request will be available for download in Keyman version 17.0.69-alpha

@darcywong00 darcywong00 mentioned this pull request Feb 12, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jahorton jahorton jahorton approved these changes

@sgschantz sgschantz Awaiting requested review from sgschantz sgschantz is a code owner

Assignees
No one assigned
Labels
android/app/ android/ cherry-pick Change already merged into another (stable) branch fix has-user-test
Projects
None yet
Milestone
A17S8
Development

Successfully merging this pull request may close these issues.
4 participants
@darcywong00 @bharanidharanj @keyman-server @jahorton