Skip to content
This repository has been archived by the owner on Oct 15, 2024. It is now read-only.

Work around Chrome Autofill issue #921

Merged
merged 7 commits into from
Jul 7, 2020
Merged

Conversation

fmeum
Copy link
Member

@fmeum fmeum commented Jul 6, 2020

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates

📜 Description

Work around Chrome issue 2 described below by adding an AccessibilityService that listens to window changes in Chrome and forces the accessibility virtual view structure to be populated.

💡 Motivation and Context

I have identified at least two distinct issues with Autofill in Chrome:

  1. Due to an Android bug introduced with Android 10, Autofill breaks if a view that is labeled not important for Autofill somehow (this part appears to be non-deterministic and vary between phones) triggers an Autofill request. I don't think that we can work around it, but there is the option for a simple workaround on the Chrome side that I will lobby for. You can try to reproduce this bug with this reproducer APK.

  2. Chrome takes a lazy approach to populating sccessibility info as documented here, which apparently leads to accessibility events not being propagated to the Autofill accessibility compatibility bridge. You can reproduce this in Chrome by browsing to https://github.com/login in two different ways from the new tab page: either tap a shortcut or enter the URL in the omnibox. The former should lead to Autofill being triggered on the login page, the latter not (or at least not always).

Might fix #747.

💚 How did you test it?

Autofill has become much more reliable with the accessibility service enabled.

📝 Checklist

  • I formatted the code with the IDE's reformat action (Ctrl + Shift + L/Cmd + Shift + L)
  • I reviewed submitted code
  • I added a CHANGELOG entry if applicable

🔮 Next steps

We should enable this service and see whether it improves Autofill (or degrades Chrome performance) on as many devices running Android 9+ as possible.

📸 Screenshots / GIFs

@fmeum fmeum added the bug label Jul 6, 2020
@fmeum fmeum requested review from Skrilltrax and msfjarvis July 6, 2020 13:23
@fmeum fmeum force-pushed the bug/chrome_autofill_fix branch from e492650 to b6ccab3 Compare July 6, 2020 13:27
@msfjarvis
Copy link
Member

I don't suppose this is going to make any difference for the first case?

@fmeum
Copy link
Member Author

fmeum commented Jul 6, 2020

I don't suppose this is going to make any difference for the first case?

I don't think so, the bug triggers well before our Autofill service is invoked and has nothing to do with the accessibility virtual view structure. This has to be worked around in Chrome, unfortunately.

@fmeum
Copy link
Member Author

fmeum commented Jul 7, 2020

I pushed a new version that debounces view node population on page change events and only populates nodes synchronously on events triggered by users. Whereas Chrome's performance degraded dramatically previously, with this change I am not noticing a difference anymore.

@fmeum
Copy link
Member Author

fmeum commented Jul 7, 2020

I filed the first issue as an Android bug: https://issuetracker.google.com/issues/160665161
If we are extremely lucky, this might get fixed during the Android 11 Beta, but I doubt it.

msfjarvis
msfjarvis previously approved these changes Jul 7, 2020
Copy link
Member

@msfjarvis msfjarvis left a comment

Choose a reason for hiding this comment

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

Seems to make things better. Gonna need at least a changelog entry and probably an onboarding flow.

@fmeum
Copy link
Member Author

fmeum commented Jul 7, 2020

This has now been integrated into the settings but can also be enabled manually if the Autofill service is already enabled. I hope that this covers all possible states, but the logic in UserPreferences is necessarily rather complex.

msfjarvis
msfjarvis previously approved these changes Jul 7, 2020
@fmeum
Copy link
Member Author

fmeum commented Jul 7, 2020

Forgot to commit the reformat result.

msfjarvis
msfjarvis previously approved these changes Jul 7, 2020
@fmeum fmeum closed this Jul 7, 2020
@fmeum fmeum reopened this Jul 7, 2020
@msfjarvis msfjarvis closed this Jul 7, 2020
@msfjarvis msfjarvis reopened this Jul 7, 2020
@fmeum fmeum closed this Jul 7, 2020
@fmeum fmeum reopened this Jul 7, 2020
@msfjarvis
Copy link
Member

Does Actions need a virgin sacrifice to allocate resources for our builds

@msfjarvis msfjarvis merged commit d192ab2 into develop Jul 7, 2020
@msfjarvis msfjarvis deleted the bug/chrome_autofill_fix branch July 7, 2020 15:03
@fmeum fmeum mentioned this pull request Jul 9, 2020
fmeum added a commit that referenced this pull request Jul 14, 2020
This reverts commit d192ab2.

An official fix has passed code review. We should just wait for it
instead of introducing a workaround that will only be relevant for a
month or two.
msfjarvis added a commit to fmeum/Android-Password-Store that referenced this pull request Jul 14, 2020
* develop: (77 commits)
  Add debug icon and update color palette (android-password-store#931)
  Revert "Work around Chrome Autofill issue (android-password-store#921)" (android-password-store#933)
  github: remove freeDebug variant from pull request matrix (android-password-store#932)
  Properly guard against invalid renaming (android-password-store#929)
  Fix navigation bar theming and reformat (android-password-store#930)
  Exclude third_party scope from reformats (android-password-store#927)
  Move password export to the IO dispatcher (android-password-store#918)
  Mention android-password-store#482 being fixed in the changelog (android-password-store#925)
  global: set an import order rule and reformat with it (android-password-store#924)
  styles: re-add alertDialogTheme override (android-password-store#923)
  Work around Chrome Autofill issue (android-password-store#921)
  Major UI overhaul and the introduction of a new icon (android-password-store#920)
  Update Public Suffix List data (android-password-store#917)
  Migrate to ActivityResultContracts (android-password-store#910)
  release: script improvements (android-password-store#915)
  Deploy both variants to snapshot directory (android-password-store#914)
  Fill OTP fields with SMS codes (android-password-store#900)
  Fix up URIish instances with @ in user name (android-password-store#913)
  build: upgrade Gradle wrapper (android-password-store#911)
  Scroll to files and enter folders when created (android-password-store#909)
  ...

Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Autofill not working with Chromium based browsers
2 participants