User-Agent: Mozilla/5.0 (000000000; 0; 000 000 00 0 000000; 00000; 0000000) 00000000000000 00000000000 Build Identifier: 2004041608 (v0.7+) Downloaded and installed Camino nightly build. Started up. Opened Bookmarks. Opened bookmark group Mac Software using triangle, and saw VersionTracker. Pressed Command-F and entered "version" in dialog box (without quotes), and clicked Find Next button. Did not display the VersionTracker bookmark. Reproducible: Always Steps to Reproduce: 1. See Details section above. 2. 3. Actual Results: No match displayed. Expected Results: Showed match on "VersionTracker" bookmark.

Confirmed using Camino/2004040108. Find command operates on the page beneath the Bookmarks editor, not the Bookmarks themselves.

Yup we shouldn't allwo suers to do a find in the bookmarks manager.

(In reply to comment #2) > Yup we shouldn't allwo suers to do a find in the bookmarks manager. I disagree. Camino should behave like Mozilla. Many users have hundreds of bookmarks acquired over many years, or bookmarks in different folders that have something in common. Being able to find and display those in a separate window is essential. For me, it's a deal-breaker when considering a switch from Mozilla to Camino.

we should disable find when the manager is present, search is done with the search field at the bottom of the manager.

(In reply to comment #4) > we should disable find when the manager is present, search is done with the > search field at the bottom of the manager. IMO, a more intuitive would be to make Find open the search field and move the cursor there, but most would be just to let the user use the Find command the way s/he probably thinks is normal. (I don't think the average user distingushes between "Find" and "Search.")

*** Bug 258973 has been marked as a duplicate of this bug. ***

In bug 258973 comment 4, Simon says: Yeah, I vote for just focussing the quicksearch field. Tidying summary, too.

(In reply to comment #7) > In bug 258973 comment 4, Simon says: Yeah, I vote for just focussing the > quicksearch field. But what do you do if the Find panel is already showing? Should we hide it when switching to bookmarks, and reshow when we leave?

*** Bug 301652 has been marked as a duplicate of this bug. ***

(In reply to comment #8) > But what do you do if the Find panel is already showing? Should we hide it when > switching to bookmarks, and reshow when we leave? At first thought that seems wacky, but there's precedent of sorts with the View Options inspector in the Finder and the Keyboard Viewer, Character Palette, and Font panel elsewhere in the OS (they hide when switching to apps that have not called them and reappear when switching back to the original apps).

this is a good idea, but the code would require a lot of changes to do it right (firstResponder, etc) and we still don't have a good answer for what happens when the find dialog is already present.

If the find window is replaced by an "in window" search bar with the same functionality the described issue won't be a problem anymore. See #258211.

This patch may be a bit controversial... but I like it. The find panel is useless in the bm manager anyway.

This just gets rid of the find dialog when the bookmark manager comes up, not when we call the IBActions for opening it. It's the Right Way, and works if you type "about:bookmarks" (which the previous patch didn't).

Comment on attachment 245228 [details] [diff] [review] Better >+ if (mFindDialog != nil) >+ [mFindDialog close]; The |if| check here doesn't do anything useful. >-// XXX this is only used to show bm after an import I think the reason this comment was here is that it's kind of a hack. Exposing internals like this isn't generally a good idea, and it's something I want to clean up in Camino's code, so adding more would make me sad. Instead, how about having MC call a method in the frontmost BWC (if any) to give it a chance to handle Find if it wants to, which returns a BOOL for whether or not it did. That gives an extensible way for BWC to do content-specific things with Find without MC having to know the details.

Addresses review comments.

Comment on attachment 248480 [details] [diff] [review] Patch r=me, this is much cleaner, thanks. Please document performFindCommand to explain that the return value is whether or not it was handled though. Also, we need a follow-up bug or two on making the find panel behavior better: - if you switch windows or tabs to an existing bookmark manager, it stays showing - if you toggle on the bookmark manager, causing the find panel to vanish, then toggle it off again, the find panel should come back. Despite that, I still think this behavior is still an improvement over where we are now, so I'm in favor of landing this now and addressing those issues later.

Comment on attachment 248480 [details] [diff] [review] Patch sr=pink regarding performFindCommand, it seems like it should have a better name, but i can't think of one. Definitely needs to be documented.

Checked in on trunk and MOZILLA_1_8_BRANCH. Filed followup work as a single bug (bug 364255).