Skip to content

[FedCM] Specify third party iframe display support#774

Merged
npm1 merged 6 commits intow3c-fedid:mainfrom
cbiesinger:iframe
Oct 8, 2025
Merged

[FedCM] Specify third party iframe display support#774
npm1 merged 6 commits intow3c-fedid:mainfrom
cbiesinger:iframe

Conversation

@cbiesinger
Copy link
Copy Markdown
Collaborator

@cbiesinger cbiesinger commented Aug 26, 2025

Fixed: #725


Preview | Diff

@cbiesinger cbiesinger requested a review from npm1 August 26, 2025 14:44
@cbiesinger cbiesinger marked this pull request as draft August 26, 2025 14:44
@cbiesinger cbiesinger removed the request for review from npm1 August 26, 2025 14:45
@cbiesinger cbiesinger marked this pull request as ready for review August 26, 2025 19:01
@cbiesinger cbiesinger requested a review from npm1 August 26, 2025 19:01
@cbiesinger
Copy link
Copy Markdown
Collaborator Author

I think this is ready for review now

Comment thread spec/index.bs Outdated
Comment on lines +1631 to +1634
1. If |metadata| is not null or failure, and |metadata|.
{{IdentityProviderClientMetadata/client_matches_top_frame_origin}} is `false`, this dialog
MUST show the [=host/registrable domain=] of |globalObject|'s [=associated Document=]'s
[=Document/origin=] to the user.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Eliminating the space that will otherwise appear between |metadata|. and {{

Suggested change
1. If |metadata| is not null or failure, and |metadata|.
{{IdentityProviderClientMetadata/client_matches_top_frame_origin}} is `false`, this dialog
MUST show the [=host/registrable domain=] of |globalObject|'s [=associated Document=]'s
[=Document/origin=] to the user.
1. If |metadata| is not null or failure, and
|metadata|.{{IdentityProviderClientMetadata/client_matches_top_frame_origin}}
is `false`, this dialog
MUST show the [=host/registrable domain=] of |globalObject|'s
[=associated Document=]'s [=Document/origin=] to the user.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, commited a variant of that.

Comment thread spec/index.bs
Comment thread spec/index.bs Outdated
@cbiesinger
Copy link
Copy Markdown
Collaborator Author

@pottis could you review this PR as well?

Comment thread spec/index.bs Outdated
Comment thread spec/index.bs
Comment thread spec/index.bs Outdated
|provider|'s {{IdentityProviderConfig/configURL}}.
1. Set |permission| to the result of running the [=request permission to
sign-up=] algorithm with |selectedAccount|, the relevant |provider|,
|clientMetadataMap|[|providerOrigin|], and |globalObject|.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The map was set with configURL as key and now it is using origin? Which one is it

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

?
it was set as (line 1091):

            |rpOrigin|, set |clientMetadataMap|[|providerOrigin|] to the result of running [=fetch the client metadata=] with

But I actually changed it to configURL because that is a bit simpler.

Comment thread spec/index.bs
@@ -1080,6 +1081,19 @@ When asked to <dfn for="create identity credential">fetch accounts</dfn> given a
<div class="issue" heading="extension">
An extension may use the following instead of the [=create identity credential/show accounts=] step, where
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Maybe wrap these in div algorithm to get things like variable highlighting?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done although that wasn't new. Fixed some bikeshed warnings that were triggered by this.

@npm1 npm1 merged commit 3d1f48b into w3c-fedid:main Oct 8, 2025
2 checks passed
github-actions Bot added a commit that referenced this pull request Oct 8, 2025
SHA: 3d1f48b
Reason: push, by npm1

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@cbiesinger cbiesinger deleted the iframe branch October 8, 2025 14:36
github-actions Bot added a commit to mattdanielbrown/WebID that referenced this pull request Oct 8, 2025
SHA: 3d1f48b
Reason: push, by pull[bot]

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

Support showing iframe origins in the UI, when they are third-party

4 participants