[FedCM] Add console warning message to FedCM breaking changes. [chromium/src : main]
0 views
Skip to first unread message
Nicolás Peña (Gerrit)
Oct 10, 2025, 3:04:07 PM (5 days ago) Oct 10
to suresh potti, AyeAye, Chromium LUCI CQ, Christian Biesinger, chromium...@chromium.org, Kaan Icer, blink-...@chromium.org, npm+...@chromium.org, yigu+...@chromium.org
Attention needed from suresh potti
Nicolás Peña added 7 comments![]( "Open in Gerrit")
Commit Message
File content/browser/webid/accounts_fetcher.cc
Line 223, Patchset 2 (Latest):
if (has_client_metadata_endpoint &&Nicolás Peña . unresolved
This is already checked in the other method? I think it should be the same condition, in which case we should merge the if and else:
```
if (!Validate...) {
if (IsWellKnow...) { OnFetchDataforIdPFailed; }
else { AddMessageToConsole;}
}
```
Line 232, Patchset 2 (Latest):
"for privacy and security reasons. This will become a hard "Nicolás Peña . unresolved
security?
Line 233, Patchset 2 (Latest):
"requirement in a future version. Please update your "Nicolás Peña . unresolved
Should we state explicit version?
File content/browser/webid/url_computations.cc
Line 51, Patchset 2 (Latest):
render_frame_host.AddMessageToConsole(Nicolás Peña . unresolved
This should go in the if statement of line 58
File third_party/blink/renderer/modules/credentialmanagement/identity_credential_error.cc
Line 32, Patchset 2 (Latest):
: (options->hasError() ? options->error() : ""))),Nicolás Peña . unresolved
Shouldn't this be behind some flag?
Line 62, Patchset 2 (Latest):
"will be removed in a future version. Use the 'error' attribute "Nicolás Peña . unresolved
ditto version
Related details
Attention is currently required from:
- suresh potti
Submit Requirements:
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. |
suresh potti (Gerrit)
Oct 14, 2025, 1:35:30 AM (yesterday) Oct 14
to Nicolás Peña, AyeAye, Chromium LUCI CQ, Christian Biesinger, chromium...@chromium.org, Kaan Icer, blink-...@chromium.org, npm+...@chromium.org, yigu+...@chromium.org
Attention needed from Nicolás Peña
suresh potti added 6 comments![]( "Open in Gerrit")
Commit Message
File content/browser/webid/accounts_fetcher.cc
This is already checked in the other method? I think it should be the same condition, in which case we should merge the if and else:
```
if (!Validate...) {
if (IsWellKnow...) { OnFetchDataforIdPFailed; }
else { AddMessageToConsole;}
}
```
suresh potti
Done
Line 232, Patchset 2:
"for privacy and security reasons. This will become a hard "Nicolás Peña . resolved
suresh pottisecurity?
Done
Should we state explicit version?
suresh potti
I was bit skeptical to mention Chrome in warning message as it shows up in all chromium based browsers. So didnt mention chrome 145 explicity.
File content/browser/webid/url_computations.cc
This should go in the if statement of line 58
suresh potti
Done
File third_party/blink/renderer/modules/credentialmanagement/identity_credential_error.cc
Line 62, Patchset 2:
"will be removed in a future version. Use the 'error' attribute "Nicolás Peña . resolved
suresh pottiditto version
Done
Related details
Attention is currently required from:
- Nicolás Peña
Submit Requirements:
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)
Oct 14, 2025, 11:02:43 AM (yesterday) Oct 14
to suresh potti, Christian Biesinger, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, Kaan Icer, blink-...@chromium.org, npm+...@chromium.org, yigu+...@chromium.org
Attention needed from Christian Biesinger and suresh potti
Nicolás Peña voted and added 4 comments![]( "Open in Gerrit")
Votes added by Nicolás Peña
| Code-Review | +1 |
4 comments
Commit Message
Nicolás Peña . unresolved
suresh pottiLink the relevant bugs?
Done
Nicolás Peña
Not done?
File content/browser/webid/accounts_fetcher.cc
Line 221, Patchset 7 (Latest):
render_frame_host_->AddMessageToConsole(Nicolás Peña . unresolved
Actually this message might be useful even when the flag is enabled since I notice you are using a more generic failure type above. So perhaps move this to before the if(flag) so this message is always printed when the validation fails?
Line 227, Patchset 7 (Latest):
"for privacy reasons. This will become a hard "Nicolás Peña . unresolved
this formatting seems off.
File third_party/blink/renderer/modules/credentialmanagement/identity_credential_error.cc
Line 60, Patchset 7 (Latest):
"will be removed in Chrome 145. Use the 'error' attribute "Nicolás Peña . unresolved
formatting
Related details
Attention is currently required from:
- Christian Biesinger
- suresh potti
Submit Requirements:
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. |
suresh potti (Gerrit)
Oct 14, 2025, 12:49:17 PM (yesterday) Oct 14
to Nicolás Peña, Christian Biesinger, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, Kaan Icer, blink-...@chromium.org, npm+...@chromium.org, yigu+...@chromium.org
Attention needed from Christian Biesinger
suresh potti voted and added 5 comments![]( "Open in Gerrit")
Votes added by suresh potti
| Commit-Queue | +0 |
5 comments
Commit Message
Nicolás Peña . resolved
suresh pottiLink the relevant bugs?
Nicolás PeñaDone
Not done?
suresh potti
Done
File content/browser/webid/accounts_fetcher.cc
Actually this message might be useful even when the flag is enabled since I notice you are using a more generic failure type above. So perhaps move this to before the if(flag) so this message is always printed when the validation fails?
suresh potti
Done
Line 227, Patchset 7:
"for privacy reasons. This will become a hard "Nicolás Peña . resolved
suresh pottithis formatting seems off.
Done
File third_party/blink/renderer/modules/credentialmanagement/identity_credential_error.cc
Shouldn't this be behind some flag?
suresh potti
removed
Line 60, Patchset 7:
"will be removed in Chrome 145. Use the 'error' attribute "Nicolás Peña . resolved
suresh pottiformatting
Done
Related details
Attention is currently required from:
- Christian Biesinger
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
suresh potti (Gerrit)
Oct 14, 2025, 12:49:27 PM (yesterday) Oct 14
to Nicolás Peña, Christian Biesinger, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, Kaan Icer, blink-...@chromium.org, npm+...@chromium.org, yigu+...@chromium.org
Attention needed from Christian Biesinger
suresh potti voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Christian Biesinger (Gerrit)
Oct 14, 2025, 1:07:51 PM (yesterday) Oct 14
to suresh potti, Christian Biesinger, Nicolás Peña, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, Kaan Icer, blink-...@chromium.org, npm+...@chromium.org, yigu+...@chromium.org
Attention needed from suresh potti
Christian Biesinger voted and added 2 comments![]( "Open in Gerrit")
Votes added by Christian Biesinger
| Code-Review | +1 |
2 comments
Patchset-level comments
File-level comment, Patchset 9 (Latest):
Christian Biesinger . resolved
lgtm. we may want to look into adding these as devtools issues as well?
File third_party/blink/renderer/modules/credentialmanagement/identity_credential_error.cc
Line 56, Patchset 9 (Latest):
execution_context->AddConsoleMessage(MakeGarbageCollected<ConsoleMessage>(Christian Biesinger . unresolved
IMO this is not needed, the exception message has this detail already
Related details
Attention is currently required from:
- suresh potti
Submit Requirements:
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. |
Christian Biesinger (Gerrit)
11:33 AM (10 hours ago) 11:33 AM
to suresh potti, Christian Biesinger, Nicolás Peña, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, Kaan Icer, blink-...@chromium.org, npm+...@chromium.org, yigu+...@chromium.org
Attention needed from suresh potti
Christian Biesinger added 1 comment![]( "Open in Gerrit")
File third_party/blink/renderer/modules/credentialmanagement/identity_credential_error.cc
Line 56, Patchset 9 (Latest):
execution_context->AddConsoleMessage(MakeGarbageCollected<ConsoleMessage>(Christian Biesinger . resolved
IMO this is not needed, the exception message has this detail already
Christian Biesinger
Nevermind, this is needed to show the message when the flag is *not* enabled. (although maybe we only want to show it if the flag is not enabled?)
Related details
Attention is currently required from:
- suresh potti
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
suresh potti (Gerrit)
11:38 AM (10 hours ago) 11:38 AM
to Christian Biesinger, Nicolás Peña, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, Kaan Icer, blink-...@chromium.org, npm+...@chromium.org, yigu+...@chromium.org
suresh potti voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
11:40 AM (10 hours ago) 11:40 AM
to suresh potti, Christian Biesinger, Nicolás Peña, AyeAye, chromium...@chromium.org, Kaan Icer, blink-...@chromium.org, npm+...@chromium.org, yigu+...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[FedCM] Add console warning message to FedCM breaking changes.
Bug: 431249539, 427474985, 438736799
Change-Id: I35c5fe3e66be7319a1fd334766660744df328405
Reviewed-by: Christian Biesinger <cbies...@chromium.org>
Commit-Queue: suresh potti <sures...@microsoft.com>
Reviewed-by: Nicolás Peña <n...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1530237}
Files:
- M content/browser/webid/accounts_fetcher.cc
- M content/browser/webid/request_service_unittest.cc
- M content/browser/webid/url_computations.cc
- M content/browser/webid/webid_browsertest.cc
- M third_party/blink/renderer/modules/credentialmanagement/identity_credential_error.cc
Change size: M
Delta: 5 files changed, 53 insertions(+), 7 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Christian Biesinger, +1 by Nicolás Peña
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages