Comment 19 for bug 1733002

Revision history for this message
In , Bfulgham-b (bfulgham-b) wrote :

Comment on attachment 365256
Patch

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

Nice work getting this together. r=me with the changes suggested, and assuming tests pass when you are done.

>> Source/WebCore/Modules/webauthn/fido/U2fCommandConstructor.h:65
>> +WEBCORE_EXPORT Optional<Vector<uint8_t>> convertToU2fSignCommand(const Vector<uint8_t>& clientDataHash, const WebCore::PublicKeyCredentialRequestOptions&, const Vector<uint8_t>& keyHandle, bool isAppId = false);
>
> I should explain the change in the ChangeLog: the checkOnly flag is never used and therefore is deleted.

Yes -- I got confused when I looked at the implementation, until I saw this note! :-)

> Source/WebKit/UIProcess/WebAuthentication/fido/U2fHidAuthenticator.cpp:214
> + response->appid = true;

Could this be:

response->appid = m_isAppId;