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

VERIFIED FIXED in mozilla1.8.1beta1

Status

()

Toolkit
Find Toolbar
VERIFIED FIXED
11 years ago
9 years ago

People

(Reporter: Peter Kasting, Assigned: Waldo)

Tracking

(Blocks: 1 bug, {fixed1.8.1})

unspecified
mozilla1.8.1beta1
x86
All
fixed1.8.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking-firefox2 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [swag: 2d])

Attachments

(3 attachments)

(Reporter)

Description

11 years ago
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.

Updated

11 years ago
Flags: blocking-firefox2+
Keywords: helpwanted
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!)
(Reporter)

Comment 2

11 years ago
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.
ask annie/ben about textbox-in-popup pain.  Right now its just a toolbar that can be hidden easily.
Assignee: nobody → jwalden+bmo
(Reporter)

Comment 4

11 years ago
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.
Whiteboard: [swag: 2d]
Created attachment 224581 [details] [diff] [review]
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...
Attachment #224581 - Flags: review?(mconnor)
Created attachment 224582 [details]
Screenshot
Attachment #224582 - Flags: ui-review?(beltzner)
Keywords: helpwanted
(Reporter)

Comment 7

11 years ago
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 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?
Attachment #224582 - Flags: ui-review?(beltzner) → ui-review-
(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.
(Reporter)

Comment 10

11 years ago
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...
(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.
Status: NEW → ASSIGNED
(Reporter)

Comment 12

11 years ago
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.
(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.
(Reporter)

Comment 14

11 years ago
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 :)
(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.
(Reporter)

Comment 16

11 years ago
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 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.
Attachment #224582 - Flags: ui-review- → ui-review+
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
Attachment #224581 - Flags: review?(mconnor) → review+
(Reporter)

Comment 19

11 years ago
(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.
(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...
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Created attachment 224772 [details] [diff] [review]
Branch patch
Attachment #224772 - Flags: approval-branch-1.8.1?(mconnor)
(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.
Blocks: 340743
Blocks: 340747
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

11 years ago
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

11 years ago
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
(Reporter)

Comment 26

11 years ago
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.
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.

Updated

11 years ago
Attachment #224772 - Flags: approval-branch-1.8.1?(mconnor) → approval-branch-1.8.1+
Fixed on branch.
Keywords: fixed1.8.1
verified with Bon echo build from 0612 on Windows, marcia also comemnted seeing same on Mac
Status: RESOLVED → VERIFIED

Comment 30

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

Comment 31

11 years ago
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 :)
(Reporter)

Comment 32

11 years ago
(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

11 years ago
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

11 years ago
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.
(Reporter)

Comment 35

11 years ago
(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.
Please add an option to turn findbar controls on and off.

Updated

11 years ago
Blocks: 346400

Comment 37

11 years ago
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?
(Reporter)

Comment 38

11 years ago
(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

11 years ago
(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.
(Reporter)

Comment 40

11 years ago
(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?"
Depends on: 346842
(Reporter)

Updated

11 years ago
No longer depends on: 346842

Comment 41

11 years ago
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".
(Reporter)

Comment 42

11 years ago
(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.

Updated

9 years ago
Depends on: 441810
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.