Bug 217354

Summary: [Contact Picker API] Add support for ContactsManager.getProperties()
Product: WebKit Reporter: Aditya Keerthi <akeerthi>
Component: New BugsAssignee: Aditya Keerthi <akeerthi>
Status: RESOLVED FIXED    
Severity: Normal CC: webkit-bug-importer, wenson_hsieh, youennf
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Aditya Keerthi 2020-10-05 17:04:34 PDT
...
Comment 1 Aditya Keerthi 2020-10-05 17:04:58 PDT
<rdar://problem/69862099>
Comment 2 Aditya Keerthi 2020-10-05 17:07:09 PDT
Created attachment 410593 [details]
Patch
Comment 3 youenn fablet 2020-10-06 00:57:24 PDT
Comment on attachment 410593 [details]
Patch

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

> Source/WebCore/ChangeLog:10
> +        by the API. These currently include name, email and tel.

Is there a spec available?

> Source/WebCore/Modules/contact-picker/ContactsManager.cpp:59
> +    Vector<ContactProperty> properties = { ContactProperty::Email, ContactProperty::Name, ContactProperty::Tel };

Is the order important? Should getProperties return a Set instead?

> LayoutTests/contact-picker/contacts-manager-get-properties.html:18
> +            shouldBeTrue("properties.includes('tel')");

If the order is important, we might want to validate email is first, then name, then tel.
If this test can be shared with other browsers as WPT, we could use resources/testharness.js.
This would be written as:

promise_test(async () => {
 ...
}, "getProperties");
Comment 4 Aditya Keerthi 2020-10-06 08:32:50 PDT
(In reply to youenn fablet from comment #3)
> Comment on attachment 410593 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=410593&action=review
> 
> > Source/WebCore/ChangeLog:10
> > +        by the API. These currently include name, email and tel.
> 
> Is there a spec available?

The spec can be found here: https://wicg.github.io/contact-api/spec/#dom-contactsmanager-getproperties.
 
> > Source/WebCore/Modules/contact-picker/ContactsManager.cpp:59
> > +    Vector<ContactProperty> properties = { ContactProperty::Email, ContactProperty::Name, ContactProperty::Tel };
> 
> Is the order important? Should getProperties return a Set instead?

The spec does not place any restrictions on the order, but this implementation matches the Blink implementation.
 
> > LayoutTests/contact-picker/contacts-manager-get-properties.html:18
> > +            shouldBeTrue("properties.includes('tel')");
> 
> If the order is important, we might want to validate email is first, then
> name, then tel.
> If this test can be shared with other browsers as WPT, we could use
> resources/testharness.js.
> This would be written as:
> 
> promise_test(async () => {
>  ...
> }, "getProperties");

These is a WPT for this method here: https://github.com/web-platform-tests/wpt/blob/master/contacts/contacts-select.https.window.js.

However that test simply verifies that properties.length > 0, since it is up to the user agent to decide which properties are supported.
Comment 5 EWS 2020-10-07 13:10:41 PDT
Committed r268144: <https://trac.webkit.org/changeset/268144>

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