1276819 - Support bookmark descriptions

Status: RESOLVED WONTFIX

(WebExtensions :: General, defect, P5)

Description

[Jerry Krinock]

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.

Comment 1

[Andy McKay]

There was broad approval of this API in the meeting in London and no major concerns.

Comment 2

[Jonathan Kingston [:jkt] he/him]

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.

Comment 3

[Jerry Krinock]

(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.

Comment 4

[Bob Silverberg [:bsilverberg]]

I will look into making descriptions available to WebExtensions.

Comment 5

[Bob Silverberg [:bsilverberg]]

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

Comment 6

[Jerry Krinock]

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)

Comment 7

[Marco Bonardo [:mak]]

(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.

Comment 8

[Bob Silverberg [:bsilverberg]]

(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?

Comment 9

[Jerry Krinock]

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.

Comment 10

[Marco Bonardo [:mak]]

(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.

Comment 11

[Jerry Krinock]

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.

Comment 12

[Jan Zavřel]

Please see my brainstorming comment at https://bugzilla.mozilla.org/show_bug.cgi?id=1384515#c39 for my use-case.
Query »tb folder:chapter1«, that I mentioned, lists results using columns Name, URL and Folder (which shows all the folders respective tab is bookmarked in).
I would add column description so as users can see and possibly insert descriptions in grid that can help them to access information faster. For example: possible storyline twist.
Users would be also able to use description field/column to filter tabs. Query »tb folder:chapter1 -desc:twist« would then filter all the tabs/resources which do not refer to any possible twist.
I consider description field as complementary to (parent) folder and tag fields which are only suitable for storing short information.
Also wanted to ask... Is this format (below) really necessary for storing descriptions? Why don’t just add simple string field to bookmarks.BookmarkTreeNode type (https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/bookmarks/BookmarkTreeNode)?
{
  "name": "bookmarkProperties/description",
  "flags": 0,
  "expires": 4,
  "value": "This is the description."
}

Comment 13

[Bob Silverberg [:bsilverberg]]

I'm still not seeing any compelling use cases for bookmark description in the bookmarks API. Taken with the fact that it is a highly unused feature which is a likely candidate for removal in the future, I'm going to drop this down to a P5 for now. We need to wait to see what happens with description and if it does become a field used by Activity Stream, and if so to what use it is put.

Comment 14

[Jerry Krinock]

(In reply to Jan Zavřel from comment #12)

> Also wanted to ask... Is this format (below) really necessary for storing
> descriptions? Why don’t just add simple string field to bookmarks.BookmarkTreeNode type

I've wondered this myself, and since no one has answered, I'll give you my guess.  As I recall, the `places` database in which bookmarks are stored appeared in Firefox 4.  It contains a half dozen or so tables, and the format you describe is necessary to establish relationships.  More broadly, in hindsight, `places` appears to have been way over-designed.  I did similar things when I was young.

Comment 15

[Jerry Krinock]

Oddly, Apple added a "Description" to bookmarks in Safari, I think it was a year or so ago.  But the only time the user sees this is in the "Add Bookmark" sheet, when the bookmark is initially added.  It is stored in Safari's Bookmarks.plist file as the value for key previewText.  But it never shows up in any user interface that I can see, neither in macOS or iOS.  I've thought maybe it is used in searching, but no, if I enter a strange word into "Description" when creating a bookmark, and then type that strange word into Safari's address field, the new bookmark is not listed in the search results.  At this time, "Description" seems to go into a black hole.  

OmniWeb's bookmarks has a "Notes" field, and iCab has "Remarks".  These would be the equivalent of "Description".  In case you've never heard of OmniWeb and iCab, those are web browsers available on macOS.  Their obscurity indicates how many users are *not* clamoring for "Description".

My suggestion:  Make it a P1 to remove this feature, and remove it today so that Mozilla can move on :)

Comment 16

[Marco Bonardo [:mak]]

Thank you for you suggestions, I already filed a bug to remove it earlier today.

Comment 17

[Marco Bonardo [:mak]]

Since we're removing bookmark description, this bug is now a wontfix.

Comment 18

[Stephan Sokolow]

Is there a plan to migrate description data somewhere?

(I ask this as someone whose opinion of Mozilla fell significantly when I was unexpectedly forced to go into my profile folder and make a backup of reading-list.sqlite for SQLiteMan-ing at my leisure when that feature went away.)

Comment 19

[Marco Bonardo [:mak]]

you can refer to bug 1402890. The current plan is to remove the UI in Firefox 62 and the data in Firefox 64, so users have 2 whole versions to recover the data through a json or html backup.