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

Inline autofill support #336

Merged
merged 17 commits into from
Dec 29, 2023
Merged

Inline autofill support #336

merged 17 commits into from
Dec 29, 2023

Conversation

arcarum
Copy link
Contributor

@arcarum arcarum commented Dec 18, 2023

This PR aims to add support for inline autofill.

The code is complete and works from my testing. There is only one minor issue that I could not find a fix for and that is the inline autofill overlapping with the toolbar icon when scrolling.

I had to modify the manifest to change "method_dummy" to "method" for it to work with the debug app (I am not sure if method_dummy is even needed so if it is unused then it could be deleted).

I am also sorry for making a new PR, but I did not know how to add to the old one.

@Helium314
Copy link
Owner

I had to modify the manifest to change "method_dummy" to "method"

The method_dummy file contains some text why it is used. It's been a while since I tested this, but I remember I sometimes had weird issues/inconsistencies when exposing all the subtypes to the system.

Doesn't it work when you add the changes to method_dummy? That's the one the system should see.

@Helium314
Copy link
Owner

There is only one minor issue that I could not find a fix for and that is the inline autofill overlapping with the toolbar icon when scrolling.

Is the toolbar needed during autofill? If not the icon could be hidden.

@Helium314
Copy link
Owner

Do the suggestions fit into the normal style? I see you set hardcoded text colors, which might look bad depending on the user's chosen colors.

@arcarum
Copy link
Contributor Author

arcarum commented Dec 19, 2023

Adding the changes to method_dummy makes it work, I just thought it was redundant to have two methods.

The toolbar is not needed during autofill, but wouldn't hiding it require making it visible again when it is used? I think it would be better if there is a solution that does not need modifying other parts of the code.

The suggestions are black text on a white background (pill shaped) so it should be look fine (or at least visible) on all colors. I believe the default autofill service is also just black text on a white background. The text color can be changed to R.attr.suggestionWordStyle but then the background will need to be changed to something suitable.

@Helium314
Copy link
Owner

The toolbar is not needed during autofill, but wouldn't hiding it require making it visible again when it is used? I think it would be better if there is a solution that does not need modifying other parts of the code.

I think if it's just setting the key to visible again on clear would be fine.
Do the suggstions also overlap with pinned keys, or only with the toolbar key?

The suggestions are black text on a white background (pill shaped) so it should be look fine (or at least visible) on all colors. I believe the default autofill service is also just black text on a white background. The text color can be changed to R.attr.suggestionWordStyle but then the background will need to be changed to something suitable.

I'm pretty sure that the first issue about this autofill implementation will be lack of theming support

@arcarum
Copy link
Contributor Author

arcarum commented Dec 19, 2023

I do not think theming support is something that will be easy to implement since the background of the suggestions uses a drawable.

What are pinned keys? I don't see anything in the suggestion bar other than the toolbar icon, but the suggestions start from after the toolbar icon and end outside the screen, when scrolling, the left suggestion will overlap the toolbar icon and go outside the screen. If there is anything in the suggestion bar then it will be underneath the suggestions.

@Helium314
Copy link
Owner

What are pinned keys?

When you long-press a key in the tool bar, the key will appear on the right edge of the suggestion strip.

I do not think theming support is something that will be easy to implement since the background of the suggestions uses a drawable.

You can simply set color filters or state lists, that's how it's done for other drawables, e.g. here: https://github.com/Helium314/openboard/blob/new/app/src/main/java/org/dslul/openboard/inputmethod/keyboard/KeyboardView.java#L634

@arcarum
Copy link
Contributor Author

arcarum commented Dec 19, 2023

While implementing the hide keys, I found an even worse bug. The onCreateInlineSuggestionsRequest() and onInlineSuggestionsResponse() functions get called on every text field. They only show suggestions on fields that actually need it but it causes issues when hiding the keys. I have no idea what is causing it and I could not narrow down the issue so I will be putting it on hold for now. You can close this PR.

@Helium314
Copy link
Owner

Can't you just return false when the suggestion list in the response is empty?

@arcarum
Copy link
Contributor Author

arcarum commented Dec 19, 2023

That is probably the best solution, it seems like in the sample code provided by Google, there is a check for start input and if the response is empty. I can only guess that these functions being called on all text fields is intentional. I have implemented the hide keys functionality and unhide them on clear(). I think theming is the only thing left.

@BlackyHawky
Copy link
Contributor

Great job! Thank you!!

I just have 2 questions:

  • is it normal that the username and password don't appear in the suggestion strip?
    I use KeePassDX and when I select the entry nothing appears in the suggestion strip.

  • is it possible to disable the suggestion strip gestures when auto-fill is visible?

Watch video
SVID_20231219_223258_1.mp4

@arcarum
Copy link
Contributor Author

arcarum commented Dec 20, 2023

Only the site name and username are visible in the suggestion strip, but clicking it should autofill the username and password properly. I use Bitwarden and it is working for me so I am not sure what is going on. Also the username is not visible in your case probably because the site name is long? I can try making the suggestion width longer if it needs it.

Should be possible, I think it can be disabled along side the keys and re enabled with them. Do you know the setting name?

@arcarum
Copy link
Contributor Author

arcarum commented Dec 20, 2023

Can you check if the autofill displays now and autofills properly? I did not changed the width because I tried it with keePassDX and saw the "..." is just how it displays the password manager entry.

@arcarum
Copy link
Contributor Author

arcarum commented Dec 20, 2023

keePassDX also only shows the username in the suggestions. Bitwarden shows the site name and username.

@arcarum
Copy link
Contributor Author

arcarum commented Dec 20, 2023

@Helium314 Are color filters set on a drawable object? The background of the suggestions is created using Icon.createWithResource(), which takes in the context and the ID of the drawable resource. I can't seem to figure out how to set the color filter because the ID is what is used to determine the drawable.

@Helium314
Copy link
Owner

Icon is a weird thing, it doesn't allow using a Drawable or accessing the internal Drawable. What's your reason for using Icon?
Anyway, looks like you can use setTint, which might be enough.

@arcarum
Copy link
Contributor Author

arcarum commented Dec 20, 2023

.setBackground() takes Icon, I will try setTint.

@arcarum
Copy link
Contributor Author

arcarum commented Dec 20, 2023

I have added theme support, I believe that is all for this PR.

@BlackyHawky
Copy link
Contributor

I have added theme support, I believe that is all for this PR.

Sorry for the late reply. (I have a lot of work at my "real" job...)

After testing, I can confirm that it now works properly. 👍💪


Just a small suggestion:

  • regarding the background colors of the chips, perhaps it would be better to choose the adjustedBackground color instead of background; this would make it possible to see the outlines of the chips. What do you think?

If you resolve the merge conflicts, the code will be very simple to write:

  • In Colors.kt, in the DynamicColors and DefaultColors classes :
override fun get(color: ColorType): Int = when (color) {
…
    CHIP, EMOJI_CATEGORY_BACKGROUND, … -> adjustedBackground
…
}

enum class ColorType {
    …
    BACKGROUND,
    CHIP,
    CLEAR_CLIPBOARD_HISTORY_KEY,
    …
  • In LatinIME.java:
.setSingleIconChipStyle(
    new ViewStyle.Builder()
        .setBackground(
            Icon.createWithResource(mDisplayContext,
                androidx.autofill.R.drawable.autofill_inline_suggestion_chip_background)
                    .setTint(themeColors.get(ColorType.CHIP)))

@arcarum
Copy link
Contributor Author

arcarum commented Dec 25, 2023

@Helium314 is there a reason to hide the entire suggestion strip for password fields?

@popogomo
Copy link

1Password is paid so I can't test it, but if the inline autofill works on other keyboards then it should work here as well.

I created a trial throwaway 1Password account for testing this. If you have an email address that you are happy to provide here, I can send you an invite from within 1Password to access the vault. Otherwise, I can post the 1Password logon details here. Its a throwaway account, so whichever you prefer (and if you don't mind testing 1Password, please).

@Helium314
Copy link
Owner

I'm not even sure whether suggestion strip can still be hidden in the current version.

@arcarum
Copy link
Contributor Author

arcarum commented Dec 25, 2023

@popogomo Thank you for the offer, but I do not want to log in with someone else's account.

@arcarum
Copy link
Contributor Author

arcarum commented Dec 25, 2023

I'm not even sure whether suggestion strip can still be hidden in the current version.

I tested it with the latest version (or about 2 commits behind the latest version) and it appears to be inconsistent. The suggestion strip for the password field sometimes appears and sometimes it does not. When it appears, the inline autofill works correctly.

@Helium314
Copy link
Owner

So the remaining issues are coloring (will be a new PR), and the sometimes missing suggestion strip, or is there anything else?
I can have a look at the suggestion strip later, it doesn't need to be part of this PR.

@arcarum
Copy link
Contributor Author

arcarum commented Dec 25, 2023

I think that is all the remaining issues.

@popogomo
Copy link

@popogomo Thank you for the offer, but I do not want to log in with someone else's account.

Sure, that's totally understandable.

Maybe you could share the debug apk with these changes so I could test 1Password and report back? Thanks.

@arcarum
Copy link
Contributor Author

arcarum commented Dec 26, 2023

You can try this.

openboard_1.4.5-debug.zip

@popogomo
Copy link

popogomo commented Dec 26, 2023

You can try this.

openboard_1.4.5-debug.zip

Thanks for this.

The results of the test (GrapheneOS, Android 14):

  1. Brave - the inline is not showing on any website
  2. Vanadium - the inline does show but doesn't offer the corresponding credentials, so I have to tap to inline, then use my biometrics to access 1Password and then find the corresponding account from the 1Password app and tap to it to fill in the details
  3. Firefox - the same as Vanadium
  4. Apps - the same as Vanadium

@arcarum
Copy link
Contributor Author

arcarum commented Dec 26, 2023

Thank you for testing it. Does the inline autofill work for 1Password with other keyboards?

@arcarum
Copy link
Contributor Author

arcarum commented Dec 26, 2023

I think this should fix it, can you try the apk below?

openboard_1.4.5-debug.zip

@popogomo
Copy link

popogomo commented Dec 26, 2023

I think this should fix it, can you try the apk below?

openboard_1.4.5-debug.zip

Thanks, we have some good progress here.

So:

  1. Firefox - works perfectly on all websites
  2. Apps - Works perfectly so far
  3. Brave - unfortunately, still no inline at all on all websites
  4. Vanadium - a strange behaviour: it doesn't show any suggestions, just the inline where I need to tap to select the credentials for the website in question. However, once I select the credentials for that website, it will show this same suggestion on all other websites, even though these credentials are not relevant to all the other websites. Hope this makes sense.

Shall I try GBoard instead and report back?

@arcarum
Copy link
Contributor Author

arcarum commented Dec 26, 2023

Yeah please try it with Gboard.

@popogomo
Copy link

Yeah please try it with Gboard.

I've tried both GBoard and Microsoft Keyboard with the same results (in fact, the latest debug version you shared worked even better than these!). After digging a bit, it seems the issue is with Brave (and Chromium in general) itself. So I assume we can't do much with this unless Brave will provide a full support.

Did you try Keepass and Bitwarden in Brave by a chance? Are they working as expected?

@arcarum
Copy link
Contributor Author

arcarum commented Dec 26, 2023

I tried it with bitwarden in brave and it works fine. I don't really know what is the issue with 1Password but if it is behaving the same with Gboard then I don't think much can be done about it.

@popogomo
Copy link

I tried it with bitwarden in brave and it works fine. I don't really know what is the issue with 1Password but if it is behaving the same with Gboard then I don't think much can be done about it.

Agree - the latest debug one is good enough to use with other browsers/apps, much better than the first debug. Thanks a million for working on this!

@Helium314 Helium314 merged commit ab183c5 into Helium314:new Dec 29, 2023
@arcarum arcarum deleted the inline_autofill branch December 29, 2023 09:58
@Pietro395
Copy link

Tried on GrapheneOS 14, Mull (Firefox) and Proton Pass, seems to work well!

I will report any problems, thank you

@mrx23dot
Copy link

Please also remember every typed email address, or in "frequently typed words",

gBoard always remembered email addresses, it's so annoying to type exact/long email addresses

(maybe cache them from address book?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants