Bug 224402 - Force the WebAuthn compatible mode to always show UI
Summary: Force the WebAuthn compatible mode to always show UI
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jiewen Tan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-04-09 18:06 PDT by Jiewen Tan
Modified: 2021-04-14 14:39 PDT (History)
5 users (show)

See Also:

Attachments
Patch (1.65 KB, patch)
2021-04-09 18:09 PDT, Jiewen Tan 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Jiewen Tan 2021-04-09 18:06:48 PDT 
Force the WebAuthn compatible mode to always show UI.
Comment 1 Jiewen Tan 2021-04-09 18:07:03 PDT 
rdar://75757362
Comment 2 Jiewen Tan 2021-04-09 18:09:42 PDT 
Created attachment 425672 [details]
Patch
Comment 3 Garrett Davidson 2021-04-12 09:01:05 PDT 
Can you update the commit message with how you tested this?
Comment 4 Brent Fulgham 2021-04-12 16:24:03 PDT 
Comment on attachment 425672 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=425672&action=review

r=me

> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalConnection.mm:102
> +#if USE(APPLE_INTERNAL_SDK)

Does this need a macOS check? or is it correct for iOS, too?
Comment 5 Jiewen Tan 2021-04-12 16:26:20 PDT 
Comment on attachment 425672 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=425672&action=review

Thanks Brent for r+ this patch.

>> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalConnection.mm:102
>> +#if USE(APPLE_INTERNAL_SDK)
> 
> Does this need a macOS check? or is it correct for iOS, too?

This method is only used by the compatible mode which is now only on macOS.
Comment 6 EWS 2021-04-12 16:54:07 PDT 
Committed r275847 (236412@main): <https://commits.webkit.org/236412@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 425672 [details].
Comment 7 Darin Adler 2021-04-13 12:46:05 PDT 
Comment on attachment 425672 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=425672&action=review

> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalConnection.mm:103
> +    // A quirk to force the compatible mode to always show UI.

This comment should say *why* we are doing this, not just say that "it’s a quirk".
Comment 8 Jiewen Tan 2021-04-14 12:24:25 PDT 
Comment on attachment 425672 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=425672&action=review

>> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalConnection.mm:103
>> +    // A quirk to force the compatible mode to always show UI.
> 
> This comment should say *why* we are doing this, not just say that "it’s a quirk".

I was trying to write a why here. But then it's hard to not reveal the feature and explain it clearly. Let me try rewrite it with a vague why.
Comment 9 Jiewen Tan 2021-04-14 14:39:24 PDT 
Land a comment fix per Darin's comment.

Committed r275966: <https://commits.webkit.org/r275966>
Committed r275966 (236519@main): <https://commits.webkit.org/236519@main>