340849 - Cleaner case-sensitive search UI

Status: RESOLVED FIXED

(Toolkit :: Find Toolbar, defect)

Description

[Peter Kasting]

Shameless stolen from bug 340747 comment #2:

We should switch to doing something like Emacs for case sensitive searching (where we automatically determine whether to do a case-sensitive search based on what the user types), and remove the persistent option from the bar.  That would make the Find bar UI cleaner and solve this bug, and make the user experience easier as well, I think.  (Currently, it's annoying to have to toggle something on to do a case-sensitive search, and annoying to have to remember to toggle it off when done.)

The specific case-sensitivity behavior I'd like is to be case-insensitive for all-lowercase searches, and case sensitive for mixed-case searches.  I'm not sure which way is better on all-uppercase searches.  Case-sensitive, I think?

For those who need to set things differently, this should probably be controlled by a pref:
0 = never case-sensitive
1 = always case-sensitive
2 = auto-determine [default]

Shouldn't be hard to code in this behavior, it's just a UI question.  It'd be a nice win IMO.

Comment 1

[Jeff Walden [:Waldo]]

Question: how does one go about determining whether a keypress is lowercase or uppercase?  Does this require CapsLock+Shift hackery, or is there some XPCOM service which can be used to correctly determine whether a character is Unicode-uppercase?  Anyway, things to think about in the patch...

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #1)
> Question: how does one go about determining whether a keypress is lowercase or
> uppercase?  Does this require CapsLock+Shift hackery, or is there some XPCOM
> service which can be used to correctly determine whether a character is
> Unicode-uppercase?  Anyway, things to think about in the patch...
> 

How about this way?

isLowerCase = char == char.toLowerCase();

Comment 3

[Jeff Walden [:Waldo]]

(In reply to comment #2)
> isLowerCase = char == char.toLowerCase();

Hmm, so apparently JS *does* do Unicode-aware toLowerCase(), as verified by the specs; didn't know that.  Sorry for the bugspam... :-(

Comment 4

[Hixie (not reading bugmail)]

Uh, woah, hold on, the Emacs behaviour is something that is all well and good most of the time, but I still quite often end up wanting to do a case-sensitive all-lowercase search. Please allow me to do that (without going into prefs each time!), like I can now.

Comment 5

[Hixie (not reading bugmail)]

(e.g. by providing a keystroke that toggles case-sensitivity on the fly.)

Comment 6

[Peter Kasting]

And how would we do the UI to show you'd just toggled into case-sensitive mode?  Being modal is bad enough, but being silently modal is just truly evil.  And if we just put the Match Case checkbox back to show this, then "auto" mode is worse than doing nothing, because how do we know when to change case sensitivity?  A tristate checkbox?

No, I'm still inclined to say that the proposed behavior is overall less bad than the current behavior.  If you want to do an all-lowercase search you can just skip matches you don't care about, or go into the prefs.  Yes, it's suboptimal, but is this really so common among so many users that it's worth the current UI pain?

I can think of other UI that lets people do this sort of thing, but none of the designs in my head are very good.  If you can come up with something that has all the advantages or the proposal here and none of the modality of the current confusion, plus isn't confusing, I'd love to hear it, because it'd certainly be better to let users have more control if we can do it cleanly.

Comment 7

[Hixie (not reading bugmail)]

When it's case-sensitive (including when it's automatically case-sensitive), show "(case-sensitive)" on the search bar somewhere.

Comment 8

[Peter Kasting]

Well, I should consider whether to do that anyway (perhaps in the space where the checkbox currently is, with a yellow circle "i" 'information' icon).  But that doesn't solve the problem of, if we provide a keyboard accelerator, how to make it not be modal and yet feel correct.  I might think if I hit alt-c that it should toggle the mode for good, not just for this search.  But that modality, and the UI of Match Case, are the exact things I'm trying to get away from.

Thoughts from beltzner (perhaps after I get a first patch hammered out) would probably be welcome...

Comment 9

[Hixie (not reading bugmail)]

Well, I guess I can live with case-insensitive lowercase and just use an editor when I want to do a special search. Nevermind.

Comment 10

[Peter Kasting]

Attachment: First cut

This implements my proposal.  accessibility.typeaheadfind.casesensitive controls case sensitivity: 0 = never, 1 = always, -1 (or anything else) = auto.  In auto mode, all-caps searches are case-sensitive.  There is no notification on the find bar of whether the current search is case-sensitive; that should conceivably be added, but I think maybe it should get an icon if it gets added, and I don't know how to get that accomplished.

If beltzner tries my patch and thinks this is the right way to go I'll ask for review.

Comment 11

[Hixie (not reading bugmail)]

> In auto mode, all-caps searches are case-sensitive.

Correction: In auto mode, all-lower-case searches are case-insensitive.

Comment 12

[Peter Kasting]

Well, yes, that's also true.  I wasn't making an exhaustive statement, just clarifying what I ended up doing with the all-caps case, which I was previously undecided about.

Comment 13

[Peter Kasting]

Comment on attachment 224948 [details] [diff] [review]
First cut

ben's ui-review over my shoulder is that this patch is a good thing and seems elegant and clever.

We're not sure about how to present UI for notifying the user of case-sensitivity or allowing them to force it on "just for this search".  Ben wonders if beltzner has ideas and says that even without that this patch seems like a good start.

Accordingly, I'm asking mconnor for review.  I haven't nominated for blocking-firefox2, but I wonder if people think this is the sort of change that would be good for that.

Comment 14

[Jeff Walden [:Waldo]]

Comment on attachment 224948 [details] [diff] [review]
First cut

>Index: browser/app/profile/firefox.js
>Index: mail/app/profile/all-thunderbird.js

>+pref("accessibility.typeaheadfind.casesensitive", -1);

This wants to be in modules/libpref/src/init/all.js; otherwise, you break any other users of the find bar because that pref won't be present for them.

Comment 15

[Peter Kasting]

I already asked ben about that, and he told me it was wrong of me to put prefs in that file that stuff in toolkit/ would be checking.  I will let someone who understands all this better explain why that is inaccurate, if it is.  I'll just do whatever people tell me to do :P

Comment 16

[Peter Kasting]

Note: If fixed, this bug will INVALIDate bug 264686, bug 265816, bug 273149, and bug 309512.

Comment 17

[Jeff Walden [:Waldo]]

(In reply to comment #15)
> I already asked ben about that, and he told me it was wrong of me to put prefs
> in that file that stuff in toolkit/ would be checking.  I will let someone who
> understands all this better explain why that is inaccurate, if it is.  I'll
> just do whatever people tell me to do :P

To some degree he's right:

http://lxr.mozilla.org/mozilla/source/modules/libpref/src/init/all.js#102

The problem is that there *is* no appropriate place for toolkit preferences yet, and expecting all toolkit users who want this to be forced to define the preference isn't a good solution.  The last time I asked bsmedberg about this, he said to keep adding toolkit-specific prefs to init.js.

Comment 18

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #17)
> The problem is that there *is* no appropriate place for toolkit preferences
> yet

(that's bug 231176)

Comment 19

[Peter Kasting]

Attachment: patch v2

OK, this puts the pref in all.js instead.

Comment 20

[Peter Kasting]

Nominating for Fx2 B1.  I think this is a nice change and probably INVALIDates bug 340747.

Comment 21

[Peter Kasting]

Adding a target milestone to increase likelihood of search queries finding me

Comment 22

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

Comment on attachment 225342 [details] [diff] [review]
patch v2

Looks like a good change. I think we do need some notification that the user has entered case sensitive mode, perhaps simply showing the label (much like we do for "Reached end of document") in the bar.

Definitely worth trying for beta and gathering user feedback.

Comment 23

[Peter Kasting]

My only concern with displaying a message is that it should maybe have an icon, and I don't know how to pull that off.

Aside from that, would "(Case Sensitive)" be good enough?  Where in the bar would you like the message to appear?  It should be easy to add just a message, I can update the patch to do that.

Comment 24

[Mike Connor [:mconnor]]

Comment on attachment 225342 [details] [diff] [review]
patch v2

I think this is quite possibly too clever for a default behaviour, but its worth giving it a shot.

Comment 25

[Peter Kasting]

Attachment: patch v3

Final patch version.  Attempting to carry over review flags.

This adds "(Case sensitive)" to the search bar when in case sensitive mode (auto or manual), and fixes the about:config pref to actually work right.

Comment 26

[Blake Kaplan (:mrbkap) (inactive)]

I just checked this patch in on the trunk and 1.8 branch for Peter:

toolkit/locales/en-US/chrome/global/findbar.properties: 1.2 -> 1.3.
toolkit/locales/en-US/chrome/global/findbar.dtd: 1.6 -> 1.7.
toolkit/components/typeaheadfind/content/findBar.js: 1.38 -> 1.39.
toolkit/components/typeaheadfind/content/findBar.inc: 1.9 -> 1.10.
modules/libpref/src/init/all.js: 3.643 -> 3.644.

Comment 27

[:Gavin Sharp [email: [e-mail]]]

Personally, I find the new behavior jarring. I very rarely want a case sensitive search, since I most often use the find bar to quickly scan a page for something, and case-sensitive is only useful when I want to find something very specific. Having to retype the text I'm searching for when I mistakenly type a capital letter or paste a string (bug 341999) is more trouble to me than having to check a check box when I want a more specific search. Silently toggling a search option based on the user input may work for emacs' target audience, but I'm willing to bet that for a lot of other users it just leads to unpredictable behavior and frustration.

Comment 28

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

(In reply to comment #27)
> Personally, I find the new behavior jarring. I very rarely want a case
> sensitive search, since I most often use the find bar to quickly scan a page

Gah. As expressed in bug 342000 comment 3, I had expected (based on where this bug came from) that we were talking about using this in FAYT mode only. If it isn't, then I really don't know if I can get behind this bug, because I don't think that the majority of users mean to perform case sensitive searches even when they enter camel caps, and the default value of accessibility.typeaheadfind.casesensitive should probably be changed.

Comment 29

[Peter Kasting]

Are you saying we should leave things as-is except for changing accessibility.typeaheadfind.casesensitive to 0?

Or are you saying we should have a "match case" button in Find mode?

I can't say the latter option sounds very good to me.  Making FAYT case sensitivity different than Find case sensitivity takes us further down the "two modes that do bizarrely different things" path, and I'm not sure our use cases motivate it.

Is there any way we can get better data than "uncertainty"?  Obviously I wouldn't have suggested this if I didn't feel users' lives would be _improved_ by this thing, but I'm not sure either of us have data to back up our "gut feelings".

Comment 30

[Peter Kasting]

Incidentally, I think the first option ("change casesensitive to 0") is bad too, since then it basically prevents people from doing case-sensitive searches at all by default, which is clearly missing functionality.

Inspired by the question I asked in bug 341999, I wonder in what circumstances Gavin runs into problems with this patch.  Asaf's patch in bug 342000 (which is behavior I had originally considered) fixes the "I had caps lock on" case, which seems like the most common source of accidentaly capitals.  I guess I've never accidentally typed a capital letter when I wasn't already typing another capital letter, so I'm not sure how I could accidentally trigger this.  Other than caps-lock, I usually intend at least the first capital letter I type...

Comment 31

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

(In reply to comment #29)
> Or are you saying we should have a "match case" button in Find mode?

Yes.

> I can't say the latter option sounds very good to me.  Making FAYT case
> sensitivity different than Find case sensitivity takes us further down the 

The rationale you used to convince me was that FAYT mode was used primarily by power user who is expecting optimized behaviour and a minimum amount of keystrokes. Should that user want to get to the extended power and UI, they just hit Accel-F.

If you think the lack of consistency would be jarring -- which it might well be -- then I'd say we should just default this to 0 and let it be a hidden pref. I think it's far safer to have false positives than negatives.

> Is there any way we can get better data than "uncertainty"?  Obviously I
> wouldn't have suggested this if I didn't feel users' lives would be _improved_
> by this thing, but I'm not sure either of us have data to back up our "gut
> feelings".

I agree that they would be improved for those who are fast and advanced searchers, which is why I supported it for FAYT. Accel-F is the Search of the Common People, for whom case sensitivity is a strange and foreign land.

Comment 32

[Peter Kasting]

> (In reply to comment #29)
> The rationale you used to convince me was that FAYT mode was used primarily by
> power user who is expecting optimized behaviour and a minimum amount of
> keystrokes.

Oh.  I never realized I was making that argument at all.  I feel pretty strongly that this behavior's value is orthogonal to FAYT mode.  It's either better or not.  If we don't think it is better for "normal users" after all, I'd far prefer to just remove it entirely than to make it a hidden pref on one of our two find modes.

Darin made some comments at lunch that suggested to me that I could instead morph the pref into an auto-case-is-on-or-off pref, and that in "off" mode the find bar would have the old checkbox and the quick find bar would be case-insensitive, whereas in the "on" mode they would both be in the style of this patch.  We would default to the "off" mode.  I could probably live with that, as it seems much better than leaving this in as an option for quick find only.  Thoughts?

(I'm tempted to go further and ask for a visible pref for "Use automatic case sensitivity when searching" in the Advanced tab, but that's probably pushing my luck.)

Comment 33

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

(In reply to comment #32)
> Darin made some comments at lunch that suggested to me that I could instead
> morph the pref into an auto-case-is-on-or-off pref, and that in "off" mode the
> find bar would have the old checkbox and the quick find bar would be
> case-insensitive, whereas in the "on" mode they would both be in the style of
> this patch.  We would default to the "off" mode.  I could probably live with
> that, as it seems much better than leaving this in as an option for quick find
> only.  Thoughts?

Sounds good to me. We'll want to blog about it and see how useful people find it, and if it's seen as one of those "hidden gem" features we can promote it to more primary UI.

> (I'm tempted to go further and ask for a visible pref for "Use automatic case
> sensitivity when searching" in the Advanced tab, but that's probably pushing 
> my luck.)

It's like we've known each other forever!

Comment 34

[Peter Kasting]

(In reply to comment #33)
> Sounds good to me. We'll want to blog about it and see how useful people find
> it, and if it's seen as one of those "hidden gem" features we can promote it to
> more primary UI.

Oh, like browser.urlbar.autofill?  :D
(Oh, whoops, I'm pushing my luck again!)
In all seriousness, I'm not sure how many people actually do tons of searching with the find bar at all, and how many of those use case-sensitive searches very much, so I suspect this doesn't hammer enough people to be worth UI space.  (Then again, those would also be arguments that doing it the automatic way isn't going to tick a lot of people off...)

Let's go for the creation of a followup patch that pushes the original patch's behavior into a hidden pref as I suggested above.  I'll morph bug 341999 into that.

In the meantime I'd still like to hear feedback from nightly users etc.  Do you think it would be useful to leave this way as the default in beta 1 for testing purposes, or would that be too off-putting for our userbase?

Comment 35

[Brett Wilson]

> In the meantime I'd still like to hear feedback from nightly users etc.  Do you
> think it would be useful to leave this way as the default in beta 1 for testing
> purposes, or would that be too off-putting for our userbase?

I like it. I haven't had the occasion to search for case-sensitive text yet, so perhaps I won't like that. I think it is worth having in b1 to see what people think.

Comment 36

[Ben Goodger (use ben at mozilla dot org for email)]

What I've seen so far as arguments against intelligent case sensitivity as default are:

1. many users may not intend to perform case sensitive searches 
2. it's now easier for users to invoke case sensitive searches without explicitly requesting it - e.g. user pastes in mixed-case and gets case-sensitive searching, perhaps against their wishes.
3. no lower case case sensitive matching
4. permanently enabling case insensitive searches is harder (a hidden pref).

My thoughts on all of this:

- case sensitive searching is a power user thing
- => we should default to case insensitive all the time
- if we're going to offer case sensitive search, we should make it difficult to get stuck in that mode such that the only way out is using the mouse. I think this is what Peter has been trying to solve. 

A few solutions I've heard to the third point:
- a tri-state button (case sensitive, case insensitive, case sensitive for this search only) - really complex
- a dual-state button (case insensitive, intelligent case sensitivity - what peter has so far) - less complex but more difficult to express in words/icons, still takes up space on the find bar.
- either of the above as context menu items on the field (less discoverable)
- either of the above as an advanced pref (even less discoverable, clutters prefs)
- leave the intelligent sensitivity option to the Quick Find "mode" (see below). 

Also:
- it seems fishy that we have two different find bar modes. After almost two years of Find Bar, I've re-trained myself to always use Ctrl+F to find, not /. Ctrl+F is the standard find shortcut. Shouldn't we offer the best solution to everyone, and not have two weird modes?

Summary:

The usability of what we have is jacked. 

One solution is to make the feature more intelligent. I would like to test my nightly with Peter's changes more to see how it works. 

If I were building a browser from scratch, I'd probably not have a case sensitivity mode, and save myself the headache. Oy. I'm sure someone will set me on fire for saying that. 

Comment 37

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

(In reply to comment #36)
> If I were building a browser from scratch, I'd probably not have a case
> sensitivity mode, and save myself the headache. Oy. I'm sure someone will set
> me on fire for saying that. 

Certainly not me. I'd make case sensitivity a hidden pref or something, keep it that way, and by default always be case insensitive. But anyway, this conversation's already gotten ranty in enough places ;)

Comment 38

[Peter Kasting]

As a note, when Ben says in comment #36 that "what [I have] now" is a checkbox between "off" and "auto", he actually meant that that was something he just came up with in his office, that I kind of liked as a better solution than the "hidden pref that toggles between old and new modes", which I think is a really bad move, but don't feel I have leverage to argue against :(

Given comment #37, what do people think of the idea of just making the simple change of defaulting accessibility.typeaheadfind.casesensitive to 0?  That does exactly what Ben and beltzner both think is better, leaves the UI clean, and still gives users the ability to do case sensitive searches automatically or always, if they wish.

I think that is honestly the best overall UE out of all the suggestions so far.  My only concern is that it's not terribly discoverable for those people who do want auto-sensitivity, but as Ben said, all the options for exposing that suck too.  If I had a gun to my head I'd probably make auto-sensitivity a context menu checkbox for the find bar -- still pretty undiscoverable, but less so (and more convenient) than a hidden pref.  I would not change "the alternative to auto is just case insensitivity".

Comment 39

[Peter Kasting]

I've implemented this in bug 342081.

I wouldn't mind if auto-case-sensitivity became more discoverable, but that's solvable later.

Comment 40

[Mikel Ward]

I've just discovered that checking the "Match case" box on the full find bar toggles the accessibility.typeaheadfind.casesensitive preference.

This means that if you enable case sensitivity when using Ctrl+F, all your / searches will be case sensitive as well.

Is this intentional?

Build identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/2006101023 Firefox/2.0

Comment 41

[Peter Kasting]

(In reply to comment #40)
> This means that if you enable case sensitivity when using Ctrl+F, all your /
> searches will be case sensitive as well.
> 
> Is this intentional?

Honestly, I can't recall at this point.  We discussed both always-lowercase and matches-findbar behavior, and you're right that the current behavior is matches-findbar (with a note in the Quick Find bar when it is case sensitive), so I'm going to assume that I did this intentionally :P