WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
200932
[WebAuthn] Warn users when multiple NFC tags present
https://bugs.webkit.org/show_bug.cgi?id=200932
Summary
[WebAuthn] Warn users when multiple NFC tags present
Jiewen Tan
Reported
2019-08-20 12:07:07 PDT
Allow user to select a different NFC tag if multiple present.
Attachments
Patch
(44.71 KB, patch)
2019-10-21 22:50 PDT
,
Jiewen Tan
no flags
Details
Formatted Diff
Diff
Patch
(44.73 KB, patch)
2019-10-22 02:06 PDT
,
Jiewen Tan
bfulgham
: review+
Details
Formatted Diff
Diff
Patch for relanding
(44.44 KB, patch)
2019-10-25 17:05 PDT
,
Jiewen Tan
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Jiewen Tan
Comment 1
2019-08-21 18:25:12 PDT
A solution here is when multiple tags are detected, we could advise users to pick one to use in the UI.
Radar WebKit Bug Importer
Comment 2
2019-08-30 11:49:34 PDT
<
rdar://problem/54890736
>
Jiewen Tan
Comment 3
2019-10-18 17:24:30 PDT
***
Bug 201017
has been marked as a duplicate of this bug. ***
Jiewen Tan
Comment 4
2019-10-21 22:50:40 PDT
Created
attachment 381511
[details]
Patch
Jiewen Tan
Comment 5
2019-10-22 02:06:18 PDT
Created
attachment 381533
[details]
Patch
Brent Fulgham
Comment 6
2019-10-23 14:27:50 PDT
Comment on
attachment 381533
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=381533&action=review
Looks good! Nice to see more WPT's passing. r=me.
> Source/WebKit/UIProcess/WebAuthentication/AuthenticatorTransportService.h:60 > + // These operation are guaranteed to execute asynchronously.
These operations are ...
> Source/WebKit/UIProcess/WebAuthentication/Cocoa/NfcConnection.mm:133 > + NSError *error = nil;
You don't have to initialize to 'nil' in ObjC. It happens automatically.
> Source/WebKit/UIProcess/WebAuthentication/Mock/MockNfcService.mm:259 > + [tags addObject:adoptNS([[WKMockNFTag alloc] initWithType:NFTagTypeGeneric4A tagID:adoptNS([[NSData alloc] initWithBytes:tagID2 length:sizeof(tagID2)]).get()]).get()];
Could something in here be 'mutable' so we can leave this as a 'const' method? (Assuming the mutation is not something that actually changes the logical constness of the object?)
Jiewen Tan
Comment 7
2019-10-23 14:38:36 PDT
Comment on
attachment 381533
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=381533&action=review
Thanks Brent for r+ this patch.
>> Source/WebKit/UIProcess/WebAuthentication/AuthenticatorTransportService.h:60 >> + // These operation are guaranteed to execute asynchronously. > > These operations are ...
Fixed.
>> Source/WebKit/UIProcess/WebAuthentication/Cocoa/NfcConnection.mm:133 >> + NSError *error = nil; > > You don't have to initialize to 'nil' in ObjC. It happens automatically.
I tried to grep different patterns on the project, and here is the result: "NSError *error;": 10 "NSError *error = nil;": 46 So I think maybe "NSError *error = nil;" is more inline with our coding practice.
>> Source/WebKit/UIProcess/WebAuthentication/Mock/MockNfcService.mm:259 >> + [tags addObject:adoptNS([[WKMockNFTag alloc] initWithType:NFTagTypeGeneric4A tagID:adoptNS([[NSData alloc] initWithBytes:tagID2 length:sizeof(tagID2)]).get()]).get()]; > > Could something in here be 'mutable' so we can leave this as a 'const' method? (Assuming the mutation is not something that actually changes the logical constness of the object?)
Good catch. This is a mistake. I was doing something else that requires dropping the const qualifier, but then later I removed those code.
Jiewen Tan
Comment 8
2019-10-23 14:58:25 PDT
Committed
r251500
: <
https://trac.webkit.org/changeset/251500
>
Matt Lewis
Comment 9
2019-10-25 14:32:01 PDT
This was rolled out because it interfered with rolling out of
https://bugs.webkit.org/show_bug.cgi?id=202561
The rollout
https://trac.webkit.org/changeset/251602/webkit
Brent Fulgham
Comment 10
2019-10-25 15:07:05 PDT
(In reply to Matt Lewis from
comment #9
)
> This was rolled out because it interfered with rolling out of > >
https://bugs.webkit.org/show_bug.cgi?id=202561
> > The rollout
https://trac.webkit.org/changeset/251602/webkit
I don't think rolling out this important bug fix because you had trouble rolling out a separate patch is appropriate.
Jiewen Tan
Comment 11
2019-10-25 17:05:38 PDT
Created
attachment 381983
[details]
Patch for relanding
Jiewen Tan
Comment 12
2019-10-26 06:49:22 PDT
Comment on
attachment 381983
[details]
Patch for relanding The iOS EWS seems stuck. cq+ it regardless.
Jiewen Tan
Comment 13
2019-10-27 15:53:42 PDT
Committed
r251645
: <
https://trac.webkit.org/changeset/251645
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug