This is a follow up for bug 285631. The new template builder has a few issues with the bookmarks window. Specifically: - sorting doesn't work - dragging causes the bookmarks in the Bookmarks menu to become messed up - bookmark folder icons disappear Opening a new window is a workaround. I already have a patch for this which I will supply soon.

fixes: - menu duplication and oddities that occur when dragging bookmarks. This is caused by some confusion over how the 'parent' feature on rules work. This patch distinguishes between the tag set on the query and the rules. This ensures that an item matches only one of two rules that only differ only by the 'parent' attribute. - also fixes a leak caused by the above - the changes for the above also allow rejecting parent rules earlier for a slight optimization - fix bookmark folder icons by setting the container attributge on the right element - bookmark window sorting was fixed by the other template builder patch

I noticed a regression from Seamonkey BuildId 1.9a1: 2006021208 to 1.9a1: 2006021309 If I drag the Favicon or a tab to a folder in my Personal Toolbar, the folder opens, I position and drop the icon, the folder menu closes, the folders icon changes from folder to bookmark, I cannot open this folder again for the rest of the session. Build data from tinderbox Seamonkey: Started 02/12 08:15, finished 02/12 10:21 2 hours, 6 minutes Started 02/13 08:40, finished 02/13 10:57 2 hours, 17 minutes http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2006-02-12+07%3A00&maxdate=2006-02-13+11%3A00&cvsroot=%2Fcvsroot This regression may be younger than this bug, but it may be fixed by this one.

Comment on attachment 212005 [details] [diff] [review] fix bookmarks issues >Index: content/xul/templates/src/nsXULContentBuilder.cpp > virtual PRBool > MayGenerateResult(nsIXULTemplateResult* aOldResult, >- void** aLocation); >+ nsIContent** aLocation); Document clearly that this is NOT an XPCOM getter (That is, that the returned aLocation does not get addreffed. In fact, it might be a better idea to return a struct that contains a PRBool and nsIContent* or something; that would make it clear what the ownership model is. >@@ -1231,20 +1231,25 @@ nsXULContentBuilder::CreateContainerCont >+ if (tag && tag != aElement->Tag()) Namespace of aElement doesn't matter? This looks like food for a followup bug to me! >Index: content/xul/templates/src/nsXULTemplateBuilder.cpp > nsXULTemplateBuilder::CompileConditions(nsTemplateRule* aRule, >+ if (!tag.IsEmpty()) >+ aRule->SetTag(NS_NewAtom(tag)); That leaks. Please fix. r=bzbarsky with that.

bz, do I need sr for this and other template bugs as well?

I think it's safe to call this an r+sr... helps to cc me when asking questions, though. ;)

Seems like this may have caused bug 328496.

Btw, the fix for this bug was checked in at 2006-02-23 10:53.