Last Comment Bug 171237 - Find in Page: highlighted result should be at the middle of the page (vertically centered) instead of last line/bottom of page
: Find in Page: highlighted result should be at the middle of the page (vertica...
Status: RESOLVED FIXED
[parity-Opera] [parity-Safari]
: dev-doc-complete, ue, user-doc-needed
Product: Core
Classification: Components
Component: Find Backend (show other bugs)
: Trunk
: All All
: P3 enhancement with 31 votes (vote)
: mozilla12
Assigned To: :Ehsan Akhgari
:
: Mike de Boer [:mikedeboer]
Mentors:
: 78833 263447 270124 286851 322479 361275 389109 397898 412334 415739 440198 471032 577866 653241 706038 (view as bug list)
Depends on: 720126
Blocks: 565552 743103 isearch 440198 631250 631270 640132 658937 707963 748991
  Show dependency treegraph
 
Reported: 2002-09-27 11:32 PDT by Sander
Modified: 2012-11-02 06:10 PDT (History)
66 users (show)
benjamin: blocking1.9-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (v1) (8.91 KB, patch)
2012-01-10 21:31 PST, :Ehsan Akhgari
roc: review+
Details | Diff | Splinter Review
Part 2 (3.03 KB, patch)
2012-01-11 12:57 PST, :Ehsan Akhgari
roc: review+
Details | Diff | Splinter Review

Description Sander 2002-09-27 11:32:08 PDT
One the the biggest practical annoyances with the otherwise wonderful type ahead
find is that when the next occurance of a searchterm is found, it's located on
the very last line visible in the window. Particularly when searching through
all text (instead of just through links) chance are you will want to read on for
at least one or two more lines to see if this occurance is really relevant for
what you're looking for, or if you want to hit F3.
I don't think the occurance should appear anywhere near the middle of the
screen, as then spotting the selected text isn't as instantanious as it is now,
but I do think it would be good if the view could move on at least a few lines.
Comment 1 Anthony DeRobertis 2002-10-01 13:48:54 PDT
Regular find has the same problem.
Comment 2 Aaron Leventhal 2002-10-26 00:04:47 PDT
nsPresShell.cpp's nsISelectionController:ScrollSelectionIntoView() calls
nsSelection.cpp's nsIFrameSelection::ScrollSelectionIntoView() calls
nsTypedSelection::ScrollIntoView() calls
nsTypedSelection::ScrollRectIntoView() which takes arguments aVPercent and
aHPercent, what percent vertically or horizontally to position at (or
NS_PRESSHELL_SCROLL_ANYWHERE).

We could use the aVPercent argument to get this done, but none of the other
interfaces currently support that, we'd need to change them all.
Comment 3 Daniel Wang 2005-10-05 12:51:52 PDT
*** Bug 78833 has been marked as a duplicate of this bug. ***
Comment 4 Daniel Wang 2005-10-05 12:59:19 PDT
*** Bug 270124 has been marked as a duplicate of this bug. ***
Comment 5 Daniel Wang 2005-10-05 13:01:49 PDT
*** Bug 263447 has been marked as a duplicate of this bug. ***
Comment 6 Vincent Lefevre 2005-10-05 23:51:31 PDT
Please correct the subject: occurence -> occurrence.
Comment 7 David E. Ross 2005-10-08 10:50:43 PDT
As indicated in bug #78833 (closed as a duplicate of this later bug), a forward
Find that requires a scroll should position the found string a few lines below
the top of the window.  Then, subsequent Find operations for nearby instances of
that same string might not require additional scrolling.  This allows the user a
better view of the context of multiple instances of the target string.  

Similarly, a backward Find should position the found string a few lines above
the bottom of the window.  
Comment 8 Jesse Ruderman 2005-12-15 16:02:09 PST
*** Bug 286851 has been marked as a duplicate of this bug. ***
Comment 9 Jesse Ruderman 2005-12-15 16:03:56 PST
Masayuki, are you interested in fixing this bug?
Comment 10 Jonathan Haas 2005-12-19 09:56:20 PST
It could also be possible to make the highlighted text be positioned in the middle of the of the window. This is done this way also in several other applications and gives a good overview over the page, too. Additional there would be no difference between backwards- and forward-searching that may confuse the user.
Comment 11 Smokey Ardisson (offline for a while; not following bugs - do not email) 2006-01-15 00:08:22 PST
*** Bug 322479 has been marked as a duplicate of this bug. ***
Comment 12 Peter Kasting 2006-06-09 17:55:42 PDT
I'll take this on the off chance I get to it at some point soon.
Comment 13 Jesse Ruderman 2006-10-03 20:03:48 PDT
See also bug 355229, same bug for scrolling to #named_anchor.
Comment 14 Ria Klaassen (not reading all bugmail) 2006-11-20 08:19:29 PST
*** Bug 361275 has been marked as a duplicate of this bug. ***
Comment 15 Peter Kasting 2007-01-29 23:16:11 PST
Not going to be getting to this one.
Comment 16 Wayne Mery (:wsmwk, NI for questions) 2007-03-28 00:40:52 PDT
isn't Chris' patch from bug 78833 comment 20 a good starting point?
Comment 17 Stuart Morgan 2007-07-21 13:01:11 PDT
*** Bug 389109 has been marked as a duplicate of this bug. ***
Comment 18 Ria Klaassen (not reading all bugmail) 2007-07-21 13:24:47 PDT
I use the code on this page http://forums.mozillazine.org/viewtopic.php?p=2824788#2824788 for quite some time, and it works excellent. I even nearly forgot that it's not Firefox doing that job...  Maybe if the author agrees, it could be integrated in Firefox?
Comment 19 Benjamin Smedberg [:bsmedberg] 2007-08-31 08:13:06 PDT
We might still accept a patch for M8 or even M9 if it's low-risk.
Comment 20 Jo Hermans 2007-09-29 06:01:02 PDT
(In reply to comment #18)
> I use the code on this page
> http://forums.mozillazine.org/viewtopic.php?p=2824788#2824788 for quite some
> time, and it works excellent. I even nearly forgot that it's not Firefox doing
> that job...  Maybe if the author agrees, it could be integrated in Firefox?
> 

That requires the userChrome.js extension (see also bug 332529)
Comment 21 Kevin Brosnan 2008-01-14 14:39:26 PST
*** Bug 412334 has been marked as a duplicate of this bug. ***
Comment 22 Justin Wood (:Callek) 2008-02-05 01:33:32 PST
*** Bug 415739 has been marked as a duplicate of this bug. ***
Comment 23 Jo Hermans 2008-06-19 02:02:22 PDT
*** Bug 440198 has been marked as a duplicate of this bug. ***
Comment 24 Dave Webber 2010-06-29 11:34:27 PDT
This also can become a problem if a website includes a floating "toolbar" which it keeps stuck in place at the bottom (or top, if scrolling upwards to a previous search result) of the content window, as this will result in the found term being scrolled underneath the "toolbar" and out of the user's sight and click space.
Comment 25 Aryeh Gregor (:ayg) (next working March 28-April 26) 2010-10-21 16:39:08 PDT
*** Bug 471032 has been marked as a duplicate of this bug. ***
Comment 26 Aryeh Gregor (:ayg) (next working March 28-April 26) 2010-10-21 16:40:34 PDT
*** Bug 577866 has been marked as a duplicate of this bug. ***
Comment 27 Tom Schneider 2010-10-21 22:15:48 PDT
> I don't think the occurance should appear anywhere near the middle of the
> screen, as then spotting the selected text isn't as instantanious as it is now,
> but I do think it would be good if the view could move on at least a few lines.

I don't agree.  The text should appear (when possible) exactly in the center
of the screen.  One then gets to see the maximum context possible.  One also knows
exactly where it will appear.  There is also then no bias by the programmer about directionality in the text.

As for noticing the text - take a look at what the PDF reader skim does.  It highlights the text brightly and then that fades to an ellipse around the text.  You can't miss it!
Comment 28 Wayne Mery (:wsmwk, NI for questions) 2010-10-23 12:23:05 PDT
*** Bug 397898 has been marked as a duplicate of this bug. ***
Comment 29 chbok 2011-04-07 07:41:29 PDT
Under Firefox 4, UI experience is worse with highlighted links that found. The "status bar" appears over searched text!
Comment 30 Jesse Ruderman 2011-04-08 18:56:55 PDT
chbok, that sounds like bug 640132. While it's true that fixing this bug would take of the overlap in many cases, it wouldn't fix it the case where the found text is at the bottom of the page (so there's no more room to scroll).
Comment 31 Lucas Malor (mail: c6kfnkn2uc AT snkmail DOT c0m) 2011-04-13 04:21:01 PDT
(In reply to comment #30)
Well, if you read bug 640132 comment 3 you can resolve this adding a padding at the end of the page if the search reaches the end. I want to suggest the padding can be used also to _centre_ that search.
Comment 32 RNicoletto 2011-04-13 08:50:09 PDT
(In reply to comment #31)
> (In reply to comment #30)
> Well, if you read bug 640132 comment 3 you can resolve this adding a padding at
> the end of the page if the search reaches the end. I want to suggest the
> padding can be used also to _centre_ that search.

Looks like bug 441098.
Comment 33 RNicoletto 2011-04-13 09:04:29 PDT
(In reply to comment #32)
> cc: RNicoletto@gmail.com(In reply to comment #31)
> > (In reply to comment #30)
> > Well, if you read bug 640132 comment 3 you can resolve this adding a padding at
> > the end of the page if the search reaches the end. I want to suggest the
> > padding can be used also to _centre_ that search.
> 
> Looks like bug 441098.

Sorry. I mean bug 440198.
Comment 34 Art K 2011-08-26 02:23:32 PDT
I just made this suggestion on the dev-apps forum and was directed here. This item was submitted Sept 2002. NINE YEARS AGO! It should have been a no-brainer. From all the duplicates noted it would seem that there were hundreds of others feel the same. Why the delay?

I also vote for vertically centering the found text, not just a few extra scroll lines, just as (another duplicate), bug 440198 suggests.
Comment 35 Tom Schneider 2011-08-26 02:54:50 PDT
(In reply to Sander from comment #0)
> I don't think the occurance should appear anywhere near the middle of the
> screen, as then spotting the selected text isn't as instantanious as it is
> now,
> but I do think it would be good if the view could move on at least a few
> lines.

I disagree.  If the search ALWAYS put the result in the exact center of the screen one would always know exactly where to look!  The most context maximum is the most useful.
Comment 36 Sander 2011-08-26 03:08:08 PDT
In reply to Tom Schneider from comment #35:
You wrote the same thing in comment #27 as well. No need to repeat yourself. FWIW, personally I don't have a strong feeling about it; I just think that the center is worse because 1) I for one have no idea where the exact center on my screen is (and the bigger the screen, the worse the uncertainty), while I _can_ pinpoint "two lines above the bottom" and 2) any find in the last half page of a site will appear at an unknown location (somewhere between the center and the bottom). 

However, whoever implements this will decide (maybe with some feedback from Mozilla UX-people). Discussing such issues beforehand only makes the bug noisier and less likely to be fixed at all.
Comment 37 Tom Schneider 2011-08-26 03:15:40 PDT
> In reply to Tom Schneider from comment #35:
> You wrote the same thing in comment #27 as well.

Ha, I hadn't noticed that, sorry for the repeat.  I suppose I'm
consistent over long periods.  As for a solution - how about making it
a user determined switch?  Then each person does what they want.

> Discussing such issues beforehand only makes the bug noisier and
> less likely to be fixed at all.

Given that it's been 9 years, perhaps a discussion of this ought to be
held so the issues are resolved.
Comment 38 Vincent Lefevre 2011-08-29 04:04:54 PDT
I think I prefer something near the center to have the maximum context, but the best position may depend on the document. Also this bug should be fixed at the same time as bug 622801 (still UNCONFIRMED unfortunately).
Comment 39 Ron Baldwin 2011-09-05 12:33:55 PDT
(In reply to Sander from comment #36)
> In reply to Tom Schneider from comment #35:
> You wrote the same thing in comment #27 as well. No need to repeat yourself.
> FWIW, personally I don't have a strong feeling about it; I just think that
> the center is worse because 1) I for one have no idea where the exact center
> on my screen is (and the bigger the screen, the worse the uncertainty),
> while I _can_ pinpoint "two lines above the bottom" and 2) any find in the
> last half page of a site will appear at an unknown location (somewhere
> between the center and the bottom). 

1) It will always be obvious where the exact center (vertically) of the screen is, because that's where the highlighted text you were searching for will be.

2) This will happen regardless of where you try to position the search result.  For example, this is exactly what happens now (search result at a "unknown" location) if the search result is near the top of the page or is already visible.  (Note this is about the vertical position; the search result already appears at an "unknown" location horizontally.)  I haven't heard any complaining about this aspect of it's behavior, and regardless, highlighting the text goes a long way to drawing your eyes to it's location, wherever that may be.
Comment 40 Mardeg 2011-11-29 02:32:58 PST
*** Bug 706038 has been marked as a duplicate of this bug. ***
Comment 41 :Ehsan Akhgari 2012-01-10 21:31:29 PST
Created attachment 587591 [details] [diff] [review]
Patch (v1)

This has always driven me nuts.  Tonight it annoyed me enough that I decided that I will spend 15 minutes to fix it once and for all!  :-)
Comment 42 :Ehsan Akhgari 2012-01-11 12:57:20 PST
Created attachment 587790 [details] [diff] [review]
Part 2

This caused failures on the try server, which helped me find a bunch of stuff which I did not fix in bug 605138.  They matter now since the false values will be passed in and used for vertical and horizontal percents.

This patches fixes those calls.
Comment 44 Mozilla RelEng Bot 2012-01-11 21:01:24 PST
Try run for f3094f75f844 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=f3094f75f844
Results (out of 211 total builds):
    exception: 1
    success: 179
    warnings: 29
    failure: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-f3094f75f844
Comment 46 Wayne Mery (:wsmwk, NI for questions) 2012-01-13 08:09:53 PST
I'm testing the win32 build from http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-f3094f75f844
In a word, this is outstanding!  thanks ehsan!
Comment 47 Steffen Wilberg 2012-01-18 07:20:39 PST
Adjusting summary to what was done.
Comment 48 Jared Wein [:jaws] (please needinfo? me) 2012-01-18 09:55:50 PST
*** Bug 440198 has been marked as a duplicate of this bug. ***
Comment 49 :aceman 2012-01-20 01:51:51 PST
This will be great. It annoyed me that the found text was often hidden behind the floating link target popup (on link heavy pages like buglists).
Comment 50 Jesper Hansen 2012-01-21 06:26:16 PST
I have found this new feature a little annoying. 
If you have a result within a few lines of each other then the page jumps which can be disturbing and confused me into believing that I was an entirely new place on the page when I fact it had only moved 2 lines down.
Comment 51 :aceman 2012-01-21 06:35:19 PST
That is also true. Maybe the centering should only be done when the new match is found more than ~25% off the vertical center.
Comment 52 David E. Ross 2012-01-21 10:31:12 PST
How about not moving the displayed page at all if the new match already appeared in the displayed page?
Comment 53 Tom Schneider 2012-01-21 11:48:06 PST
(In reply to David E. Ross from comment #52)
> How about not moving the displayed page at all if the new match already
> appeared in the displayed page?

No because then you get huddled into the bottom of the page and the original problem occurs.
Comment 54 Tom Schneider 2012-01-21 11:50:14 PST
(In reply to :aceman from comment #51)
> That is also true. Maybe the centering should only be done when the new
> match is found more than ~25% off the vertical center.

This might work!

I think that one solution is to allow a set of switches so that each person
can adjust it themselves.  First option is the original (obnoxious in my opinion)
method of always being at the bottom of the page.  Second is always putting the result
in the exact vertical center.  Third is a distance trigger as aceman suggested.
Comment 55 :aceman 2012-01-21 12:03:27 PST
Yes, but please discuss this in bug 720126, which was created for this problem from comment 50.
Comment 56 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-02-13 10:13:30 PST
*** Bug 653241 has been marked as a duplicate of this bug. ***
Comment 57 Tom Schneider 2012-02-28 15:19:18 PST
I came across another example for
Build identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:10.0.2) Gecko/20120216 Firefox/10.0.2 SeaMonkey/2.7.2

At
http://www.mindscience.org/database
search for the word 'prism'
It is lost under the bottom bar.
Comment 58 Eric Shepherd [:sheppy] 2012-04-23 12:17:57 PDT
Documentation updated:

https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsISelectionController#Scroll_constants

And mentioned on Firefox 12 for developers.
Comment 59 Christian Kujau 2012-06-26 12:53:24 PDT
Wow, a 10-year-bug fixed, congrats :-) But what if I don't like this change (and have never considered it as a bug), is there a magic about:config switch to turn the old behaviour back on?
Comment 60 Matt Brubeck (:mbrubeck) 2012-06-26 13:11:41 PDT
(In reply to Christian Kujau from comment #59)
> Wow, a 10-year-bug fixed, congrats :-) But what if I don't like this change
> (and have never considered it as a bug), is there a magic about:config
> switch to turn the old behaviour back on?

There isn't; sorry.  If there are specific reasons that this change bothers you, and if you think those reasons could be fixed without losing the main benefits from this change, feel free to file them as follow-up bugs.  For example there is bug 720126 which is fixed in Firefox 14 (currently on the beta channel).
Comment 61 Christian Kujau 2012-06-26 16:29:30 PDT
Thanks for pointing me to 720126, this is exactly the issue I have since this one (171237) got "fixed". I'll wait for Firefox 14 then.

Note You need to log in before you can comment on or make changes to this bug.