Add support for FedCM commands #12096
Conversation
|
|
Codecov ReportPatch and project coverage have no change.
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## trunk #12096 +/- ##
=======================================
Coverage 57.40% 57.40%
=======================================
Files 86 86
Lines 5369 5369
Branches 206 206
=======================================
Hits 3082 3082
Misses 2081 2081
Partials 206 206 ☔ View full report in Codecov by Sentry. |
cbiesinger
force-pushed
the
fedcm
branch
2 times, most recently
from
a320aa5
to
c9fd32a
Compare
As specified in: https://fedidcg.github.io/FedCM/#automation Currently implemented in Chromium.
|
I believe this is ready for review now! However, the test I've written requires Chrome 115, and I don't know how to handle that? |
|
We would need to make some changes in the test setup to allow beta Chrome to be used for testing. Not sure if we will have time to do that before the actual Chrome 115 is released. Would you be OK if we review this PR when that happens? |
|
Hm, what kind of support would you need? I could perhaps look into doing that work if you outline what is needed. I could also change this CL so that the test is skipped if the command throws a NotImplemented error? Either way -- it seems like the review doesn't have to wait until then, only the merging? I would certainly appreciate getting comments earlier :) |
|
You are right, I will find time in the next couple of days to review this. |
|
@diemol -- fyi, Chrome 115 has now been released (https://chromereleases.googleblog.com/2023/07/stable-channel-update-for-desktop.html), so just updating to the latest stable should allow this PR to be merged |
|
ping? |
|
@cbiesinger do you know why we are getting the following error in these tests now?
CI Link: https://github.com/SeleniumHQ/selenium/actions/runs/8418971824/job/23050594831#step:16:503 |
|
@titusfortner FYI, see last comment |
This lets us re-enable the FedCM tests, which this change does we well. In addition, newer versions of Chrome look for a "login_url" field instead of "signin_url", so this updates the fedcm.json file accordingly. This test failure was pointed out here: SeleniumHQ#12096 (comment)
|
I have created PR #14070 to fix that issue. |
This lets us re-enable the FedCM tests, which this change does we well. In addition, newer versions of Chrome look for a "login_url" field instead of "signin_url", so this updates the fedcm.json file accordingly. This test failure was pointed out here: SeleniumHQ#12096 (comment)
Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it
Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.
Description
This adds the FedCM webdriver commands that are specified in:
https://fedidcg.github.io/FedCM/#automation
Currently implemented in Chromium, but Firefox is interested as well:
w3c-fedid/FedCM#395 (comment)
w3c-fedid/FedCM#465 (comment)
Bug #12088
Motivation and Context
This lets identity providers and relying parties use Selenium's Java API to control
the FedCM browser dialog to test that their website works correctly when
using the FedCM API.
Types of changes
Checklist