User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2008020204 Minefield/3.0b3pre Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2008020204 Minefield/3.0b3pre All tags associated with a bookmark are removed when moving an item from the root of the Bookmarks Menu folder to another folder. Reproducible: Always Steps to Reproduce: 1. Create a bookmark in the Bookmark Menu folder 2. Add some tags to the bookmark 3. Create another folder within the Bookmark Menu folder 4. Move the bookmark to the folder which was just created. Actual Results: Bookmark is moved without the given tags Expected Results: Bookmark is moved with the given tags Experienced this issue when moving items using Cut and Paste. Not sure if this happens in other situations (Drag and drop doesn't seem to work in this build aswell)
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2008020223 Minefield/3.0b3pre At the moment he only way to drag a bookmark into a folder is in the sidebar BTW. No problem migrating the tag. The problem occurs only while trying to cut & paste.
Created attachment 310080 [details] [diff] [review] fix de/serializes tags as a property of the bookmark. this has a few benefits: 1. complete partial serializations: tags are carried around w/ the item they describe. 2. backup size: currently we serialize the tag root, where a uri is serialized once for for each tag it has. this means that a bookmark with 3 tags is saved in the backup as 4 bookmarks, inflating the size of the backup, and slowing down import and export. 3. restore perf: instead of tagging the URI once for each tag, we can now make one call to the tag service for all a uri's tags. maybe a few less db queries, but certainly less xpconnect traversal. notes: 1. adds exclude filtering for json export, which could be useful to extension consumers using these methods for sync, etc. 2. older backups since json landed will still work, since we still handle import of the tag root, if present.
Comment on attachment 310080 [details] [diff] [review] fix not ready, need to adjust index values
Created attachment 310114 [details] [diff] [review] fix
Comment on attachment 310114 [details] [diff] [review] fix Dietrich, doesn't this cause us to remove tags as you *redo* item removal? That can be fixed by only setting the tags property if you're removing the last bookmark for the bookmark's url.
(In reply to comment #7) > (From update of attachment 310114 [details] [diff] [review]) > Dietrich, doesn't this cause us to remove tags as you *redo* item removal? That > can be fixed by only setting the tags property if you're removing the last > bookmark for the bookmark's url. > No it doesn't cause tags to be removed when doing redo item removal. The child transactions (adding the tags) are executed yet again.
I still don't think it's right. We should export the tags-root *as-is* and don't re-tag after restoring from backup.
Per irc, i looked at only serializing tags inline for cut. However, it suffers from the same problem: Annotations on tag items will be lost. Our current approach towards tags in the transaction layer has the same problem. Basically any tag removal via the tagging API incurs the loss of annotations on the "tag bookmarks". I think we need to document that the tag root is "special internal use", and can be modified out-of-band, so extensions should not use it directly. Use the tagging API instead. And in 3.1 we can fix bug 424160, moving tags out of the bookmark tree.
Dietrich, this isn't just annotations, this is alsp a speed issue and may lead to different Tags Root in certain cases, I'm sure. I would really like not to change our current, pretty cheap, solution with something that heavy.
(In reply to comment #11) > Dietrich, this isn't just annotations, this is alsp a speed issue and may lead > to different Tags Root in certain cases, I'm sure. I would really like not to > change our current, pretty cheap, solution with something that heavy. > I'm ok with backup/restore the whole root for now. My point is that regardless of how we fix this bug, we still need to discourage extension developers from annotating tag bookmarks, due to the dataloss problem. How could this lead to a different tags root?
Created attachment 311851 [details] [diff] [review] fix v3 - always backup tag root - always restore tag root if present - only inline tags for copy/drag - always restore inline tags if present
I would rename aInlineTags to something more generic, there are few more places i would use that info. Maybe aIsUICommand?
+ var tags = aData.tags.split(", "); + childTxns.push(this.ptm.tagURI(itemURL, tags)); We should filter the incoming tags list here, otherwise undoing this transaction will remove old tags as well.
Created attachment 311864 [details] [diff] [review] fix v4 addresses comments 14 and 15.
I don't understand how's that addressing comment 15. basically, here's my scenraio: 1. have two bookmarks a, b for the same uri, set with some tags 2. copy&paste b 3. undo paste b Are the tags for a still set?
Created attachment 311911 [details] [diff] [review] fix v5 sorry, my misunderstanding. this adds filtering to properly handle that case.
Comment on attachment 311911 [details] [diff] [review] fix v5 r=mano
Checking in browser/components/places/content/utils.js; /cvsroot/mozilla/browser/components/places/content/utils.js,v <-- utils.js new revision: 1.128; previous revision: 1.127 done Checking in toolkit/components/places/src/utils.js; /cvsroot/mozilla/toolkit/components/places/src/utils.js,v <-- utils.js new revision: 1.8; previous revision: 1.7 done
Verified with: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/20080429006 Minefield/3.0pre
Test case https://litmus.mozilla.org/show_test.cgi?id=7478 has been created in litmus for regression testing.
(In reply to comment #22) > Test case https://litmus.mozilla.org/show_test.cgi?id=7478 has been created in > litmus for regression testing. Aakash, could you please also add it to the 3.0 testrun, since it has been fixed for Firefox 3? It would be great.
Sure, created test case https://litmus.mozilla.org/show_test.cgi?id=7479 for rel3.0 regression testing
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h". In Thunderbird 3.0b, you do that as follows: Tools | Message Filters Make sure the correct account is selected. Click "New" Conditions: Body contains places-to-b-and-h Change the action to "Delete Message". Select "Manually Run" from the dropdown at the top. Click OK. Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter. Gerv
Bug present in FF6 !
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0) Gecko/20100101 Firefox/6.0 This issue indeed seems to have been regressed into the latest release version of Firefox 6.
another bug has already been filed on the new issue, correctly, this should never ever be reopened.