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

Add autofill endpoints and processing model #1797

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

yoavweiss
Copy link

@yoavweiss yoavweiss commented Feb 23, 2024

This is a rough sketch PR to demonstrate how one can resolve #1796

As this is our first time contributing to WebDriver, feedback on the overall approach, as well as the details is more than welcome! :)

Co-authored by @theindra


Preview | Diff

@sadym-chromium
Copy link
Contributor

Please note that webdriver classic is in maintenance mode, while new features are expected to be added to webdriver bidi.

<ol>
<li><p>If the <a>current top-level browsing context</a> is <a>no longer open</a>,
return <a>error</a> with <a>error code</a> <a>no such window</a>.
<li><p>Let <var>element</var> be the result of <a>trying</a> to
Copy link

Choose a reason for hiding this comment

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

@yoavweiss element should be derived from a specified elementId, right?

Copy link
Author

Choose a reason for hiding this comment

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

Not necessarily. IIUC (and it's highly likely I don't), Web elements can be represented using their node ID.

return <a>error</a> with <a>error code</a> <a>no such window</a>.
<li><p>Let <var>element</var> be the result of <a>trying</a> to
<a>deserialize a web element</a> given request body.
<li><p>The <a>user agent</a> should <a data-cite="HTML#concept-fe-autofill">autofill</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if the page doesn't have a form in it. Currently this looks like it should attempt to do the right thing but the user is never given any feedback.

Copy link
Author

Choose a reason for hiding this comment

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

I can add a check to enforce the existence of a form owner (and throw if there isn't one?)

Copy link
Author

Choose a reason for hiding this comment

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

Ended up returning an error. Please make sure I did that correctly :)

index.html Show resolved Hide resolved
@css-meeting-bot
Copy link
Member

The Browser Testing and Tools Working Group just discussed Autofill Testing.

The full IRC log of that discussion <AutomatedTester_> Topic: Autofill Testing
<AutomatedTester_> github: https://github.com//pull/1797
<AutomatedTester_> https://www.w3.org/wiki/WebDriver/2024-05-BiDi
<AutomatedTester_> Yoav: We've been looking at the autofill space recently at shopify. There are a lot of issues with interop here and and how to ahndle this
<AutomatedTester_> ... it would be great if we could have a way to automate this through testing with webdriver and with WPT
<AutomatedTester_> ... working with Christian we have a test suite for the various issues that we have come against
<AutomatedTester_> ... we have put forward a webdriver PR on how to solve this
<AutomatedTester_> ... sadym has pointed out that we are now working on bidi which we have moved acorss to
<AutomatedTester_> ... and we have a simple API here where we save the data and then we have a way to trigger it
<AutomatedTester_> so... does this solution seem sensible?
<AutomatedTester_> q?
<jgraham> q+
<AutomatedTester_> theindra: I think Yoav has pointed out most of this
<AutomatedTester_> ... in the spec we have spent a lot of time trying to make this work
<AutomatedTester_> ... so for merchants there is a lot of work going into this
<AutomatedTester_> ... so automated tests would be good here
<AutomatedTester_> ack next
<AutomatedTester_> jgraham: I don't know a lot about autofill and I've reached out to mozilla here on how to solve it
<AutomatedTester_> ... so from wpt point of view there is no spec and was browser defined
<whimboo> https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#autofill
<AutomatedTester_> Yoav: there is something in the html spec and unfortunlaty it's not as precise as it should be
<AutomatedTester_> ... there is a browser feature component here ad the UI can be different
<gsnedders> q?
<gsnedders> q+
<AutomatedTester_> ... some of the tests are implementing the current spec but where we want the spec to be
<AutomatedTester_> jgraham: from my point of view... i've seen autofill issues that look like browser issues and people get confused
<AutomatedTester_> ... so agree it's boundray breaking here
<AutomatedTester_> q?
<AutomatedTester_> ack next
<AutomatedTester_> gsnedders: from our side that we, apple, do ignore what the html calls field type
<AutomatedTester_> ... and there are a lot fo sites that try disable autofill
<AutomatedTester_> ... and we use heuristics to figure out what need this. We, apple, have issues with the spec being overly strict in the spec. We don't see the major issue in the webdriver at the moment but we need to look at this closer
<AutomatedTester_> Yoav: so... what you're saying is this has become a little bit of an adversary issue
<AutomatedTester_> .... do you know why sites do this?
<AutomatedTester_> gsnedders: some companies believe this is good security which is not the actual truth for them
<AutomatedTester_> ... I don't know enough of the why but I know it exists
<orkon> q+
<AutomatedTester_> Yoav: I see why uou dont want to solve that as too strict but it would be good to solve this for sites that are not being adversarial.
<AutomatedTester_> theindra: There is a spec for turning off the spec for turning off "disabling"
<AutomatedTester_> gsnedders: yes and we ignore that sometimes as it causes "breakages" which is better for the users not the developer
<AutomatedTester_> Yoav: I see where that is can make sense e.g. passwords being blocked from autofilled
<AutomatedTester_> ... it would be good to solve this for the 80% rather than for the perfect
<AutomatedTester_> gsnedders: where is this implemented in chromium
<AutomatedTester_> ack next
<AutomatedTester_> orkon: We, google, would need to discuss this with the autofill team. we know that this already exists in CDP
<AutomatedTester_> ... the current PR isn't necessarily in line with how browsers are working, like heuristics mentioned by gsnedders
<AutomatedTester_> ... it might be better to fill in the form and then get the browser to save that form and use it in a later tests
<AutomatedTester_> Yoav: is the issue with the rigidity of the "save" API.
<AutomatedTester_> orkon: there are times where we can have multiple credit card fields
<AutomatedTester_> ... and how would we do that in an API version?
<AutomatedTester_> Yoav: I see what you're saying as there could be real world examples with payment forms.
<AutomatedTester_> orkon: yes. there are payment forms across different iframes
<AutomatedTester_> Yoav: this is useful. It would be good if we can have these details in the PR
<AutomatedTester_> q?

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.

Add ability to save autofill data and trigger autofill on form elements
5 participants