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)
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.
Updated•7 years ago
|
Component: Untriaged → WebExtensions: General
Product: Firefox → Toolkit
Updated•7 years ago
|
Priority: -- → P1
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → bob.silverberg
Assignee | ||
Comment 1•7 years ago
|
||
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
Reporter | ||
Comment 2•7 years ago
|
||
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 → ---
Assignee | ||
Comment 3•7 years ago
|
||
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
Comment 4•7 years ago
|
||
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?
Assignee | ||
Comment 5•7 years ago
|
||
(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 ago → 7 years ago
Resolution: --- → DUPLICATE
Reporter | ||
Comment 6•7 years ago
|
||
This is likely to be fixed as part of the work in bug 1452073 and/or bug 1448885.
Updated•6 years ago
|
Product: Toolkit → WebExtensions
You need to log in
before you can comment on or make changes to this bug.
Description
•