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

Android: enable iframe support #536

Merged
merged 20 commits into from
Apr 9, 2024
Merged

Android: enable iframe support #536

merged 20 commits into from
Apr 9, 2024

Conversation

GioSensation
Copy link
Member

@GioSensation GioSensation commented Mar 24, 2024 •
edited
Loading

Reviewer: @shakyShane
Asana: https://app.asana.com/0/0/1206521793046282/f

Description

Enables iframe autofill on Android by migrating to the new webview API.

Steps to test

All integration tests updated to match the new messaging pattern. Plus tested extensively by Craig on Android and by me on other platforms.

@GioSensation
WIP
b66ca23 
Signed-off-by: Emanuele Feliziani <[email protected]>
@GioSensation
Progress for in-context signup, still WIP
41a2772 
Signed-off-by: Emanuele Feliziani <[email protected]>
@GioSensation
WIP Email Protection stuff
3ac8e2e 
Signed-off-by: Emanuele Feliziani <[email protected]>
@GioSensation
Wire Email Protection into Android transport
156538a 
Signed-off-by: Emanuele Feliziani <[email protected]>
@GioSensation
Improve getAlias API
d4c15b8 
Signed-off-by: Emanuele Feliziani <[email protected]>
@GioSensation
Email Protection progress
74f40df 
Signed-off-by: Emanuele Feliziani <[email protected]>
@GioSensation
Just add builds from a WIP
032e5b6 
Signed-off-by: Emanuele Feliziani <[email protected]>
@GioSensation
WIP build to fix multiple calls
371a33e 
Signed-off-by: Emanuele Feliziani <[email protected]>
@GioSensation
Move logic to messaging.js
f727e02 
Signed-off-by: Emanuele Feliziani <[email protected]>
@GioSensation
Update integration tests
9524ccd 
Signed-off-by: Emanuele Feliziani <[email protected]>
@GioSensation
Improve logging
c63607f 
Signed-off-by: Emanuele Feliziani <[email protected]>
@GioSensation
Improve playwright test documentation
18bd31d 
Signed-off-by: Emanuele Feliziani <[email protected]>
@GioSensation
Commit compiled files
a7db4e9 
Signed-off-by: Emanuele Feliziani <[email protected]>
@GioSensation
Update docs
6c6d6db 
Signed-off-by: Emanuele Feliziani <[email protected]>
@GioSensation GioSensation self-assigned this Mar 24, 2024
@GioSensation
Improve docs
7e3c35f 
Signed-off-by: Emanuele Feliziani <[email protected]>
@GioSensation
Update getAlias to not call if in-context failed
d706ad4 
Signed-off-by: Emanuele Feliziani <[email protected]>
@GioSensation
More comments
b9a239a 
Signed-off-by: Emanuele Feliziani <[email protected]>
@GioSensation GioSensation requested a review from shakyShane March 25, 2024 10:57
@GioSensation GioSensation marked this pull request as ready for review March 25, 2024 10:57
shakyShane
shakyShane requested changes Mar 25, 2024
View reviewed changes
Copy link
Collaborator

@shakyShane shakyShane left a comment

Choose a reason for hiding this comment

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

Thanks @GioSensation - I love how we're consolidating more and more APIs/functionality to match across platform.

Just a single question for you to have a look at 👍🏻

src/DeviceInterface/AndroidInterface.js Show resolved Hide resolved
src/DeviceInterface/AndroidInterface.js Outdated Show resolved Hide resolved
packages/messaging/android.js Show resolved Hide resolved
shakyShane
shakyShane reviewed Mar 25, 2024
View reviewed changes
packages/messaging/android.js
if (!(androidSpecificName in window)) {
throw new MissingHandler(`Missing android handler: '${methodName}'`, methodName)
}
return window[androidSpecificName]
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the story around these handlers, do we need to attempt to capture them? or is that out of scope for this work?

I know that previously the exec-time of Android's JS couldn't be guaranteed to be 'first' - can it now?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about that, and even attempted a quick solution, but then I backtracked. It's not strictly in scope, and we should think this through a bit more, especially if we think of that as improving security, because it may not change anything. We would likely still do it for a variety of reasons, but we need a triage etc. I will scope a project to follow up.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice - yeah with a follow-up project in place then we can approve this and move forward 💪🏻

@GioSensation
Explicitly return undefined
6461a12 
Signed-off-by: Emanuele Feliziani <[email protected]>
@GioSensation GioSensation requested a review from shakyShane March 26, 2024 12:02
@GioSensation
Merge branch 'main' of github.com:duckduckgo/duckduckgo-autofill into…
b88d3f6 
… ema/android-iframe-support

Signed-off-by: Emanuele Feliziani <[email protected]>

# Conflicts:
#	dist/autofill-debug.js
#	dist/autofill.js
#	swift-package/Resources/assets/autofill-debug.js
#	swift-package/Resources/assets/autofill.js
@GioSensation
Ensure password gen feature flag is respected
4c16e52 
Signed-off-by: Emanuele Feliziani <[email protected]>
@GioSensation GioSensation merged commit 8b046b1 into main Apr 9, 2024
1 check passed
This was referenced Apr 9, 2024
Closed
Closed
Closed
Closed
Merged
Merged
Closed
Merged
Merged
Merged
sjbarag added a commit that referenced this pull request Jun 20, 2024
@sjbarag
Revert "Android: enable iframe support (#536)"
0ff72f6 
This reverts commit 8b046b1, disabling
Android iframe support due to a bug in the native WebView. See
https://app.asana.com/0/1203822806345703/1207304124810216/f for details.
sjbarag added a commit that referenced this pull request Jun 20, 2024
@sjbarag
android: revert "Android: enable iframe support (#536)"
573731e 
This reverts commit 8b046b1, disabling
Android iframe support due to a bug in the native WebView. See
https://app.asana.com/0/1203822806345703/1207304124810216/f for details.
sjbarag added a commit that referenced this pull request Jun 20, 2024
@sjbarag
android: revert "Android: enable iframe support (#536)" (#582)
18dd599 
This reverts commit 8b046b1, disabling
Android iframe support due to a bug in the native WebView. See
https://app.asana.com/0/1203822806345703/1207304124810216/f for details.
sjbarag added a commit that referenced this pull request Jun 21, 2024
@sjbarag
generation: ensure password gen feature flag is respected
7620d46 
This was originally introduced as a commit squashed into 8b046b1
(Android: enable iframe support (#536), 2024-04-09), but was accidentally
removed via 18dd599 (android: revert "Android: enable iframe support
(#536)" (#582), 2024-06-20). Reintroduce that commit, to ensure the
`password_generation` runtime config flag is again respected and prevent
password prompts after a user clicks "Never save for site".

(cherry picked from commit 4c16e52)
@sjbarag sjbarag mentioned this pull request Jun 21, 2024
Merged
sjbarag added a commit that referenced this pull request Jun 21, 2024
@sjbarag
generation: ensure password gen feature flag is respected (#583)
2b81745 
This was originally introduced as a commit squashed into 8b046b1
(Android: enable iframe support (#536), 2024-04-09), but was accidentally
removed via 18dd599 (android: revert "Android: enable iframe support
(#536)" (#582), 2024-06-20). Reintroduce that commit, to ensure the
`password_generation` runtime config flag is again respected and prevent
password prompts after a user clicks "Never save for site".

(cherry picked from commit 4c16e52)
@dbajpeyi dbajpeyi mentioned this pull request Sep 16, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shakyShane shakyShane shakyShane approved these changes

Assignees

@GioSensation GioSensation

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@GioSensation @shakyShane