Last Comment Bug 240760 - cmd-F (find dialog) should focus quicksearch when organizing bookmarks
: cmd-F (find dialog) should focus quicksearch when organizing bookmarks
Status: RESOLVED FIXED
: access, fixed1.8.1.1
Product: Camino Graveyard
Classification: Graveyard
Component: Bookmarks (show other bugs)
: unspecified
: PowerPC Mac OS X
-- normal (vote)
: Camino2.0
Assigned To: froodian (Ian Leue)
:
:
Mentors:
: 258973 301652 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-04-16 19:38 PDT by Dave Aton
Modified: 2006-12-18 11:38 PST (History)
7 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch (7.57 KB, patch)
2006-11-09 17:32 PST, froodian (Ian Leue)
no flags Details | Diff | Splinter Review
Better (6.42 KB, patch)
2006-11-10 09:10 PST, froodian (Ian Leue)
stuart.morgan+bugzilla: review-
Details | Diff | Splinter Review
Patch (8.00 KB, patch)
2006-12-12 19:40 PST, froodian (Ian Leue)
stuart.morgan+bugzilla: review+
mikepinkerton: superreview+
Details | Diff | Splinter Review

Description User image Dave Aton 2004-04-16 19:38:40 PDT
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.
Comment 1 User image Greg K. 2004-04-17 00:30:30 PDT
Confirmed using Camino/2004040108. Find command operates on the page beneath the
Bookmarks editor, not the Bookmarks themselves.
Comment 2 User image Jasper 2004-04-17 05:57:46 PDT
Yup we shouldn't allwo suers to do a find in the bookmarks manager.
Comment 3 User image Dave Aton 2004-04-17 06:33:43 PDT
(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.
Comment 4 User image Mike Pinkerton (not reading bugmail) 2004-04-19 07:25:31 PDT
we should disable find when the manager is present, search is done with the
search field at the bottom of the manager.
Comment 5 User image Dave Aton 2004-04-19 14:02:43 PDT
(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.")
Comment 6 User image Smokey Ardisson (offline for a while; not following bugs - do not email) 2005-07-06 00:43:33 PDT
*** Bug 258973 has been marked as a duplicate of this bug. ***
Comment 7 User image Smokey Ardisson (offline for a while; not following bugs - do not email) 2005-07-06 00:49:16 PDT
In bug 258973 comment 4, Simon says: Yeah, I vote for just focussing the
quicksearch field.

Tidying summary, too.
Comment 8 User image Simon Fraser 2005-07-06 08:50:30 PDT
(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?
Comment 9 User image Smokey Ardisson (offline for a while; not following bugs - do not email) 2005-07-21 22:14:35 PDT
*** Bug 301652 has been marked as a duplicate of this bug. ***
Comment 10 User image Smokey Ardisson (offline for a while; not following bugs - do not email) 2005-07-21 22:23:04 PDT
(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).
Comment 11 User image Mike Pinkerton (not reading bugmail) 2005-12-11 10:06:46 PST
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.
Comment 12 User image Martin Girschick 2006-04-13 09:31:11 PDT
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.
Comment 13 User image froodian (Ian Leue) 2006-11-09 17:32:46 PST
Created attachment 245165 [details] [diff] [review]
Patch

This patch may be a bit controversial... but I like it.  The find panel is useless in the bm manager anyway.
Comment 14 User image froodian (Ian Leue) 2006-11-10 09:10:46 PST
Created attachment 245228 [details] [diff] [review]
Better

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 15 User image Stuart Morgan 2006-11-18 15:37:18 PST
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.
Comment 16 User image froodian (Ian Leue) 2006-12-12 19:40:52 PST
Created attachment 248480 [details] [diff] [review]
Patch

Addresses review comments.
Comment 17 User image Stuart Morgan 2006-12-16 10:47:43 PST
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 18 User image Mike Pinkerton (not reading bugmail) 2006-12-18 09:48:05 PST
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.
Comment 19 User image froodian (Ian Leue) 2006-12-18 11:38:21 PST
Checked in on trunk and MOZILLA_1_8_BRANCH.  Filed followup work as a single bug (bug 364255).

Note You need to log in before you can comment on or make changes to this bug.