[webkit-reviews] review granted: [Bug 217354] [Contact Picker API] Add support for ContactsManager.getProperties() : [Attachment 410593] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Aditya Keerthi
<akeerthi at apple.com>'s request for review:
Bug 217354: [Contact Picker API] Add support for
ContactsManager.getProperties()
https://bugs.webkit.org/show_bug.cgi?id=217354

Attachment 410593: Patch

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




--- Comment #3 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 410593
  --> https://bugs.webkit.org/attachment.cgi?id=410593
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");