345786 - Match case button missing from Find Bar

Status: RESOLVED FIXED

(Toolkit :: Find Toolbar, defect)

Description

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

Bug 340849 went through a couple of iterations looking at making an "auto-case sensitve" mode for the Find Bar, and part of the solution was to remove the "Match Case" button from the bar.

The hidden pref is great, and I think will be a real gem for power users, but when it's not set, there still needs to be a way for users to get to their case sensitivity for those (admittedly few!) times that they need it.

What I'd suggest is:

 - no changes to the FastFind bar
 - when accessibility.typeaheadfind.casesensitive == (0||1) show the "Match Case" checkbox on the Find Toolbar and toggling the checkbox toggles these values
 - when accessibility.typeaheadfind.casesensitive == -1, only show a "(Case Sensitive)" label on the Find Toolbar when the user triggers auto case sensitivity mode

So net: add the "Match Case" box back to the Find Bar when accessibility.typeaheadfind.casesensitive == (0||1)

Comment 1

[Peter Kasting]

One other "net" out of this is that I should remove the "(Case sensitive)" label when the match case button is present.

Note that the Quick Find bar keeps the mode that the Find bar has, and has the "(Case sensitive)" label if applicable.  That is the current behavior (I think) and shouldn't change.

Comment 2

[Jesse Ruderman]

How about leaving the checkbox visible even for users with "auto-case sensitive" mode enabled?  (Typing a capital letter would then make the box become checked.) The advantages of using a checkbox rather than a label are that these users would still be able to do an all-lowercase case-sensitive search, and would still be able to do a case-insensitive search after pasting a mixed-case string.

Comment 3

[Peter Kasting]

(In reply to comment #2)
> How about leaving the checkbox visible even for users with "auto-case
> sensitive" mode enabled?  (Typing a capital letter would then make the box
> become checked.)

Checkboxes should never auto-toggle.  They should ESPECIALLY never toggle after a user has twiddled them, even once.  I've considered this before, and I just can't come up with a UI that doesn't make me run screaming.

I can see the desire for a "auto mostly, but sometimes forced one way" UI, but I don't think a checkbox is the right way to get that.  At this point I consider bug 341999 to be about solving that for Firefox 3.  I just posted more thoughts on this there.

Comment 4

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

(In reply to comment #3)
> I can see the desire for a "auto mostly, but sometimes forced one way" UI, but
> I don't think a checkbox is the right way to get that.  At this point I
> consider bug 341999 to be about solving that for Firefox 3.  I just posted more
> thoughts on this there.

As an added problem with the approach, it would mean that in some cases (ie: when case sensitive mode was set to "auto") clicking the checkbox would be temporary, but in other cases clicking the checkbox would actually change the setting. Boo.

Comment 5

[Peter Kasting]

Marking "at risk" as I'm still involved with bug 339127 (spellcheck attribute).  This shouldn't be too hard to do.

Comment 6

[Peter Kasting]

CCing jminta, who said at dinner tonight he might be able to help with this.

Comment 7

[Joey Minta]

(In reply to comment #6)
> CCing jminta, who said at dinner tonight he might be able to help with this.
> 
I got some feeds stuff assigned to me today, so it looks like I won't have time after all. :-(

Comment 8

[Peter Kasting]

Attachment: patch v1

This should do it.

Comment 9

[Peter Kasting]

Requesting blocking, removing at-risk

Comment 10

[Peter Kasting]

Argh, I hadn't meant to RESOLVE FIXED :(

Comment 11

[Mike Connor [:mconnor]]

Comment on attachment 232050 [details] [diff] [review]
patch v1

+    matchCaseCheckbox.hidden = this.mUsingMinimalUI ||
+      (this.mTypeAheadCaseSensitive != 0 && this.mTypeAheadCaseSensitive != 1);
+    matchCaseText.hidden = !matchCaseCheckbox.hidden;

why not just compare to 2?  r=me otherwise

Comment 12

[Peter Kasting]

(In reply to comment #11)
> (From update of attachment 232050 [details] [diff] [review] [edit])
> +    matchCaseCheckbox.hidden = this.mUsingMinimalUI ||
> +      (this.mTypeAheadCaseSensitive != 0 && this.mTypeAheadCaseSensitive !=
> 1);
> +    matchCaseText.hidden = !matchCaseCheckbox.hidden;
> 
> why not just compare to 2?  r=me otherwise

Because we actually treat any value other than 0 or 1 as meaning auto... I think this is noted in the comments on the pref itself.  The "official value" is in theory -1 (that was what the default was set to for the week or so that auto case was on by default).

Comment 13

[Peter Kasting]

Checked into trunk to bake.

/mozilla/toolkit/components/typeaheadfind/content/findBar.inc 1.13
/mozilla/toolkit/components/typeaheadfind/content/findBar.js 1.45
/mozilla/toolkit/locales/en-US/chrome/global/findbar.dtd 1.9

Comment 14

[Peter Kasting]

Comment on attachment 232050 [details] [diff] [review]
patch v1

Requesting approval for branch.

Comment 15

[Peter Kasting]

I should note: the l10n impact here is that it re-adds a string from Fx 1.0/1.5 that was removed in B1.

Comment 16

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

Comment on attachment 232050 [details] [diff] [review]
patch v1

a=drivers for landing on the branch.

Comment 17

[Peter Kasting]

Fixed on branch.

/mozilla/toolkit/components/typeaheadfind/content/findBar.inc 1.4.4.8
/mozilla/toolkit/components/typeaheadfind/content/findBar.js 1.21.2.22
/mozilla/toolkit/locales/en-US/chrome/global/findbar.dtd 1.3.4.6

Comment 18

[Ria Klaassen (not reading all bugmail)]

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