1271567 - Add ability to read/write extra metadata (annotations) for each bookmark

Status: RESOLVED WONTFIX

(WebExtensions :: Untriaged, defect)

Description

[YUKI "Piro" Hiroshi]

Some my addons uses bookmark annotations via `PlacesUtils.setAnnotationsForItem()` and `PlacesUtils.getAnnotationsForItem()`. For example, Tree Style Tab https://addons.mozilla.org/en-US/firefox/addon/tree-style-tab/ ) stores tree structure of tabs for bookmarks created from those tabs. And, when tabs are opened from a bookmark folder, tree structure of opened tabs are automatically restored based on annotations.

On the other hand, WebExtensions's bookmarks API doesn't provide ability to read/write such metadata for each bookmark.

https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/Bookmarks/update

After XUL is ended we need something to alter `PlacesUtils.setAnnotationsForItem()` and `PlacesUtils.getAnnotationsForItem()`. Moreover, already stored annotation should be accessible via the new API for backward compatibility.

Comment 1

[YUKI "Piro" Hiroshi]

Plan 1: global annotation

~~~
// get metadata
chrome.bookmarks.get(id', function(result) {
  var bookmark = result;
  ...
});

// set metadata
var annotations = bookmark.annotations || {};
annotations.myPrivateData = 'foo';
chrome.bookmarks.update(
  id,
  { url: "http://www.example.com/",
    title: "example",
    annotations: annotations
  },
  function(result) {
  }
);
~~~

Pros:

 * Easy to use.
 * Very simple.
 * Bookmark related addons can collaborate with shared annotations as their message.

Cons:

 * If multiple addons use same annotation name, they will conflict.

Comment 2

[YUKI "Piro" Hiroshi]

Sorry, the sample for "get" is wrong. Correct code:

~~~
// get metadata
chrome.bookmarks.get(id', function(result) {
  var bookmark = result;
  var annotations = bookmark.annotations || {};
  ...
});
~~~

Comment 3

[YUKI "Piro" Hiroshi]

Plan 2: global annotation with methods for annotations

~~~
// get
chrome.bookmarks.getAnnotations(id, function(annotations) {
});

// set
annotations.myPrivateData = 'bar';
chrome.bookmarks.setAnnotations(id, annotations, function(result) {});
~~~

Pros:

 * Easy and simple (same to the plan 1)
 * Clearly separated from existing API (chrome.bookmarks.get/update).
   Even if Google Chrome (or others) uses the "annotations" property of bookmarks
   for different purpose, this API still works.

Cons:

 * If multiple addons use same annotation name, they will conflict. (same to the plan 1)
 * Not natural for existing chrome.bookmarks.get/update, especially update.
   The API accepts an object as its second parameter, and it seems to accept
   more other properties in the future.
   (Possibly, already. Sorry but I don't familiar to Chrome's API.)

Comment 4

[YUKI "Piro" Hiroshi]

Plan 3: private annotation for addons

I have no new idea about API interfaces. Simply, this plan means a modified version of the plan 1 (or the plan 2) with a restriction: each addon can read/write annotations created by itself. Even if multiple addons set annotations with same name, they are treated as different annotations. This behavior is similar to the existing storage API.

Pros:

 * Easy and simple (same to the plan 1, 2)
 * Conflict-free. Very safe for multiple addons.

Cons:

 * Bookmark related addons cannot collaborate based on annotations.
   For example, even if an addon X stores an annotation named "tags",
   another addon Y cannot list bookmarks which have same "tags" annotations.

Comment 5

[YUKI "Piro" Hiroshi]

You'll have a question: why don't use the existing storage API for the plan 3?

The answer is: annotations will be destructed automatically for deleted bookmarks, like WeakMap. Actually, the plan 3 can be done by existing (and already planned) bookmarks API, but you have to observe changes for bookmarks. If you forget to delete stored annotation when a bookmark is deleted, then it will become a garbage.

Comment 6

[Caitlin Neiman [:caitmuenster]]

This has been added to the agenda for the May 9 WebExtensions triage meeting. 

Wiki: https://wiki.mozilla.org/Add-ons/Contribute/Triage

Agenda: https://docs.google.com/document/d/1q7UD3DxsT5z0lO3EbOyF9Iln32Wg_e9LYMdHZ80BEb4/edit#

Comment 7

[Bob Silverberg [:bsilverberg]]

Marco, we are considering supporting this extension to the bookmarks API, but I know you've talked in the past about a questionable future for annotations. Can you comment on whether you think it is a good or bad idea to create an API that interacts with annotations?

Comment 8

[Marco Bonardo [:mak]]

(In reply to Bob Silverberg [:bsilverberg] from comment #7)
> Marco, we are considering supporting this extension to the bookmarks API,
> but I know you've talked in the past about a questionable future for
> annotations. Can you comment on whether you think it is a good or bad idea
> to create an API that interacts with annotations?

Off-hand it may not be a great idea. The annotations API is synchronous, it does main-thread IO. It's also under consideration for a full rewrite (bug 699844) for which we don't have a resourcing plan atm. Since the future design is not finalized, guessing an API now may not work well.
Long term plans include experimental things like Mentat, that are still in an ideation state.

Off-hand looks like there may be already existing workarounds for this, the add-on could store information in one of the better performing stores (indexedDB, async storage) and associate that information with guids. Bookmark guids don't usually change, unless the database is replaced manually (and if that ends up being a problem we could provide an API that identifies the current bookmarks db through an hash). Alternatively the info could be associated to 64bits hashes generated from the url, and then it should mostly work (the collision probability should be low enough).

Comment 9

[Bob Silverberg [:bsilverberg]]

Thanks for the response Marco. Caitlin, can you make sure this ends up on the agenda for next week's meeting? I think we can finalize a decision at that time.

Comment 10

[Caitlin Neiman [:caitmuenster]]

Sure thing! This has been added to the agenda for the May 16 WebExtensions Triage meeting. Marco, would you be able to join us? 

Wiki: https://wiki.mozilla.org/Add-ons/Contribute/Triage

Agenda: https://docs.google.com/document/d/1vrhHNOelBty4hXcjQ8VbFk-azHRFFDVyGka7H0VpEa8/edit#

Comment 11

[Marco Bonardo [:mak]]

In reply to Caitlin Neiman (http://pronoun.is/she) from comment #10)
> Sure thing! This has been added to the agenda for the May 16 WebExtensions
> Triage meeting. Marco, would you be able to join us?

Unfortunately Tuesdays are not great days for late meetings (If I read it correctly it would be at 7:30 PM for me), other days would work better :(

Comment 12

[Bob Silverberg [:bsilverberg]]

Based on the additional information from Marco, and the fact that there are a number of possible workarounds we do not feel that this is a good WebExtensions API to implement at this time.

Comment 13

[YUKI "Piro" Hiroshi]

Any description about this viewpoint?:

> You'll have a question: why don't use the existing storage API for the plan
> 3?
> 
> The answer is: annotations will be destructed automatically for deleted
> bookmarks, like WeakMap. Actually, the plan 3 can be done by existing (and
> already planned) bookmarks API, but you have to observe changes for
> bookmarks. If you forget to delete stored annotation when a bookmark is
> deleted, then it will become a garbage.

By this reason, manual association between bookmarks and other storage forces us to re-implement garbage collection by our hand. Is it not a topic that WebExtensions should solve?

Comment 14

[YUKI "Piro" Hiroshi]

Even if GUID of bookmarks are permanent, the problem still there: "you have to observe changes for
> bookmarks. If you forget to delete stored annotation when a bookmark is
> deleted, then it will become a garbage."

Comment 15

[Caitlin Neiman [:caitmuenster]]

Hi Piro, thanks for raising these questions. I don't think that these concerns are enough to overturn the decision to not implement the API.