Specify "Use another account". #678

cbiesinger · 2024-11-04T22:05:27Z

This depends on PR #660 (Mode API).

Bug: w3c-fedid/active-mode#3

Preview | Diff

This depends on PR w3c-fedid#660 (Mode API). Bug: w3c-fedid/active-mode#3

spec/index.bs

npm1

In the case the user is signed in to just one account, it does not appear to show the use other account feature. So the logic needs to change to account for that

spec/index.bs

cbiesinger · 2024-11-07T20:30:57Z

In the case the user is signed in to just one account, it does not appear to show the use other account feature. So the logic needs to change to account for that

I updated the PR to add the additional logic where required, PTAL

spec/index.bs

npm1

lgtm with nit

spec/index.bs

TallTed · 2024-11-08T19:59:36Z

spec/index.bs

+To <dfn>request permission to sign-up</dfn> the user with a given an {{IdentityProviderAccount}}
+|account|, some {{IdentityProviderModeSettings}} |modeSettings|, an {{IdentityProviderAPIConfig}}
+|config|, an {{IdentityProviderRequestOptions}} |provider|, and a |globalObject|, run the following
+steps. This returns a boolean.
    1. Assert: These steps are running [=in parallel=].


The parallel steps would be better presented as bullets (which might bullets within a numbered step), rather than as numbered steps. As this stands, I am unsure exactly which steps are running in parallel.

This would not be correct. Please check the definition of "in parallel": https://html.spec.whatwg.org/multipage/infrastructure.html#in-parallel

The definition of "in parallel" is not sufficient for me (and I have some familiarity with the subject) to know whether the numbered steps (1)-(20) of which this Assert is (1) are to run in parallel with each other (which appears possible, and indeed, the most likely meaning) or that this sequence of 20 steps is to occur in parallel with other operations (sequences, single steps, etc.) in the spec.

Presuming that one goal of writing this specification is to have it be comprehensible by readers who are new to the subject, I suggest that some rewording would be helpful.

A PR to the HTML spec is welcome, but I think it's pretty clear personally:

means those steps are to be run, one after another

"In parallel" does not mean "in sequence", no matter what spec is written as if it does.

Also, I highly doubt that most readers of this spec will go to the HTML spec, so while I may well submit a PR there (if I can find where to do so; it wasn't obvious when I looked for it yesterday), I think it better to provide more clarity in the FedCM spec.

For better or worse, "in parallel" is an existing term in specland. Ted, do you have a specific suggestion for how to improve this?

In general we should assume that readers either know what these terms mean or click through to their definition, IMO.

(https://github.com/whatwg/html/blob/main/source is where you would send PRs for the HTML spec)

Unfortunately, GitHub's web interface (my primary tool) can't handle docs the size of that WHATWG source. But, I was able to view it, and noted the "parallel queue".

<p>A <dfn export>parallel queue</dfn> represents a queue of algorithm steps that must be run in series.</p>

I think that's a more apt description of this 20-step sequence. Something like

Suggested change

1. Assert: These steps are running [=in parallel=].

1. Assert: These steps comprise a [=parallel queue=] that should run [=in parallel=] with any other active algorithms and/or [=parallel queues=].

(That does presume that plurals of defined terms are properly handled; the last might need change from [=parallel queues=] to [=parallel queue=].)

I've added a note to whatwg/html#10049, but noting that they have 1923 open issues and 169 pending pull requests, I don't have high hopes of it being addressed in a timely fashion.

@domenic — Your thumb-down emoji doesn't communicate even as well as the "in parallel" that is meant to be understood as some language other than English. If you have an argument against my points, please do me the courtesy of writing it out.

I am at a loss. @domenic, you are not participating usefully. Please refrain from emoting unexplained "thumb down". If you have an argument against what I've said, please put into words, so I and others can consider your point(s) against my own.

spec/index.bs

hlflanagan · 2024-11-26T16:45:00Z

Discussed on https://github.com/fedidcg/meetings/blob/main/2024/2024-11-26-notes.md. Needs further review.

wseltzer · 2025-01-14T16:59:16Z

Discussed at January 14, 2025 meeting minutes

bvandersloot-mozilla · 2025-02-05T19:50:06Z

Why does this behavior need to be gated behind an IDP opt in?

cbiesinger · 2025-02-05T19:55:26Z

Why does this behavior need to be gated behind an IDP opt in?

As mentioned in one of the meetings, some IDPs do not support multiple accounts for one person so this would not be useful for them.

bvandersloot-mozilla · 2025-02-05T20:15:33Z

Follow up: is there a reason it varies between active and passive if it is expected to be per-IDP?

cbiesinger · 2025-03-06T15:59:33Z

We are happy to make this a single toplevel boolean supports_use_other_account. I will update the PR accordingly.

cbiesinger · 2025-03-13T19:17:11Z

@bvandersloot-mozilla @npm1 please take another look, I have updated the PR to use a single toplevel boolean.

spec/index.bs

npm1 · 2025-03-17T21:49:04Z

spec/index.bs

@@ -1426,6 +1446,12 @@ an {{IdentityProviderAPIConfig}} |config|, an {{IdentityProviderRequestOptions}}
            1. The user agent MAY use the
                {{IdentityCredentialRequestOptions/context}} and |provider|'s
                {{IdentityCredentialRequestOptions/mode}} to customize the dialog shown.
+            1. If |supportsUseOtherAccount| is true, the account chooser MUST


I wonder if this should instead be covered by showing an initial step to select an account if this is true, otherwise go direct to request permission?

You're suggesting that we should require showing the account chooser first if "supports use other account" is true, even if there is only one account signed in to the IDP?

Yea... that matches the Chrome implementation (for active mode which the only case we are keeping anyways)

That sounds reasonable to me.

Allows "supports_use_other_account" in the toplevel of the config file. Also renames "labels" in the account endpoint to "label_hints" and flattens "accounts: { include: "foo" }" to just a toplevel "account_label". All changes behind the new "FedCmOtherAccountAndLabelsNewSyntax" flag. Even with the flag enabled, the old syntax is still accepted for now. As per the discussions here: w3c-fedid/FedCM#669 w3c-fedid/FedCM#678 Bug: 404568028 Change-Id: Iba33c5f546c68f35ca2a1228096a7a31a68767a9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6370034 Reviewed-by: Nicolás Peña <[email protected]> Reviewed-by: Alex Moshchuk <[email protected]> Commit-Queue: Christian Biesinger <[email protected]> Cr-Commit-Position: refs/heads/main@{#1435018}

wseltzer · 2025-03-25T15:57:03Z

Discussed March 25.

Specify "Use another account".

a191a15

This depends on PR w3c-fedid#660 (Mode API). Bug: w3c-fedid/active-mode#3

cbiesinger requested a review from npm1 November 4, 2024 22:05

TallTed suggested changes Nov 5, 2024

View reviewed changes

spec/index.bs Outdated Show resolved Hide resolved

spec/index.bs Outdated Show resolved Hide resolved

spec/index.bs Outdated Show resolved Hide resolved

spec/index.bs Outdated Show resolved Hide resolved

spec/index.bs Outdated Show resolved Hide resolved

npm1 requested changes Nov 5, 2024

View reviewed changes

spec/index.bs Outdated Show resolved Hide resolved

cbiesinger added 2 commits November 7, 2024 15:21

ted changes

3d56908

npm comment

cbd0adf

fix indentation

af610e6

npm1 reviewed Nov 7, 2024

View reviewed changes

spec/index.bs Outdated Show resolved Hide resolved

spec/index.bs Show resolved Hide resolved

spec/index.bs Outdated Show resolved Hide resolved

fix indentation

cd1c73d

npm1 approved these changes Nov 7, 2024

View reviewed changes

spec/index.bs Outdated Show resolved Hide resolved

cbiesinger added 2 commits November 7, 2024 16:43

fix nit

82ea3fa

fix indentation

533de6b

TallTed suggested changes Nov 8, 2024

View reviewed changes

cbiesinger added the agenda+ label Nov 13, 2024

cbiesinger added 2 commits November 22, 2024 18:35

oxford commas

20b53f0

Merge remote-tracking branch 'origin' into otheraccount

c9b78be

TallTed reviewed Nov 25, 2024

View reviewed changes

spec/index.bs Outdated Show resolved Hide resolved

TallTed mentioned this pull request Nov 25, 2024

"in parallel" in algorithm headers can be confusing whatwg/html#10049

Open

npm1 requested a review from bvandersloot-mozilla January 14, 2025 16:37

hlflanagan removed the agenda+ label Feb 11, 2025

cbiesinger added 2 commits March 13, 2025 14:45

Merge remote-tracking branch 'origin/main' into otheraccount

ccf4850

single boolean

b5417b3

npm1 reviewed Mar 17, 2025

View reviewed changes

otherwise

ab1d327

yi-gu added the agenda+ label Mar 19, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Specify "Use another account". #678

Specify "Use another account". #678

cbiesinger commented Nov 4, 2024 •

edited by pr-preview bot

Loading

npm1 left a comment

cbiesinger commented Nov 7, 2024

npm1 left a comment

TallTed Nov 8, 2024

domenic Nov 20, 2024

TallTed Nov 21, 2024

domenic Nov 22, 2024

TallTed Nov 22, 2024

cbiesinger Nov 22, 2024

cbiesinger Nov 22, 2024

TallTed Nov 25, 2024 •

edited

Loading

TallTed Nov 26, 2024

TallTed Feb 5, 2025

hlflanagan commented Nov 26, 2024

wseltzer commented Jan 14, 2025 •

edited

Loading

bvandersloot-mozilla commented Feb 5, 2025

cbiesinger commented Feb 5, 2025

bvandersloot-mozilla commented Feb 5, 2025

cbiesinger commented Mar 6, 2025

cbiesinger commented Mar 13, 2025

npm1 Mar 17, 2025

cbiesinger Mar 19, 2025

npm1 Mar 20, 2025

bvandersloot-mozilla Mar 25, 2025

wseltzer commented Mar 25, 2025

	1. Assert: These steps are running [=in parallel=].
	1. Assert: These steps comprise a [=parallel queue=] that should run [=in parallel=] with any other active algorithms and/or [=parallel queues=].

Specify "Use another account". #678

Are you sure you want to change the base?

Specify "Use another account". #678

Conversation

cbiesinger commented Nov 4, 2024 • edited by pr-preview bot Loading

npm1 left a comment

Choose a reason for hiding this comment

cbiesinger commented Nov 7, 2024

npm1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TallTed Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hlflanagan commented Nov 26, 2024

wseltzer commented Jan 14, 2025 • edited Loading

bvandersloot-mozilla commented Feb 5, 2025

cbiesinger commented Feb 5, 2025

bvandersloot-mozilla commented Feb 5, 2025

cbiesinger commented Mar 6, 2025

cbiesinger commented Mar 13, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wseltzer commented Mar 25, 2025

cbiesinger commented Nov 4, 2024 •

edited by pr-preview bot

Loading

TallTed Nov 25, 2024 •

edited

Loading

wseltzer commented Jan 14, 2025 •

edited

Loading