Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9b2pre) Gecko/2007112805 Minefield/3.0b2pre - Build ID: 2007112805 1. Go to History > Show all history... 2. You have to perform a bookmarks search in the upper right corner to be able to change the search to "history". Expected result: 1. Search box in upper right corner shows "history" and searches history. 2. In general a way to access the extended search without performing a search.

See https://bugzilla.mozilla.org/show_bug.cgi?id=378798

IMO, this is a bug. Yes, the search field says "Search bookmarks." But I've selected my history window. I see a bunch of history in the window. Why would typing in the search box search an entirely different set of data? The right way to fix this is probably to explicitly show the History source as being selected in the LHS source bar, and to automatically update the search bar to "Search in <foo>" where <foo> is the currently selected source.

this should be really fixed with a unified search in Bug 378798, i suggest duping to that

Requesting blocking as this is going to be a glaring mistake if not fixed. There seems to be bugs pointing back and forth to should be 'fixed' with.. but that in turn points somewhere else. If there is no intent to correct the issue of opening 'Show all History' and getting a search box that says 'search in history' and provide the correct 'View Menu' options related to History, not the bookmarks View Menu as per the mockup here: http://people.mozilla.com/~faaborg/files/granParadisoUI/places_OrganizerHistory_i5.png Then I would suggest perhaps that 'History' be removed/hidden in the Library and in the Application/Browser Window : History->Show All History be changed from opening the Library to merely opening the Side-bar.

Some discussion here: http://forums.mozillazine.org/viewtopic.php?p=3290126#3290126

Morphing. The right solution is that the Library search should default to "Selected Folder". This will also result in the grey text in that field updating correctly. When a user selects "Show all History" that folder will be selected and the searchbox will say "Search in 'History'". (In reply to comment #4) > Then I would suggest perhaps that 'History' be removed/hidden in the Library > and in the Application/Browser Window : History->Show All History be changed > from opening the Library to merely opening the Side-bar. Absolutely not.

this fixes the scopeBar style, i've removed bookmarks menu and bookmarks toolbar scopes, you can see the three states (selected, hover, unselected), so could potentially fix Bug 425854 and Bug 426148

The Save button is still styled oddly, otherwise I think it looks good. Waiting on beltzner though.

yes the save button is not so cool, if someone has a suggestion for that, hwv could be addressed later

Does this default to the selected top level folder (bookmarks, tags, history) or literally the selected folder? If you are in a folder with then bookmarks, doing a search just over those ten bookmarks is kind of silly since you can see all of them. The screen shot of searching just the bookmarks toolbar folder has me concerned.

The text in the search box currently uses the searchBookmarks string ("Search Bookmarks"), so it makes sense to have a searchHistory string ("Search History") to display when the history folder is open.

(In reply to comment #13) > Does this default to the selected top level folder (bookmarks, tags, history) > or literally the selected folder? if you're in a history container searches into full history. in Tags or all Bookmarks search full bookmarks (toolbar, menu, unfiled). in a valid Bookmarks folder searches in that folder, the user can then click "All Bookmarks" in the scope bar to move to full bookmarks search. (In reply to comment #14) > Created an attachment (id=313281) [details] > Add searchHistory string > > The text in the search box currently uses the searchBookmarks string ("Search > Bookmarks"), so it makes sense to have a searchHistory string ("Search > History") to display when the history folder is open. It does already "Search in 'History' using the title of the top history container, so actually a new string is not needed, It's not the same as Search Bookmarks but it's quite similar imo

(In reply to comment #15) > It does already "Search in 'History' using the title of the top history > container, so actually a new string is not needed, It's not the same as Search > Bookmarks but it's quite similar imo I think "Search History" looks much better than "Search in 'History'", so I do think it is needed and matters.

Comment on attachment 313281 [details] [diff] [review] Add searchHistory string Yeah, I agree, we might as well polish this up.

ok, i'll merge Reed string change in my patch

we can check-in reed string change. But if this patch is ok and mano has enough time to review we could check in the full patch. I'll be able to address comments in 3 hours if needed

Aren't some files included twice in your patch?

string: Checking in browser/locales/en-US/chrome/browser/places/places.properties; /cvsroot/mozilla/browser/locales/en-US/chrome/browser/places/places.properties,v <-- places.properties new revision: 1.40; previous revision: 1.39 done

Comment on attachment 313395 [details] [diff] [review] patch And on the next patch, please avoid calling asQuery multiple times

(In reply to comment #20) > Aren't some files included twice in your patch? oh damn, i included the path twice when generating :/ sorry

Looks good, what about updating winstripe & pinstripe though?

(In reply to comment #25) > Looks good, what about updating winstripe & pinstripe though? gnomestripe and pinstripe, you mean?

pinstripe has been already fixed some time ago, has a round style (i don't have a mac to check but stylesheet appear good)... winstripe has the oldest Library style between the three (has not been updated in a long time). will take a look in ubuntu for gnomestripe

gnomestripe looks already good to me

Comment on attachment 313501 [details] [diff] [review] patch >Index: toolkit/components/places/src/utils.js >=================================================================== >RCS file: /cvsroot/mozilla/toolkit/components/places/src/utils.js,v >retrieving revision 1.12 >diff -u -8 -p -r1.12 utils.js >--- toolkit/components/places/src/utils.js 3 Apr 2008 21:20:39 -0000 1.12 >+++ toolkit/components/places/src/utils.js 4 Apr 2008 00:36:48 -0000 >@@ -298,16 +298,33 @@ var PlacesUtils = { > Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER_SHORTCUT, > Ci.nsINavHistoryResultNode.RESULT_TYPE_QUERY, > Ci.nsINavHistoryResultNode.RESULT_TYPE_DYNAMIC_CONTAINER], > nodeIsContainer: function PU_nodeIsContainer(aNode) { > return this.containerTypes.indexOf(aNode.type) != -1; > }, > > /** >+ * Determines whether or not a ResultNode is an history related container. nit: a result-node >Index: browser/components/places/content/places.js >=================================================================== >RCS file: /cvsroot/mozilla/browser/components/places/content/places.js,v >retrieving revision 1.152 >diff -u -8 -p -r1.152 places.js >--- browser/components/places/content/places.js 3 Apr 2008 20:51:54 -0000 1.152 >+++ browser/components/places/content/places.js 4 Apr 2008 00:36:50 -0000 >@@ -218,24 +218,71 @@ var PlacesOrganizer = { > > // Make sure the search UI is hidden. > PlacesSearchBox.hideSearchUI(); > if (resetSearchBox) { > var searchFilter = document.getElementById("searchFilter"); > searchFilter.reset(); > } > >- // Update the "Find in <current collection>" command and the gray text in >- // the search box in the toolbar if the active collection is the current >- // collection. >+ this._setSearchScopeForNode(node); >+ }, >+ >+ /** >+ * set the search scope based on node properties mega-nit: Sets...the node's properties >+ * @param aNode >+ * the node to set up scope for hrm, s/for/from? >+ */ >+ _setSearchScopeForNode: function PO__setScopeForNode(aNode) { Pur aNode.itemId in a variable. Plus the pinstripe changes we discussed over irc, which is the only reason for denying. ;)

fixed review comments

Comment on attachment 313803 [details] [diff] [review] patch I'm just curious, why do we disable this button rather than hiding it? This is supposed to work like a context menu IMO.

maybe a user could blind-click the space taken by a button, knowing its position... we disable options in context menus for the same kind of pointed node, instead of hiding them...

Comment on attachment 313803 [details] [diff] [review] patch r=mano, but pinstripe my need futher tweaks for disabled appearance (use graytext for the label, we might just do that already).

disabled appearance should be already set up by default on a toolbarbutton on pinstripe, still i don't have a Mac to tweak that further and to be sure of that... So please, if after this check-in something is wrong in the disabled state on pinstripe, file a followup blocking this.

Checking in browser/components/places/content/places.js; /cvsroot/mozilla/browser/components/places/content/places.js,v <-- places.js new revision: 1.153; previous revision: 1.152 done Checking in browser/components/places/content/places.xul; /cvsroot/mozilla/browser/components/places/content/places.xul,v <-- places.xul new revision: 1.128; previous revision: 1.127 done Checking in browser/themes/pinstripe/browser/places/organizer.css; /cvsroot/mozilla/browser/themes/pinstripe/browser/places/organizer.css,v <-- organizer.css new revision: 1.9; previous revision: 1.8 done Checking in browser/themes/winstripe/browser/places/organizer.css; /cvsroot/mozilla/browser/themes/winstripe/browser/places/organizer.css,v <-- organizer.css new revision: 1.7; previous revision: 1.6 done Checking in toolkit/components/places/src/utils.js; /cvsroot/mozilla/toolkit/components/places/src/utils.js,v <-- utils.js new revision: 1.13; previous revision: 1.12 done

I tested the fix with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9pre) Gecko Minefield/3.0pre. Seems to work fine. But one question I have: Should we use "Search Tags" as emtpy text for the search field if the user selects the Tag container inside the left pane?

no, not for now at least, searching into tags is quite complex and will become more complex when we will change how tag works... so for now we should search in all bookmarks when Tags is selected

PS: however file a followup on search in Tags, for future reference

verified with: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9pre) Gecko/2008050704 Minefield/3.0pre

(In reply to comment #37) > more complex when we will change how tag works... so for now we should search > in all bookmarks when Tags is selected That doesn't work at the moment. I've filed bug 432778 therefor. (In reply to comment #38) > PS: however file a followup on search in Tags, for future reference This is bug 432779 now.

Test cases were created for 3.1 and 3.0 test runs on litmus for regression testing. For 3.0, https://litmus.mozilla.org/show_test.cgi?id=7528 For 3.1, https://litmus.mozilla.org/show_test.cgi?id=7529

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