Open Bug 342101 Opened 14 years ago Updated 3 years ago

Find bar: Auto-highlight all matches in page

Categories

(Toolkit :: Find Toolbar, defect, P2)

defect

Tracking

()

People

(Reporter: pkasting, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: p=0)

Attachments

(1 file, 2 obsolete files)

When someone uses the find bar, we should auto-highlight all matches in the page., something like Google cache, emacs, etc.  The "highlight" button can then be removed, as it will basically be useless.

This depends on bug 263683, because until highlighting is done using selections instead of via screwing with the DOM, this will slow down Find by an unacceptable amount.

This is distinct from bug 62467 and bug 157788 because this is a bug for the Find bar code in Firefox, which is not the same as the Find dialog or FAYT code in Seamonkey.
Note that this is similar to bug 240432, but this is specifically focused on doing this as the result of searching with the find bar, whereas that bug is more vaguely requesting the ability to highlight things in response to (apparently) arbitrary criteria.
Highlighting all matches will probably help in the case of Gmail's inbox, where the findbar has the bad habit to search first for the term on the first page of your inbox, while you're on the second page (51 - 100).
> This depends on bug 263683, because until highlighting is done using selections
> instead of via screwing with the DOM, this will slow down Find by an
> unacceptable amount.
The highlight slowness is bug 251862. You may want to adjust dependencies.
Adding bug 251862 as a dependency.
Depends on: 251862
Not going to be getting to this one.
Assignee: pkasting → nobody
Product: Firefox → Toolkit
Hardware: PC → All
Version: unspecified → Trunk
Blocks: 565552
This is a WIP patch that doesn't work properly in some cases but I'm attaching it here anyways in case someone else wants to build on top of it.

The main road block for this bug is the performance of highlighting many matches on a page.  This should be fixed by bug 251862 and dependencies. I noticed that another browser doesn't bother highlighting if there are over 100 matches but that also requires platform work to do.
Attached patch More comprehensive patch (obsolete) — Splinter Review
Thanks MattN, I used your patch as the basis for this.

This patch is meant to apply atop my patch for bug 537013.

One of the unresolved issues with this patch is whether or not there should be a half-second delay before highlight-on-find.  If not, some of the changes to tests can be undone.

Will add a dedicated test for the bits not hit by other tests soon.
Assignee: nobody → unusualtears
Attachment #565396 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attached patch Add test caseSplinter Review
Test case for now:

1. Load a page and find to highlight the content
2. Load on top of that page
3. Go back and see highlight still present
4. Go forward, close find bar
5. Go back and see highlight gets removed

Looking around a bit, it looks like bug 547218, comment 1 discusses the fix for that bug would remove the use case represented by [3], keeping the highlights on documents in history.  If so, re-highlighting on location change could be added the same place that highlights are removed if the find bar has closed while on a different document in the history (see change to browser.js in the patch).
Attachment #649881 - Attachment is obsolete: true
Duplicate of this bug: 811647
Depends on: 429732
No longer depends on: 251862
I think "highlight all by default" and "always highlight" are two different things. Many times, auto-highlighting will actually be useful, but not giving the user a choice is a bit detrimental I think.

For example, sometimes you'll just want the information as will be provided by bug 257016, other times when using QuickFind you want to search for something specific. As the patch is, the highlights would also be triggered by a QuickFind search (and I'm assuming they will be removed when closing the find bar, otherwise it makes no sense at all to remove the button) and the QuickFind bar closes on a timer. This becomes a very arbitrary decision to remove the highlights without knowing if the user is done with them or not. And if they aren't removed automatically, then it becomes extremely laborious just to simply remove them (having to open the find bar and erase the search field just for this).
If I'm right, we will not be able to land a patch for this bug until bug 429732 is fixed, right?

Waiting for bug 429732 to be fixed, what do you think about having "Highlight all" state remembered (i.e. kept it in a pref, in a similar way to what we currently do for the "Case sensitive" state with "accessibility.typeaheadfind.casesensitive").

This way, we don't have to bother all users with "Highlight All" always activated and no way to deactivate it (which might be annoying because of bug 429732) while preventing users who want this option to have to toggle "Highlight All" button every time they open a new tab.

What do think?

I'm fine to try to write a patch for this if you think this can be a good idea ;)
Please see bug 923801. In it I bring up a (IMO big) concern about the behavior of how firefox "remembers" the Match Case button. I would hope that, if your suggestion does go through, it addresses that very same concern for the Highlight All button.
(In reply to Luís Miguel [:Quicksaver] from comment #12)
> Please see bug 923801.

Thanks. Looks interesting.

> it addresses that very same
> concern for the Highlight All button.

I believe the important thing here is to be consistent: these two button ("Case", "Highlight All") look similar but behave differently, and this is really confusing IMHO.
Whiteboard: [feature] p=0
Hello.  I'm not familiar with the rules and customs of this bugzilla community.  So, please pardon me if my contribution isn't helpful, and please help me refine it in that case.

Here it is:

I use Firefox 27.0.1.  Every day, several times a day, I press ctrl+f someword alt+a.  This searches for someword and highlights all results.

I would rather not have to hit alt+a *every time*.  I looked in about:config, but I don't see a setting for this.

I used to click Highlight All, but then I adapted, using alt+a is faster.

So, if you add an about:config setting, I'll find it.

If you make the Highlight All button state persistent, I'll use it.

If you add some sort of crazy machine-learning system to the keyboard input handler so that one day it asks me "You seem to always hit alt+a after typing a word after hitting ctrl+f, would you like me to do this for you from now on?", I'll click yes.

You see what I mean?  :) I don't care which path you choose, so long as it saves me keystrokes AND I still have the option to disable highlight on demand (not just if it is slow, but also if it is ugly).

This bug is 7 years, 9 months, and 9 days old.

...Of course, I'm glad you all aim for perfection.  It's a great practice, and the practice has formed a great product! 

But may I recommend that for any given bug, after some time, let's say... five years?  Maybe it's time to be BOLD, pick one path, and walk it!

Thank you again, cheers,
--Dave
No longer blocks: fxdesktopbacklog
Flags: firefox-backlog+
Whiteboard: [feature] p=0 → p=0
(In reply to gseattle from comment #16)

Mozilla is open source: instead of complaining like this, you better try to fix this yourself and send your patch here.
If you can't implement this (no shame, not everyone has programming skills), and can't add any value to this bug (20 lines long comment to say "I want this absolutely" isn't "added value"), you better remain silent, or at least stick with describing problematic use cases, and vote for the bug.
As far as I can see, nobody from the Mozilla Corporation says in the bug "this is bad and we don't want it", just nobody took the time to implement this. So your very nice speech about "evil corporation" doesn't make sense at all.
But if you know Javascript/C++ and are willing to take some time to fix this, I'm sure plenty of nice other contributors will be glad to help you :)

I have to say I'm not affiliated in any way with Mozilla: I'm just a passionate geek contributing from time to time to Gecko, among other open source projects.
(In reply to gseattle from comment #19)

Could you please stop spamming this bug?
Thanks.
Assignee: unusualtears → nobody
Status: ASSIGNED → NEW
> Great business model where no one at the company is ever responsible for
> anything, users are always to blame for not having written it themselves.
> And then of course invite the public, then belittle them and hide
> (censoring-ish) their comments. Nice.

> It'll take you developers who know the code all of four minutes to make the
> 'Highlight All' state stick.

This is one of the worst things you could say without having the know-how. Not only is it not true, but very insulting to anyone who has ever contributed to Mozilla code; negative motivation is no motivation. You have been asked before, please stop spamming the bug unless you have new information to contribute.

FYI in the meantime there are alternatives, use an add-on called FindBar Tweak. (And no, nothing in that add-on took four minutes to code.)
Duplicate of this bug: 1193736
Duplicate of this bug: 697953
Priority: -- → P2
I think this bug can be closed as duplicate of bug 1291284, which has been implemented.
Bug 1291284 doesn't remove the "Highlight all" button, but make the preference true by default, which does pretty much the same as what this bug initially requested.
You need to log in before you can comment on or make changes to this bug.