Closed Bug 1617490 Opened 4 years ago Closed 2 years ago

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

Categories

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

Product:
Thunderbird
Component:
Add-Ons: Extensions API
Type:
defect

Tracking

(thunderbird_esr102+ wontfix)

Status:
RESOLVED FIXED
Milestone:
106 Branch
Tracking Flags:
Tracking Status
thunderbird_esr102 + wontfix

People

(Reporter: TbSync, Assigned: TbSync)

References

Details

Attachments

(4 files, 1 obsolete file)

Bug 1617490 - Add binary photo data to returned vCards of local address books. r=darktrojan
48 bytes, text/x-phabricator-request
Details | Review
Bug 1617490 - Add contacts.getPhoto(). r=mkmelin
48 bytes, text/x-phabricator-request
Details | Review
Bug 1617490 - Use File.createFromNsIFile() and FileReader to get photo file type instead of looking at magic bytes. r=darktrojan
48 bytes, text/x-phabricator-request
Details | Review
Bug 1617490 - Add contacts.setPhoto(). r=darktrojan
48 bytes, text/x-phabricator-request
Details | Review

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 :-)

We should have this ready before next ESR.

Priority: -- → P3

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?

tracking-thunderbird_esr102: --- → ?
Flags: needinfo?(john)
Assignee: nobody → john
Status: NEW → ASSIGNED
Flags: needinfo?(john)

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?

Flags: needinfo?(geoff)

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. :-)

Flags: needinfo?(geoff)
Attachment #9289844 - Attachment is obsolete: true
Keywords: checkin-needed-tb, leave-open
status-thunderbird_esr102: --- → affected
tracking-thunderbird_esr102: ? → +
Target Milestone: --- → 105 Branch

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/4e0c54d3de93
Add binary photo data to returned vCards of local address books. r=darktrojan

Keywords: checkin-needed-tb

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

Backout by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/146e6ffd7074
Backed out changeset 4e0c54d3de93 for test failures. rs=backout

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.

Keywords: checkin-needed-tb

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/a636e86c4055
Add binary photo data to returned vCards of local address books. r=darktrojan

Keywords: checkin-needed-tb
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/255a062e0e0e
follow-up - fix linting. rs=eslint DONTBUILD

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

Backout by mkmelin@iki.fi:
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

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

Depends on D154488

Check in attempt #3. Try is looking good.

Keywords: checkin-needed-tb

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

Keywords: checkin-needed-tb
Target Milestone: 105 Branch → 106 Branch

Requesting check-in for D155877.

Keywords: checkin-needed-tb

Pushed by geoff@darktrojan.net:
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

Keywords: checkin-needed-tb

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?

Flags: needinfo?(mkmelin+mozilla)

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

Flags: needinfo?(mkmelin+mozilla)
Keywords: leave-open → checkin-needed-tb

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/06d36a2bac03
Add contacts.setPhoto(). r=darktrojan

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Keywords: checkin-needed-tb
Resolution: --- → FIXED

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

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: