Closed Bug 1388890 Opened 7 years ago Closed 7 years ago

Using the {browser,chrome}.bookmarks.update WebExtensions API is destructive for tags

Categories

(WebExtensions :: General, defect, P1)

54 Branch
defect

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 1225916

People

(Reporter: dan, Assigned: bsilverberg)

References

Details

User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.90 Safari/537.36 Steps to reproduce: 1. Install this Add-On: https://addons.mozilla.org/EN-US/firefox/addon/bookmark-301-updater/ 2. Create a bookmark to https://www.slashdot.org/ (which at the time of this bug report 301s to https://slashdot.org/) 3. Add tags to this newly-created bookmark. 4. Follow the bookmark. 5. Observe that the add-on updates the URL for this bookmark to reflect the 301 sent from the server. (ie, it's now 'https://slashdot.org/'.) This is an extract from the add-on's source with the two variables defined/hard-coded to match the example in this bug report. ``` let old_url = 'https://www.slashdot.org/'; let new_url = 'https://slashdot.org/'; chrome.bookmarks.search({url: old_url}, function(bookmarkItems) { for (let item of bookmarkItems) { chrome.bookmarks.update(item.id, { url: new_url }); } }); ``` Note that this bug report is opened after verifying feedback from a real user who reported the problem in a review. https://addons.mozilla.org/EN-US/firefox/addon/bookmark-301-updater/reviews/863373/ The documentation for this WebExtensions API explicitly states that "Any items not specified aren't changed in the referenced bookmark" (Firefox) and "unspecified properties will be left unchanged" (Chrome). https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/bookmarks/update https://developer.chrome.com/extensions/bookmarks#method-update Actual results: Bookmark no longer has any tags. Expected results: Bookmark should have retained its tags.
Component: Untriaged → WebExtensions: General
Product: Firefox → Toolkit
Priority: -- → P1
Assignee: nobody → bob.silverberg
The bookmarks.update API is not destructive. What you are seeing is a result of how tags work. Tags are assigned to URLs, not to individual bookmarks, so when you change the URL of a bookmark via bookmarks.update(), it becomes disassociated with any tags that were added to its old URL. You will find that if you update the same bookmark again, reassigning the old URL to it, the tags will re-appear.
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
Resolution: --- → INVALID
Thanks for looking into this Bob. I agree that the underlying system links tags to URLs (not bookmarks), but I don't agree that this bug should be closed as INVALID. When using the built-in bookmark manager, tags are not lost when changing the URL of a bookmark. From what I can tell, if there is only one bookmark to the old URL, the internal tag-to-URL mapping is updated with the new URL (old URL is no longer tagged). If there are multiple bookmarks to the old URL, the tag-to-URL internal mapping is updated to also include the new URL (old URL remains tagged). In both of these cases, all bookmarks retain their tags. This is the behaviour that I expect from the bookmarks.update() API: bookmarks retain their tags. The documentation explicitly states that "items not specified aren't changed in the referenced bookmark". There is no mention of tags in the API, so I expect these to remain unchanged when calling the API. The internal tag-to-URL mapping needs to be updated when the bookmarks.update() API is called, just like it is when using the built-in bookmark manager.
Status: RESOLVED → UNCONFIRMED
Resolution: INVALID → ---
Thanks for the further info Dan. If that's the current behaviour of the front-end, then I agree that it would be best if we did the same for bookmarks.update().
Status: UNCONFIRMED → NEW
Ever confirmed: true
Just because the front end does it that way, doesn't mean we need to write the API that way. Would it make sense to just implement bug 1225916 instead? That way the extension could then be the one who decides if it makes sense to move the tags or not?
(In reply to Andy McKay [:andym] from comment #4) > Just because the front end does it that way, doesn't mean we need to write > the API that way. Would it make sense to just implement bug 1225916 instead? > That way the extension could then be the one who decides if it makes sense > to move the tags or not? This is a valid point, and if our proposed tagging API is going to associate tags with URLs (as opposed to tags with bookmarks), which is also how they behave internally and what has been proposed at bug 1225916, then it makes sense to just cover this with a tagging API and not add anything to the bookmarks API.
Status: NEW → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → DUPLICATE
This is likely to be fixed as part of the work in bug 1452073 and/or bug 1448885.
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.