337654 - Find (ctrl-f) and FAYT (/ or ') should be visually distinct

Status: VERIFIED FIXED

(Toolkit :: Find Toolbar, defect)

Description

[Peter Kasting]

As per discussion on dev.apps.firefox, the ctrl-f and / find modes should be visually distinct.  A quick hack to do this would be to remove most of the find bar's controls and change its label in FAYT mode.  A probably-better fix would be to make FAYT spawn a more lightweight control, like a small box containing only a textfield, in the lower corner of the page.

Comment 1

[Mike Beltzner [:beltzner, not reading bugmail]]

Something like:

.----------------------------------------------------------...
|Jump to: (O._foo___________________)
'----------------------------------------------------------...

Would work well, I think. A textbox covering a portion of the page would be hard, and by the time we could get it, we might be able to get one that just fades into the background (yay, cairo!)

Comment 2

[Peter Kasting]

So making the find bar not take up the whole width of the window is really difficult?  Weird.

I'm not sure about "jump to"... as a user, what does that mean?  It sounds like some special function I'm uncertain of.  I use / for "find"ing :(  Not that I have any better suggestions... "Incremental find" and "find as you type", but those are both long and would just as aptly describe ctrl-f.

Comment 3

[Mike Connor [:mconnor]]

ask annie/ben about textbox-in-popup pain.  Right now its just a toolbar that can be hidden easily.

Comment 4

[Peter Kasting]

So I don't forget, there was some discussion almost a month ago of other possible text labels for FAYT mode, and besides "Jump To", two other suggestions were "Look For" and "Quick Find", the last of which I think is the best option.

Comment 5

[Jeff Walden [:Waldo]]

Attachment: Implement a "Quick Find:" bar

This patch simply turns off all chrome in the find bar except for the label and the textbox when doing a TAF search.  Screenshot in a sec...

Comment 6

[Jeff Walden [:Waldo]]

Attachment: Screenshot

Comment 7

[Peter Kasting]

Looks good.  Only question I'd have is whether the close button should still appear.

Incidentally, I did ask a few people here about textboxes in popups.  Apparently it works perfectly fine (even in 2.0) as long as you set "ignorekeys" to true.  So technically it doesn't seem like it'd be hard to pull that off.  If it's doable I think that UI would be even better, especially since it wouldn't cause the page to resize vertically like the current find bar does.

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 224582 [details]
Screenshot

I think we're almost there, but I don't want to make the "Find Next" case completely undiscoverable. Waldo made a great point in IRC, that with the FAYT case, the toolbar often disappears before the user can mouse to it, and we've got an obvious indicator that the user is likely a keyboard-shortcut-junkie.

So, maybe what we should do is add labels in the FAYT case that call out the shortcut for "Find Next" ie, add a label (not a button) after the textbox: "(Next: Ctrl+G)"   or "(Next: ⌘+G)" for OSX.

pkasting: thoughts here, since you're one of the drivers of this?

Comment 9

[Jeff Walden [:Waldo]]

(In reply to comment #8)
> So, maybe what we should do is add labels in the FAYT case that call out the
> shortcut for "Find Next" ie, add a label (not a button) after the textbox:
> "(Next: Ctrl+G)"   or "(Next: ⌘+G)" for OSX.

It may be worth briefly considering whether to display accel+G or simply F3.  F3 might be easier to hit (and would mean one less string to translate), but since it's not exposed in UI (except in help docs) it may be better to just show what's already shown elsewhere.

Comment 10

[Peter Kasting]

mconnor's original description of the use cases for Quick Find (in the newsgroups) suggested to me that this is designed for people who are trying to jump to a particular point in the page, or doing something else where generally they're trying to search once.  If that's the case, I think we should not add labels about searching again.  The Find bar already provides good UI for doing repeated searches, the Edit menu contains a keyboard shortcut for finding again, hitting ctrl-g won't bring the Quick Find bar back up, we're trying to make the UI for Quick Find lightweight, etc.  I would prefer to not have this, and say that people who wish to search again AND don't know the keyboard shortcuts should use normal Find.

I don't have a log of Waldo's point in IRC, so I don't really understand it.  Why do we care about users mousing to the Quick Find bar?  I thought the whole point was that it wasn't mouse-driven?  Someone fill me in...

Comment 11

[Jeff Walden [:Waldo]]

(In reply to comment #10)
> I don't have a log of Waldo's point in IRC, so I don't really understand it. 
> Why do we care about users mousing to the Quick Find bar?  I thought the whole
> point was that it wasn't mouse-driven?  Someone fill me in...

It's not a common activity, but it happens.

Say you're TAFing for something because you want to find something, and then you either realize that you want to highlight search instances or match case.  (I think I usually hit this with LXR, where most code is case-sensitive, but I've never remembered my temporary annoyance long enough to file a bug.)  You're in TAF thought mode, so you're not thinking in terms of underlined accesskeys displayed on the find bar (and even if you were, it takes a particularly special breed of keyboard user to remember non-menu underlined accesskeys without looking at them).  You reach for the mouse to move down to check the box, but the find bar times out before you can get to it (or comes close enough that you have to move quickly to hit the box).  This is particularly bad if you're using a laptop with a touch pad.  If you do manage to get to it, moving back to the keyboard to refine the search or search further is just as time-consuming, and you're just as likely to hit the timeout.  Essentially, the timeout means that the find bar is really only usable from the keyboard in TAF mode, so you might as well remove the extra widgetry that's from a practical standpoint unusable in TAF mode.

Comment 12

[Peter Kasting]

OK, that sounds like an argument for doing exactly what we've just done on this patch, which is to remove most of the UI from the Quick Find bar.  So... was there a problem?  What change are you driving at with that argument?  Sounded like beltzner was giving it as a justification for adding a label for Find Next, but that doesn't sound like where you were going.

Comment 13

[Jeff Walden [:Waldo]]

(In reply to comment #12)
> OK, that sounds like an argument for doing exactly what we've just done on this
> patch, which is to remove most of the UI from the Quick Find bar.  So... was
> there a problem?  What change are you driving at with that argument?  Sounded
> like beltzner was giving it as a justification for adding a label for Find
> Next, but that doesn't sound like where you were going.

I think he was driving at that, too (that a TAF user is a keyboard user and thus might appreciate the shortcut reminder).  I wasn't really driving at anything with the comment -- the initial context in which I mentioned it was giving my rationalization for why removing UI from the toolbar in TAF mode was a good idea.

Comment 14

[Peter Kasting]

OK, I understand, thanks.

Since we don't seem to document the / or ' behavior anywhere user-visible, I think anyone who hits this feature is probably either capable of figuring out ctrl-g on their own, or is very angry that that dang bar came up again while they were typing in some textbox.  Neither case is one that seems to justify putting more text on the bar.  The second case makes me wonder if Quick Find should be disabled by default and only on with a pref :)

Comment 15

[Jeff Walden [:Waldo]]

(In reply to comment #14)
> Since we don't seem to document the / or ' behavior anywhere user-visible,

(see Keyboard Shortcuts in help docs, but I don't think it's mentioned anywhere else in help docs)

> The second case makes me wonder if Quick Find should be disabled by default
> and only on with a pref :)

Options>Advanced>General>Begin finding when you begin typing (disabled by default), but other than that it's completely hidden.

So anyway, is the keyboard shortcut desired or not?  I have it implemented and can post the patch, but thinking about it more has convinced me that it's probably overkill for the type of user that's actually going to use TAF, and it's not clear to me that a decision has been reached on whether or not it's a good idea.

Comment 16

[Peter Kasting]

No no, "begin finding when you begin typing" removes the need to hit / or '.  I mean disabling FAYT/Quick Find altogether.  There isn't a pref to do that.  But that's getting sidetracked; this is the wrong bug for that discussion.

My opinion is that the current UI looks good and a label should not be added.  beltzner is the guy in charge.

BTW, if you remove the close button like you're currently doing, you should make sure there aren't situations where the user can "stick" the bar to the screen, because then they can't close it.  For example, what happens if you hit / and then ctrl-f?  Are there any other circumstances the timeout can get canceled?

Comment 17

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 224582 [details]
Screenshot

OK, pkasting's comments convinced me that the label isn't neccessary. Let's try it like this and see how we feel about it.

Also, on the topic of pkasting's latest point, I'd expect that if someone pressed "Accel-F" while the FAYT bar was up, it would be replaced with the full featured Find Bar.

Comment 18

[Mike Connor [:mconnor]]

Comment on attachment 224581 [details] [diff] [review]
Implement a "Quick Find:" bar

>+  /**
>+   * Opens and displays the find bar.
>+   *
>+   * @param show
>+   *   true if the find bar is to contain minimalist UI, false (or undefined, as
>+   *   happens if no arguments are provided) if the find bar is to display more
>+   *   generalized search UI

please align the description with the param name

@param foo
       true if bar should be...

show is pretty misleading, as a variable name.  set to true, we hide things, not show things.  showMinimalUI would be better.
 
>+    this.useFastFindUI(show);

this should be be updateFindUI, see below for question about why its necessary.

>+   * @param show
>+   *   true if minimalist UI should be enabled, false if general-purpose UI
>+   *   should be used
>+   */

same comment about better variable naming.

>+  useFastFindUI: function (show)
>+  {
>+    var findBar = document.getElementById("FindToolbar");
>+    for (var i = 0, sz = findBar.childNodes.length; i < sz; i++) {

why are we not just doing i < findbar.childNodes.length here?

is there a case where other consumers will want to directly call this, necessitating a separate function?

r=me with nits picked and questions answered

Comment 19

[Peter Kasting]

(In reply to comment #17)
> Also, on the topic of pkasting's latest point, I'd expect that if someone
> pressed "Accel-F" while the FAYT bar was up, it would be replaced with the full
> featured Find Bar.

That's what I would hope, too.  I simply hadn't tested it.  I wanted to make sure we were aware of the dangers if you ever couldn't get rid of the Quick Find bar, I'm not convinced it can actually happen.

Comment 20

[Jeff Walden [:Waldo]]

(In reply to comment #18)
> why are we not just doing i < findbar.childNodes.length here?

As mentioned on IRC, this may have to do extra work to re-resolve |findbar.childNodes| on every iteration; the work's probably negligible, but in the general case it may not be, so most recent code I've written uses this pattern on the off chance that calculating |sz| actually is costly.  Slight readability concerns and no real perf gain suggest that it's not really worth doing here, so I changed it to the usual pattern in the checked-in patch.

> is there a case where other consumers will want to directly call this,
> necessitating a separate function?

Personally, I'd rather see |openFindBar| be the only "public" way to change this.  It's not particularly difficult to call the method, anyway, so I think this will work even for users who want to access implementation methods like this.

Branch patch in a sec...

Comment 21

[Jeff Walden [:Waldo]]

Attachment: Branch patch

Comment 22

[Jeff Walden [:Waldo]]

(In reply to comment #16)
> For example, what happens if you hit / and then ctrl-f?  Are there any other
> circumstances the timeout can get canceled?

The timeout's never explicitly canceled except when a new one is created if the old one's not expired, and when it times out the find bar is hidden only if the current find mode isn't normal find (i.e., it's generic TAF or links-only TAF).  Otherwise, the bar stays, and because it was opened via the cmd_find-->gFindBar.onFindCmd() which calls |openFindBar(true)|, the close button will be displayed.

Comment 23

[Mike Beltzner [:beltzner, not reading bugmail]]

Waldo: if I search for "foo" using FAYT, and then Accel-F my way into the Find Bar, do we persist the search term ("foo") over into the new bar? I would expect that we do, but wanted to make sure ...

Comment 24

[Peter Lairo]

If a search term is not found, there should be a label that says so (e.g., "Phrase not found", like for Find). Just making the background red is not explicit enough.

I agree that that FAYT should be visually distinct from Find, but not by removing the Next, Previous and Highlight All buttons. Those are useful in FAYT too. 

The distinction could rather be:
 - the label (Find vs. Quick Find)
 - the absence of the close button in FAYT
 - the fact that the FAYT bar goes away after x seconds of inactivity.

BTW: It would be helpful if the Next and Previous buttons' tooltips included the keyboard shortcuts, including "F3": 
  Find the next occurrence of the phrase (Ctrl+G or F3)

Comment 25

[Stefan Breunig]

Agreed, removing next/prev/highlight is just stupid, finding one occurence and looking for the next one is quite common. Should we open a new bug for this, or is there already one?
xeen

Comment 26

[Peter Kasting]

The suggestion in comment #24 (to leave the buttons) is not visually distinct enough.  Frankly, even the current patch is not as distinct as I'd like, hence my textbox-in-popup suggestions.

If you are using FAYT instead of Find, and you want to find another match, using ctrl-g or F3 is the suggested method.  Relying on access to the buttons on a bar that disappears after a few seconds is poor.  If you need the buttons, why not use ctrl-f?  You don't even need to think ahead to use ctrl-f, just use FAYT as normal, and then if you realize you need the more heavyweight functionality of Find, hit ctrl-f and you get it.

I ask this because if there's a use case that really demands buttons in quick find mode and can't be solved using ctrl-f, we need to rethink the purpose of quick find altogether.

Comment 27

[Mike Beltzner [:beltzner, not reading bugmail]]

Interesting aside: FAYT was, I believe, initially created as an optimization for accessibility reasons, allowing sight-challenged user to quickly jump to text within a page. I stand with pkasting on his rationale: this is either a feature for this special-case user, or for the advanced keyboard user, neither of whom is likely to benefit from modifying the behaviour using the mouse. That use case is easily (and gracefully!) supported by Accel-F.

Comment 28

[Jeff Walden [:Waldo]]

Fixed on branch.

Comment 29

[Tracy Walker [:tracy]]

verified with Bon echo build from 0612 on Windows, marcia also comemnted seeing same on Mac

Comment 30

[tommyjb]

How about some visual differentiation between generic-find and link-only-find?  The lack thereof could easily confuse people.

Comment 31

[bugzilla.mozilla.org]

I wonder, with the prev/next buttons removed, why is there any toolbar at all for FAYT?

what sense does the text entry field make?

What I'm typing is highlighted on the page anyway and I can just continue typing right on the page - I do not need this toolbar at all - or do I?


btw, FAYT with options "start immediately, in all text" was the first thing that got me away from other browsers. Having to use any "toolbar" or "find dialog" just gets in the way of the content. And it's so great to navigate a page without using the mouse :)

Comment 32

[Peter Kasting]

(In reply to comment #31)
> I wonder, with the prev/next buttons removed, why is there any toolbar at all
> for FAYT?
> 
> what sense does the text entry field make?

To show you what you're entering and have matched on, and that you're in a different mode than normal.  Showing no entry box at all is far too visually confusing, especially in the case of finding no matches, where nothing at all is highlighted and you don't know what the browser is actually doing.

As you can see earlier in the bug, I'd really like an entry box that is less invasive and disturbs content less, but that's technically difficult.

Comment 33

[MechCommanderRei]

Could at least the "Find Next" button be left in, so that Quick Find is still basically useful for people who know "/" and nothing else?

(They are out there; Till now, it hasn't been necessary to know more.  I myself only learned "F3" by hanging around the MozillaZine forums.)

Comment 34

[MechCommanderRei]

Alternatively, Jus on MozillaZine suggests that Quick Find automatically highlight all matches, like in Opera 9.  This would keep it useful without having to add buttons back in.

Comment 35

[Peter Kasting]

(In reply to comment #34)
> Alternatively, Jus on MozillaZine suggests that Quick Find automatically
> highlight all matches, like in Opera 9.  This would keep it useful without
> having to add buttons back in.

Both find modes should auto-highlight all matches.  See bug 342101.

Comment 36

[Alexander Hessentswey]

Please add an option to turn findbar controls on and off.

Comment 37

[Peter Lairo]

FWIW: I must say that I'm not enjoying the castrated quick-find. I had been using FAYT as a replacement for Find because it's so damned convenient to just start typing what you're looking for, and it saved me the *extra step* of hitting CTRL+F. Now it's either an extra step (hitting CTRL+F) or the loss of discoverable access to Highlight all, Next, and Previous (I prefer mouse access for these). Now we have loss of functionality in exchange for some unnecessary visual distinction.

Was it ever explained *why* Find and FAYT *need* to be visually distinct, particularly in this loss-of-functionality way?

Comment 38

[Peter Kasting]

(In reply to comment #37)
> it's so damned convenient to just
> start typing what you're looking for, and it saved me the *extra step* of
> hitting CTRL+F.

Then file an RFE that turning on the typeaheadfind pref should cause typing to trigger Find mode rather than Quick Find mode.

I despise Quick Find mode altogether and think it's not worth having it in.  Almost every user comment I've seen about the new behavior leads me to believe users generally want Find behavior and wish that they could toggle on "bring up the find bar when I start typing".

> Was it ever explained *why* Find and FAYT *need* to be visually distinct,
> particularly in this loss-of-functionality way?

Yes, several times, on bugs and on the forums.  The summary is, because the two modes differ in about 4 different ways.  We can't use the same UI for two different modes that behave differently.  My argument is that we shouldn't have the second mode at all.

Comment 39

[Peter Lairo]

(In reply to comment #38)
> Yes, several times, on bugs and on the forums.  The summary is, because the 
> two modes differ in about 4 different ways.

Could you please say what the different modes are? (I searched for quick, FAYT, and typeaheadfind in mozilla.dev.planning and in mozilla.dev.apps.firefox but found nothing)

It just seems silly to castrate FAYT just for the sake of differentiating one function that has two different names.

Should these different modes turn out to be sufficiently ... different, then I'd gladly file an RFE for an option to "start find when i start typing" (which in essence would be FAYT -> craziness). It just seems we already have that, we just need to rename the option and undo this bug fix.

Comment 40

[Peter Kasting]

(In reply to comment #39)
> Could you please say what the different modes are? (I searched for quick, FAYT,
> and typeaheadfind in mozilla.dev.planning and in mozilla.dev.apps.firefox but
> found nothing)

You can read the original thread here:
http://groups.google.com/group/mozilla.dev.apps.firefox/browse_frm/thread/7507421fb3d710e6/6f7bf63c19ddef38

Off the top of my head, the behavior of <enter> differs (find next versus follow link), the quick find bar dismisses when blurred, and the quick find bar auto-dismisses after five seconds.  It seems like there was another difference but I can't think of it at the moment.  (Besides Quick find supporting a links-only search when started with ', that is...)

> It just seems silly to castrate FAYT just for the sake of differentiating one
> function that has two different names.

It's not one function, it's two.  See Mike Connor's reply #15 in that thread.

> Should these different modes turn out to be sufficiently ... different, then
> I'd gladly file an RFE for an option to "start find when i start typing" (which
> in essence would be FAYT -> craziness). It just seems we already have that, we
> just need to rename the option and undo this bug fix.

No, you're misunderstanding me (I think... or I'm misunderstanding you).  I don't want the fix reverted or any options renamed.  All I would suggest is that turning on the "start finding when I start typing" pref should pop up Find mode, not Quick Find mode.  So the only way to get to Quick Find is by typing / or '.  However, there are all kinds of potential issues with this, such as "maybe users of this wanted autodismiss behavior" and "if you type some, blur the bar, and then type some more, do we append to or replace the bar's content?"

Comment 41

[Jesse Ruderman]

It seems strange that "/" now brings up a full-width toolbar, but without useful buttons like "find next" (for people who don't know the keyboard shortcut) and "highlight all".  It looks like there are two competing solutions:

* Bug 347261, "/ FAYT should invoke Find rather than QuickFind".  I'm guessing this means getting rid of the "disappears after 5 seconds" behavior and making the find toolbar buttons available again.

* Bug 346400, "Make quicksearch not take up the entire width of window".

Comment 42

[Peter Kasting]

(In reply to comment #41)
> It seems strange that "/" now brings up a full-width toolbar, but without
> useful buttons like "find next" (for people who don't know the keyboard
> shortcut) and "highlight all".

See the voluminous original discussion for rationale behind the button removal.  (Quick summary: in theory "FAYT"/"Quick Find" is supposed to not actually be "find" but rather some accessibility "navigate to further down on the page" feature.  Or something like that.)

> It looks like there are two competing
> solutions:
> 
> * Bug 347261, "/ FAYT should invoke Find rather than QuickFind".  I'm guessing
> this means getting rid of the "disappears after 5 seconds" behavior and making
> the find toolbar buttons available again.

It means "Make hitting '/' do the same thing as 'ctrl-f'."  It does not change the appearance of Quick Find mode, but it makes '/' trigger normal find mode, not Quick Find.

Fixing this bug as well as the bug on making FAYT start Find mode instead of Quick Find mode (whose number I can't remember... may have been WONTFIXed again) would obviate the need for a Quick Find mode at all.  Which would probably make mconnor very unhappy and me very happy.

> * Bug 346400, "Make quicksearch not take up the entire width of window".

These two bugs are not mutually exclusive.  This one is on finishing the original design, which was to make the UI as minimal as possible.  As you implied, there's no need for a full-width toolbar when it's all empty, but we didn't do it originally due to XUL crappiness.