559160 - nsINavBookmarksService.removeItem untags items, so manual untagging is unnecessary

Status: RESOLVED INVALID

(Firefox :: Sync, defect, P5)

Description

[Ed Lee :Mardak]

Existing code for removing bookmarks manually untags when it might not need to.

Comment 1

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

comment 0 is actually true if the tagging service is alive, that may be true or false. Provided you ensure something initialized the tagging service, you can avoid manual untagging.

Comment 2

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

there is an alternative though, that is to register the tagging service in the "bookmarks-observer" category instead of manually adding a bookmarks observer to it, that would mean any bookmarks notification will init it automatically. Could even make much sense looking at its initialization code.

Comment 3

[Richard Newman [:rnewman]]

The only tag operations I see in bookmarks.js:

  /**
   * Associate the provided URI with the provided array of tags.
   * If the provided URI is falsy, returns immediately.
   */
  _tagURI: function _tagURI(bookmarkURI, tags) {
    if (!bookmarkURI || !tags) {
      return;
    }

    // Filter out any null/undefined/empty tags.
    tags = tags.filter(function(t) t);

    // Temporarily tag a dummy URI to preserve tag ids when untagging.
    let dummyURI = Utils.makeURI("about:weave#BStore_tagURI");
    PlacesUtils.tagging.tagURI(dummyURI, tags);
    PlacesUtils.tagging.untagURI(bookmarkURI, null);
    PlacesUtils.tagging.tagURI(bookmarkURI, tags);
    PlacesUtils.tagging.untagURI(dummyURI, null);
  },


Does this look sane to you, Marco? If so, I think we can close this.

Comment 4

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

well, could be this bug was created when there was actually some code doing manual removal. I can confirm I don't see any code doing that atm. Next time we should annotate the code in question in the report :(

I'll likely file a bug apart to change the tagging service to use a category cache, since I like that idea :)