388860 - New folder button on Add Bookmark panel does nothing

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[cmtalbert]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a7pre) Gecko/2007071904 Minefield/3.0a7pre
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a7pre) Gecko/2007071904 Minefield/3.0a7pre

Clicking the "New Folder" button on the "Add Bookmarks" panel does nothing.

I have confirmed this bug also on Windows XP: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007071905 Minefield/3.0a7pre

The odd thing about this is that no exception is thrown in the Error Console, or even in the console spew on a debug build.



Reproducible: Always

Steps to Reproduce:
1. Go to a page in Minefield
2. Click Bookmarks -> Bookmark This Page
3. Click the drop down arrow 
4. Click on "New Folder"

Actual Results:  
No folder is created, regardless of where in the current tree you are selected.

Expected Results:  
Folder should be created

== Regression Range == 
Works: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007071008 Minefield/3.0a7pre

Breaks: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007071105 Minefield/3.0a7pre

Bonsai query for this range (on the entire tree)
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=mozilla%2F&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-07-10+00%3A00%3A00&maxdate=2007-07-11+13%3A00%3A00&cvsroot=%2Fcvsroot

Comment 1

[Jim Jeffery not reading bug-mail 1/2/11]

Broken also on Windows:

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a7pre) Gecko/2007071905 Minefield/3.0a7pre Firefox/3.0 ID:2007071905

Comment 2

[(not reading, please use [e-mail] instead)]

I think this is something I broke, with my fix for bug #385990

Comment 3

[(not reading, please use [e-mail] instead)]

Attachment: fix

avoid my history query optimization by specifying queryType=1 (QUERY_TYPE_BOOKMARKS)

Comment 4

[Dietrich Ayala (:dietrich)]

Comment on attachment 273053 [details] [diff] [review]
fix

hrm, we should probably recommend that queryType always be used for bookmarks queries now.

Comment 5

[(not reading, please use [e-mail] instead)]

> hrm, we should probably recommend that queryType always be used for bookmarks
> queries now.

I've added an assert so that we'll catch this problem next time, and it won't be silent.

I'll attach that patch.

Comment 6

[(not reading, please use [e-mail] instead)]

Attachment: patch + assert

carrying over dietrich review

Comment 7

[(not reading, please use [e-mail] instead)]

fixed.

Checking in content/bookmarkProperties.js;
/cvsroot/mozilla/browser/components/places/content/bookmarkProperties.js,v  <--
 bookmarkProperties.js
new revision: 1.56; previous revision: 1.55
done
Checking in content/bookmarkProperties.xul;
/cvsroot/mozilla/browser/components/places/content/bookmarkProperties.xul,v  <--
  bookmarkProperties.xul
new revision: 1.27; previous revision: 1.26
done
Checking in content/tree.xml;
/cvsroot/mozilla/browser/components/places/content/tree.xml,v  <--  tree.xml
new revision: 1.79; previous revision: 1.78
done

Comment 8

[(not reading, please use [e-mail] instead)]

re-opening, I backed out because the tree was closed.

Comment 9

[Asaf Romano (gone)]

I don't think this is right, queryType should be ignored for simple folder queries. I would rather make getinsertionpoint check queryType only if the root is a query node (nodeIsQuery).

Comment 10

[(not reading, please use [e-mail] instead)]

Attachment: patch, per mano (plus some cleanup)

Comment 11

[Asaf Romano (gone)]

Comment on attachment 273172 [details] [diff] [review]
patch, per mano (plus some cleanup)

r=mano

Comment 12

[(not reading, please use [e-mail] instead)]

fixed.

Checking in content/bookmarkProperties.js;
/cvsroot/mozilla/browser/components/places/content/bookmarkProperties.js,v  <--
 bookmarkProperties.js
new revision: 1.58; previous revision: 1.57
done
Checking in content/tree.xml;
/cvsroot/mozilla/browser/components/places/content/tree.xml,v  <--  tree.xml
new revision: 1.81; previous revision: 1.80
done

Comment 13

[Carsten Book [:Tomcat]]

Litmus Triage Team: cc'ing al and tracy for the in-litmus flag

Comment 14

[Gervase Markham [:gerv]]

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