1617490 - contacts API : storing photos must be handled by the API directly

Status: RESOLVED FIXED

(Thunderbird :: Add-Ons: Extensions API, defect, P3)

Description

[John Bieling (:TbSync)]

Currently, if I want to add a photo to contact, I need to set the following contact properties:

• PhotoName
• PhotoType
• PhotoURI

with PhotoURI pointing to a file inside the Photos folder inside the users profile. With WebExtensions however direct filesystem access is not possible, so the contacts API must acceppt the image data directly and take care of the storage.

We still need the PhotoURI property, to store an external link, so do not throw that away :-)

Comment 3

[John Bieling (:TbSync)]

We should have this ready before next ESR.

Comment 4

[Mark Banner (:standard8)]

With 102 it is now not possible to get a photo url from the contacts API. I've tried various things to implement an experimental API to access the photos, but for some reason they've not worked. Please can we either work on a workaround, or get a fix for this?

Comment 5

[John Bieling (:TbSync)]

Adding a photo to a local address book:

• there is a PhotoName property ("410cc2d6-0730-416c-b104-1fc0e8388aa2.jpg")
• there is no PHOTO property in the vCard string

Receiving a contact with photo from an (icloud) CardDAV server:

• there is no PhotoName property
• there is a PHOTO property in the vCard string (PHOTO;VALUE=URI:https://www.removed.de/media/5fbf95ed-ee4e\r\n -4100-ae24-104f7a917669/john.jpg?w=212&h=300&scale=both&action=crop&anchor\r\n =middlecenter)

Adding a photo to a contact in an address book on an (icloud) CardDAV server:

• there is no PhotoName property
• there is a PHOTO property in the vCard string (PHOTO;VALUE=URI:https://gateway.icloud.com/contacts/1776546672/ck/card/a54b\r\n d9fca43466ccfef17cd5995e2ffc)

My ownCloud server uses PHOTO;ENCODING=b;TYPE=JPEG:<binarydata> for case 2 and 3.

Proposal:

• We should add a PHOTO;ENCODING=b;TYPE=JPEG:<binarydata> entry to the VCARD string for case 1.
• Updating the PHOTO value in the vCard property should update the photo used by Thunderbird (I did not check if that is the case already)
• We should probably also add contacts.getPhoto() / contacts.setPhoto() methods, which uses a DOM file object to set/get the photo

@Geoff: Since you have the most insights, how does that sound?

Comment 6

[Geoff Lankow (:darktrojan)]

There's several things here:

• For local contacts we don't store the photo in the vCard to avoid (a) storing it in the database and (b) having to parse it every time we want the card. For remote contacts we leave the photo in the vCard to avoid having to extract it and reinsert it. Since IIRC you're already constructing a new vCard you could insert/extract the photo as you go, but:
• The format changes. For version 3 vCards the PHOTO;ENCODING=b;TYPE=JPEG:<binarydata> format is common (and TYPE=PNG) but it could be PHOTO;VALUE=URI:, usually with a data URI. For version 4 vCards only the URI type is allowed. Some servers will accept the wrong thing, others will eat the photo if they're feeling hungry (hello Google).
• If an extension's supplying a photo you should probably at least discourage huge photos, if not block them outright (or reduce to a more reasonable size, the address book front end does this).

In general, photos are a nightmare. Good luck. :-)

Comment 7

[John Bieling (:TbSync)]

Attachment: Bug 1617490 - Add binary photo data to returned vCards of local address books. r=darktrojan

Comment 8

[John Bieling (:TbSync)]

Attachment: Bug 1617490 - Add binary photo data to returned vCards of local address books. r=darktrojan

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/4e0c54d3de93
Add binary photo data to returned vCards of local address books. r=darktrojan

Comment 10

[Geoff Lankow (:darktrojan)]

This patch breaks test_ext_experiments.js. It's here in your Try run.

Comment 11

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/comm-central/rev/146e6ffd7074
Backed out changeset 4e0c54d3de93 for test failures. rs=backout

Comment 12

[John Bieling (:TbSync)]

The function context.extension.addressBookManager.convert() is now async. That will break Experiments (like it broke the test), but that manager is not documented anywhere. Will announce it on TopicBox.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/a636e86c4055
Add binary photo data to returned vCards of local address books. r=darktrojan

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/255a062e0e0e
follow-up - fix linting. rs=eslint DONTBUILD

Comment 15

[Magnus Melin [:mkmelin]]

comm/mail/components/extensions/test/xpcshell/test_ext_addressBook.js is failing everywhere. Going to backout.
https://treeherder.mozilla.org/logviewer?job_id=388749470&repo=comm-central&lineNumber=4505

Comment 16

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/comm-central/rev/d984031776d8
Backed out changeset 255a062e0e0e for test failures. rs=backout
https://hg.mozilla.org/comm-central/rev/4ad4924ef320
Backed out changeset a636e86c4055 for test failures. rs=backout

Comment 17

[John Bieling (:TbSync)]

The image vanished from the patch (size 0). I have no idea why that happened. Sorry for the extra steps:

Try run:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=02ad5c5644814bf7576969c4c0e78e5231d76b4a

Comment 18

[John Bieling (:TbSync)]

Attachment: Bug 1617490 - Add contacts.getPhoto(). r=mkmelin

Depends on D154488

Comment 19

[John Bieling (:TbSync)]

Check in attempt #3. Try is looking good.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/28d216ed2544
Add binary photo data to returned vCards of local address books. r=darktrojan

Comment 21

[John Bieling (:TbSync)]

Try run with contacts.getPhoto():
#1: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=6183cc67cc6c388fc940b20102829e850ec19268
#2: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=1ad6733237fdce98eab5cf692c2e267cddb909fb

Try #2 had issues with bct4 on Linux/Mac, but after retriggering, they are both green:
https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=1ad6733237fdce98eab5cf692c2e267cddb909fb&selectedTaskRun=fPijrtGkTzu5_amkpXRWSQ.1

Comment 22

[John Bieling (:TbSync)]

Attachment: Bug 1617490 - Use File.createFromNsIFile() and FileReader to get photo file type instead of looking at magic bytes. r=darktrojan

Depends on D155877

Comment 23

[John Bieling (:TbSync)]

Requesting check-in for D155877.

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/fb28bd03bd00
Add contacts.getPhoto(). r=mkmelin
https://hg.mozilla.org/comm-central/rev/b3a6790be34f
Use File.createFromNsIFile() and FileReader to get photo file type instead of looking at magic bytes. r=darktrojan

Comment 25

[John Bieling (:TbSync)]

Attachment: Bug 1617490 - Add contacts.setPhoto(). r=darktrojan

Comment 26

[John Bieling (:TbSync)]

I am not able to fully analyse this try-run:
https://treeherder.mozilla.org/jobs?repo=try-comm-central&selectedTaskRun=JXdiFkodRGOCA3VUNFWfdw.0&revision=d9644ab8b3959dc5a01d28cfd54d64bb2decb30e

Is it save to check this in? I am not able to view the failing xpcshell tests in debug and cannot verify them. Any suggestions?

Comment 27

[John Bieling (:TbSync)]

I have created a new try-run and I do not see how I cause the reported fails. Requesting check-in.

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/06d36a2bac03
Add contacts.setPhoto(). r=darktrojan

Comment 29

[John Bieling (:TbSync)]

Note to myself: When (if) we uplift this, do not forget bug 1794659.