Bug 217354 – [Contact Picker API] Add support for ContactsManager.getProperties()

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Aditya Keerthi]

...

[Aditya Keerthi]

<rdar://problem/69862099>

[Aditya Keerthi]

Created attachment 410593 [details]
Patch

[youenn fablet]

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");

[Aditya Keerthi]

(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.

[EWS]

Committed r268144: <https://trac.webkit.org/changeset/268144>

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