Set Sec-Fetch-Dest to email-verification for those requests [chromium/src : main]
Nicolás Peña (Gerrit)
Nicolás Peña voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
- Christian Biesinger
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Christian Biesinger (Gerrit)
Christian Biesinger added 7 comments![]( "Open in Gerrit")
You should probably add email verification to https://source.chromium.org/chromium/chromium/src/+/main:services/network/cors/cors_url_loader_factory.cc;l=767 as well
Set Sec-Fetch-Dest to email-verification for those requestsMaybe add a link to https://github.com/WICG/email-verification-protocol in the description
network::mojom::RequestDestination destination,why this change? isn't this always WebIdentity?
network::mojom::RequestDestination::kWebIdentity,I almost wonder if this should be a constructor argument, since the IDP subclass always uses webidentity and the email verification subclass alwas uses email verification
TEST_F(IdpNetworkRequestManagerTest, FetchWellKnownRequestDestination) {guess these can't hurt but fwiw we do check this in wpt as well (https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/external/wpt/fedcm/support/request-params-check.py;l=1?q=commonchec%20fedcm&ss=chromium)
constexpr char kEmailVerification[] = "email-verification";No other destination uses a dash, I don't think this one should be the exception
"email-verification",Weird, not sure why https://crrev.com/c/5925915 added webidentity here, the renderer should never see it.
Anyway, this should probably not be under the FedCM heading, maybe link to https://github.com/WICG/email-verification-protocol ?
Related details
- Nicolás Peña
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nicolás Peña (Gerrit)
Nicolás Peña voted and added 7 comments![]( "Open in Gerrit")
Votes added by Nicolás Peña
| Commit-Queue | +1 |
You should probably add email verification to https://source.chromium.org/chromium/chromium/src/+/main:services/network/cors/cors_url_loader_factory.cc;l=767 as well
Done
Set Sec-Fetch-Dest to email-verification for those requestsMaybe add a link to https://github.com/WICG/email-verification-protocol in the description
Done
why this change? isn't this always WebIdentity?
Done
network::mojom::RequestDestination::kWebIdentity,I almost wonder if this should be a constructor argument, since the IDP subclass always uses webidentity and the email verification subclass alwas uses email verification
Oh good idea. Done
TEST_F(IdpNetworkRequestManagerTest, FetchWellKnownRequestDestination) {guess these can't hurt but fwiw we do check this in wpt as well (https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/external/wpt/fedcm/support/request-params-check.py;l=1?q=commonchec%20fedcm&ss=chromium)
Acknowledged
constexpr char kEmailVerification[] = "email-verification";No other destination uses a dash, I don't think this one should be the exception
Hmm maybe it should be `emailverification`?
Weird, not sure why https://crrev.com/c/5925915 added webidentity here, the renderer should never see it.
Anyway, this should probably not be under the FedCM heading, maybe link to https://github.com/WICG/email-verification-protocol ?
I guess we can add notreached for these in the converter since it is only used for the request destination attribute getter which should never return these... Done
Related details
- Christian Biesinger
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |