Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Status

()

Toolkit
WebExtensions: General
P2
normal
a year ago
17 days ago

People

(Reporter: Jerry Krinock, Assigned: bsilverberg, NeedInfo)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [bookmarks][design-decision-approved]triaged)

(Reporter)

Description

a year ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:46.0) Gecko/20100101 Firefox/46.0
Build ID: 20160502172042

Steps to reproduce:

I read the API documentation for WebExtensions > chrome.bookmarks,  https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/bookmarks.


Actual results:

I did not find any capability to *get* or *set* bookmark Description attribute.


Expected results:

Because WebExtensions are slated to replace the Add-Ons SDK, and because bookmarks extensions which sync bookmarks require access to all bookmark attributes, either

(1) The chrome.bookmarks API implemented by WebExtensions for Firefox should contain additional functions to *get* and *set* bookmark Description, or,

(2) There should be a link to some Mozilla policy statement which publicly states that bookmark Description is obsolete and will be removed in some future version of Firefox.
Summary: Bookmark Description : Either support in WebExtensions or Obsolete → Support bookmark descriptions
Whiteboard: [bookmarks][design-decision-needed]

Updated

a year ago
Whiteboard: [bookmarks][design-decision-needed] → [bookmarks][design-decision-needed]triaged
There was broad approval of this API in the meeting in London and no major concerns.
Whiteboard: [bookmarks][design-decision-needed]triaged → [bookmarks][design-decision-approved]triaged

Updated

a year ago
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

9 months ago
Component: WebExtensions: Untriaged → WebExtensions: General
Priority: -- → P5
Happy to raise a separate bug, however I would like the ability to get categories or tags too.

My use-case is to simplify container assignment in the future by using meta data the user has already assigned to the bookmark.
(Reporter)

Comment 3

3 months ago
(In reply to Jonathan Kingston [:jkt] from comment #2)
> Happy to raise a separate bug, however I would like the ability to get categories or tags too.

Tags already have Bug 1225916.
(Assignee)

Comment 4

24 days ago
I will look into making descriptions available to WebExtensions.
Assignee: nobody → bob.silverberg
Priority: P5 → P2
(Assignee)

Comment 5

24 days ago
From what I can deduce, the "description" of a bookmark or folder is stored as an annotation on the item which looks like:

{
  "name": "bookmarkProperties/description",
  "flags": 0,
  "expires": 4,
  "value": "This is the description."
}

This is returned by the PlacesUtils method that we use for getTree(), but not by the PlacesUtils.bookmarks methods we use for get() and search(), so I think that returning the description via those methods would be a lot of work, and will also change the data returned by those methods to be different than that returned by Chrome.

Therefore, I think adding API methods to get and set the description would be the better approach. In terms of implementation, it looks like we can use PlacesUIUtils.getItemDescription [1] to get the description, but setting the description will require some extra code somewhere. I did find the editItemDescription function [2], also in PlacesUIUtils.jsm, but it looks like that's not meant to be used any longer, so a similar implementation will need to be added somewhere.

Marco, could you please take a look at what I've written here and let me know if this seems like a reasonable thing to do and if my ideas for implementation make sense? Also if there are any other issues with doing this that I have not considered.

[1] http://searchfox.org/mozilla-central/rev/fdb811340ac4e6b93703d72ee99217f3b1d250ac/browser/components/places/PlacesUIUtils.jsm#773
[2] http://searchfox.org/mozilla-central/source/browser/components/places/PlacesUIUtils.jsm#1693
Flags: needinfo?(mak77)
(Reporter)

Comment 6

23 days ago
Bob, just to clarify … when you say 

> Therefore, I think adding API methods to get and set the description would be the better approach.

Are you proposing adding something like:

   browser.bookmarks.getDescription(id)
   browser.bookmarks.setDescription(id, description)

as a "better approach" than adding a `description` property to BookmarkTreeNode?

If so, I would point out that adding the property is what I proposed for `keywords`, in Bug 1276817.  It was technically quite a similar situation.  Initially, I was told to add the property, but then Marco killed that idea [1] before I posted my patch.  Although he killed it for a different reason in that case, I found that performance of, for example, getting the whole bookmarks tree, took quite a hit because it required, under the hood, fetching from the separate moz_keywords database table for each item.

I still think that just deprecating the `description` feature, as was effectively done for `keywords`, is a good solution, especially given the complexity of Firefox bookmarks storage.  I don't know what they were thinking when they made all of those separate tables, but it was definitely not the simple model of BookmarkTreeNodes with properties as set forth in WebExtensions `bookmarks` API.  But if Firefox must retain this little-used feature to distinguish it from Google Chrome, then maybe performance could be improved by providing a bulk getter and setter, wherein you'd pass an array of bookmark id/descriptions, or NULL to get all descriptions.  Of course, that would push the mess (and possibly the performance hit) on to extension developers. 

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1276817#c25)
(In reply to Bob Silverberg [:bsilverberg] from comment #5)
> From what I can deduce, the "description" of a bookmark or folder is stored
> as an annotation on the item which looks like:

You should not rely on the current storage of that information, Activity stream is actually adding a description field to moz_places and we could likely, in the short future, just use that (TBD).
Let's keep the discussion storage-agnostic for now.
Actually, AS is also adding methods to History.jsm to fetch and set the new description field.
That's ortogonal to bookmarks, but with a brief discussion with them we could end up deciding to just drop bookmarks description and use/update the  History one. And then you'd have your API to set/fetch information.

> Therefore, I think adding API methods to get and set the description would
> be the better approach. In terms of implementation, it looks like we can use
> PlacesUIUtils.getItemDescription [1] to get the description, but setting the
> description will require some extra code somewhere. I did find the
> editItemDescription function [2], also in PlacesUIUtils.jsm, but it looks
> like that's not meant to be used any longer, so a similar implementation
> will need to be added somewhere.

yes, editItemDescription is not what you want.

Is the description really necessary as a bookmark feature? I mean, it exists in our UI, but I'm not convinced it's being used by users that much, we'd need to measure its usage.
I think comment 6 sort of nailed the problem, we could decide we don't care about providing a description hook in WebExt unless we have clear use-cases in mind.
Flags: needinfo?(mak77)
(Assignee)

Comment 8

23 days ago
(In reply to Marco Bonardo [::mak] from comment #7)
> 
> Is the description really necessary as a bookmark feature? I mean, it exists
> in our UI, but I'm not convinced it's being used by users that much, we'd
> need to measure its usage.
> I think comment 6 sort of nailed the problem, we could decide we don't care
> about providing a description hook in WebExt unless we have clear use-cases
> in mind.

I agree that this is the question we should be asking now. Jerry did suggest in comment 6 that we could just deprecate bookmark descriptions, from his point of view.

Eric, I was directed to this bug by Andy with a suggestion that one or more add-on authors were asking for it to port their add-ons to WebExtensions. Could you look into this and see what use cases they have?
Flags: needinfo?(eric)
(Reporter)

Comment 9

22 days ago
Thank you guys for considering this issue.  To clarify, the issue is that Firefox' user interface (menu > Bookmarks > Show All Bookmarks) supports properties, such as `description`, which are not supported by WebExtensions.  Unsupported, evil techniques are therefore being used to access these properties.

In my view, as an extension developer, there are two acceptable resolutions.  Either

(a) Officially deprecate these properties, and eventually remove them from the user interface and from the places database.

or

(b) Support these properties in WebExtensions.  As I explained in Comment 6, the duration of operations like getting the entire bookmarks tree would become significantly longer than doing the same operation in a browser which has a simple bookmarks data model, such as Google Chrome.

OK, there could be a third alternative, much more work, 

(c)  Eeeek, change Firefox' data model to look like WebExtensions and Firefox' user interface.  The `description` property would become simply another column in moz_bookmarks.  It could then be simply added to bookmarks.BookmarkTreeNode, and performance would be on par with other browsers whose bookmarks do not feature these wonderful extra properties.
(In reply to Jerry Krinock from comment #9)
> (a) Officially deprecate these properties, and eventually remove them from
> the user interface and from the places database.

Honestly, description is exposed only in the properties dialog, and it's hidden by default in the Library (both column and the field).

As I said, Activity Stream is likely to make a better use of description, but for that we likely don't need it to be user-editable. So a possibility could be to make description just a non-editable field and AS will take care of it.

I don't have a clear plan in mind atm, we'll have to wait and see how AS evolves from here, and which data it will backfill. But off-hand I don't think description is either a killer nor a useful feature in its current form. Thus, can't think of use-cases where extensions may do a good use of it, since it's pretty much invisible to the average user.
(Reporter)

Comment 11

17 days ago
Marco, indeed Bookmark Description is very hard to find, and possibly has less users than the people on this bug.

By the way, I just looked at the changes to my places database after updating to FirefoxNightly 56, and see that it has two new columns in moz_places, one of them is `description TEXT`.  The Bookmark descriptions, old ones as well as new edits, still remain in moz_items_annos.

I presume that this new column is to support Activity Streams, and indicates that indeed the Activity Streams developers are going off in a different direction.
You need to log in before you can comment on or make changes to this bug.