Bug 208703 – [WebAuthn] Customize a bit more on the macOS LocalAuthentication prompt

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Jiewen Tan]

Customize a bit more on the macOS LocalAuthentication prompt.

[Radar WebKit Bug Importer]

<rdar://problem/60136974>

[Jiewen Tan]

Created attachment 392696 [details]
Patch

[Jiewen Tan]

Created attachment 392771 [details]
Patch

[Darin Adler]

Comment on attachment 392771 [details]
Patch

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

> Source/WebCore/en.lproj/Localizable.strings:887
> +"Touch ID to allow signning into â%@â with Touch ID." = "Touch ID to allow signning into â%@â with Touch ID.";

Spelling error here in the word "signing"

> Source/WebCore/platform/LocalizedStrings.cpp:1218
> +    return formatLocalizedString(WEB_UI_CFSTRING("Touch ID to allow signning into â%@â with Touch ID.", "Allow using Touch ID to sign into the specified website on this device"), rpId.createCFString().get());

Ditto.

> Source/WebCore/platform/LocalizedStrings.h:344
> +    WEBCORE_EXPORT String makeCredentialTouchIDPromptTitle(const String& rpId);

Do we really have to call this "rpId"? Are there words we could use instead?

> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalConnection.mm:52
> +#elif PLATFORM(MAC)

I think we should just use #else here.

[Jiewen Tan]

Comment on attachment 392771 [details]
Patch

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

Thanks Darin for the r+!

>> Source/WebCore/en.lproj/Localizable.strings:887
>> +"Touch ID to allow signning into â%@â with Touch ID." = "Touch ID to allow signning into â%@â with Touch ID.";
> 
> Spelling error here in the word "signing"

Fixed.

>> Source/WebCore/platform/LocalizedStrings.cpp:1218
>> +    return formatLocalizedString(WEB_UI_CFSTRING("Touch ID to allow signning into â%@â with Touch ID.", "Allow using Touch ID to sign into the specified website on this device"), rpId.createCFString().get());
> 
> Ditto.

Fixed.

>> Source/WebCore/platform/LocalizedStrings.h:344
>> +    WEBCORE_EXPORT String makeCredentialTouchIDPromptTitle(const String& rpId);
> 
> Do we really have to call this "rpId"? Are there words we could use instead?

We could call it domain.

>> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalConnection.mm:52
>> +#elif PLATFORM(MAC)
> 
> I think we should just use #else here.

Fixed.

[Jiewen Tan]

Created attachment 393535 [details]
Patch for landing

[Jiewen Tan]

Comment on attachment 393535 [details]
Patch for landing

Always be tricked by the fact that webkit-patch upload will bundle any local changes into one patch.

[Jiewen Tan]

Created attachment 393537 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 393537 [details]
Patch for landing

Clearing flags on attachment: 393537

Committed r258442: <https://trac.webkit.org/changeset/258442>

[Jiewen Tan]

Created attachment 393687 [details]
Part 2

[Brent Fulgham]

Comment on attachment 393687 [details]
Part 2

r=me

[Jiewen Tan]

Created attachment 394070 [details]
Part 2

[Jiewen Tan]

(In reply to Brent Fulgham from comment #11)
> Comment on attachment 393687 [details]
> Part 2
> 
> r=me

Thanks for the review. I just upload a newer version which addresses comments from our Tuesday meeting.

[Darin Adler]

Comment on attachment 394070 [details]
Part 2

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

> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalConnection.mm:55
> +    NSBundle *mainBundle = [NSBundle mainBundle];
> +    bundleName = appBundle.infoDictionary[(id)_kCFBundleDisplayNameKey];
> +    if (!bundleName)
> +        bundleName = appBundle.infoDictionary[(id)kCFBundleNameKey];
> +    if (!bundleName)
> +        bundleName = [mainBundle bundleIdentifier];

This code is not compiling on iOS bots

[Jiewen Tan]

(In reply to Darin Adler from comment #14)
> Comment on attachment 394070 [details]
> Part 2
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=394070&action=review
> 
> > Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalConnection.mm:55
> > +    NSBundle *mainBundle = [NSBundle mainBundle];
> > +    bundleName = appBundle.infoDictionary[(id)_kCFBundleDisplayNameKey];
> > +    if (!bundleName)
> > +        bundleName = appBundle.infoDictionary[(id)kCFBundleNameKey];
> > +    if (!bundleName)
> > +        bundleName = [mainBundle bundleIdentifier];
> 
> This code is not compiling on iOS bots

I fixed that locally but was struggling to get an iOS device to run. Will upload a new patch soon once I'm done testing on my iOS device.

[Jiewen Tan]

Created attachment 394239 [details]
Part 2

[Jiewen Tan]

Created attachment 394240 [details]
Part 2

[Brent Fulgham]

Comment on attachment 394240 [details]
Part 2

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

R=me

> Source/WebKit/ChangeLog:12
> +        And tunes a bit on the makeCredential string.

It also polishes the text used for makeCredential.

> Source/WebKit/ChangeLog:14
> +        Besides that, it also enhances the iOS titles as well.

Besides that, it also enhances the iOS title strings.

[Jiewen Tan]

Comment on attachment 394240 [details]
Part 2

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

Thanks Brent for the r+.

>> Source/WebKit/ChangeLog:12
>> +        And tunes a bit on the makeCredential string.
> 
> It also polishes the text used for makeCredential.

Fixed.

>> Source/WebKit/ChangeLog:14
>> +        Besides that, it also enhances the iOS titles as well.
> 
> Besides that, it also enhances the iOS title strings.

Fixed.

[Jiewen Tan]

Created attachment 394434 [details]
Patch for landing

[EWS]

Committed r258961: <https://trac.webkit.org/changeset/258961>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 394434 [details].