242056 - Selection doesn't scroll to find position when the match is inside a very long <a> tag

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect)

Description

[Adam Hauner]

2004042109/1.7RC1/W2K

Repro:
1. open page http://www.cen.uiuc.edu/bstats/days/97-09/970909.html
2. search for example "PlanetWeb"
3. after first match skip to the begin of page, click with mouse somewhere to 
   text
4. hit F3 to search again

Actual:
Browser after step 4 skips to line with:
"Hosts       Specific Browser Version"
not to line with "PlanetWeb", which isn't shown on display.

Expected:
Browser should skip to line/screen with PlanetWeb string.

Maybe small hint about "why" is in HTML source snippet, see unclosed "<a name=...":

=====================
<p>
<a name=BrowserDNS>
<h2>Browser DNS Domains</h2>
<pre>
 Browser DNS Domains            Hosts     %
 ------------------------------------------
  1. edu                         2941  23.6
  2. unregistered                2573  20.6
=====================

In DOM-view this "A NAME" is repeated:
=====================
<h2><a name="BrowserDNS">Browser Versions - All</a></h2>
<pre><a name="BrowserDNS"> Hosts       Specific Browser Version
 ---------------------------------------------------------------------
 1           AIR_Mosaic(16bit)/v1.00.198.07
 1           AIR_Mosaic(16bit)/v3.10.06.07
 7           Active Worlds Browser
 1           AltaVista Top1000 CustomCrawl
 1           Amiga-AWeb/3.0DEMO
=====================

Maybe it's Parser bug...

Comment 1

[Stefan Borggraefe]

Confirming with a Linux build from yesterday and Firefox 0.8.

Find does not work if the search result is inside a very long <a> tag. Other
tags like <pre> or <span> don't show this problem.

Browser/Search is the wrong component for this. Lets try Browser/XP Apps:GUI
Features, but this may be wrong either.

Comment 2

[Stefan Borggraefe]

Attachment: Testcase

Instructions how to use this testcase:
  1. Load the testcase into the browser
  2. Select "Edit|Find in this page..." and enter the search term "bug"
  3. Press the Find button so "bug" becomes the active search term
  4. Close the find dialog
  5. Click with the mouse at the beginning of the document
  6. Select "Edit|Find Again"

Result:
In Step 6 Mozilla doesn't jump to the first occurance of the string "bug".

Comment 3

[rbs]

See also bug 78833.

Comment 4

[Akkana Peck]

One thing I notice with that testcase: when I do "find again" (I'm using the
accel-G keybinding) it actually does skip to the right place, and I very briefly
see "bug" selected at the bottom of the page; then it skips back to the
beginning of the document.  So the question is, what's making it scroll back home?

I'm not seeing any editor involvement here at all (I tested in a browser window
and saw the problem).  Seems like whatever is making it scroll the second time
is either in the find code or (more likely) in the selection controller's
ScrollSelectionIntoView.  Changing component to Selection.

Comment 5

[rbs]

re: comment 4

> I very briefly see "bug" selected at the bottom of the page; then it skips
> back to the beginning of the document.  So the question is, what's making it
> scroll back home?

I found the problem. See the stack trace below. MoveFocusToCaret() gives the
focus to <a>, which causes another ScrollFrameIntoView(). However, this one is
targeted at the primary frame -- not at the continuation frame that is selected.
Hence the scroll destroy the more careful scrolling that the find's selection
die earlier.

The fix is simply to change the order. Will attach a patch.

PresShell::ScrollFrameIntoView(const PresShell * const 0x0347b3d0, nsIFrame *
0x0362bcd4, int -1, int -1) line 4241 + 35 bytes
nsHTMLAnchorElement::SetFocus(nsIPresContext * 0x03008148) line 286
nsEventStateManager::FocusElementButNotDocument(nsIContent * 0x034ae418) line 4742
nsEventStateManager::MoveFocusToCaret(nsEventStateManager * const 0x035d3630,
int 1, int * 0x0012b218) line 4840
nsWebBrowserFind::SetSelectionAndScroll(nsIDOMWindow * 0x03462b7c, nsIDOMRange *
0x034a85a0) line 470
nsWebBrowserFind::SearchInFrame(nsIDOMWindow * 0x03462b7c, int 0, int *
0x0012b600) line 807
nsWebBrowserFind::FindNext(nsWebBrowserFind * const 0x03579fb0, int *
0x0012b600) line 157 + 23 bytes
XPTC_InvokeByIndex(nsISupports * 0x03579fb0, unsigned int 3, unsigned int 1,
nsXPTCVariant * 0x0012b600) line 102
XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWrappedNative::CallMode
CALL_METHOD) line 2027 + 43 bytes
XPC_WN_CallMethod(JSContext * 0x02d565f8, JSObject * 0x00ec9ac8, unsigned int 0,
long * 0x03614868, long * 0x0012b8d0) line 1287 + 14 bytes
js_Invoke(JSContext * 0x02d565f8, unsigned int 0, unsigned int 0) line 1281 + 23
bytes
js_Interpret(JSContext * 0x02d565f8, long * 0x0012c304) line 3366 + 15 bytes
js_Invoke(JSContext * 0x02d565f8, unsigned int 1, unsigned int 2) line 1301 + 13
bytes
js_InternalInvoke(JSContext * 0x02d565f8, JSObject * 0x02d93d00, long 47791384,
unsigned int 0, unsigned int 1, long * 0x0012c568, long * 0x0012c564) line 1378
+ 20 bytes
JS_CallFunctionValue(JSContext * 0x02d565f8, JSObject * 0x02d93d00, long
47791384, unsigned int 1, long * 0x0012c568, long * 0x0012c564) line 3618 + 31 bytes
nsJSContext::CallEventHandler(JSObject * 0x02d93d00, JSObject * 0x02d93d18,
unsigned int 1, long * 0x0012c568, long * 0x0012c564) line 1292 + 33 bytes
nsJSEventListener::HandleEvent(nsJSEventListener * const 0x0366ce08, nsIDOMEvent
* 0x0307b868) line 174 + 51 bytes
nsXBLPrototypeHandler::ExecuteHandler(nsIDOMEventReceiver * 0x035c3cf8,
nsIDOMEvent * 0x0307b868) line 460
nsXBLWindowHandler::WalkHandlersInternal(nsIDOMEvent * 0x0307b868, nsIAtom *
0x0026e700 {???}, nsXBLPrototypeHandler * 0x036fe720) line 324 + 24 bytes
nsXBLWindowKeyHandler::WalkHandlers(nsXBLWindowKeyHandler * const 0x02ee3d90,
nsIDOMEvent * 0x0307b868, nsIAtom * 0x0026e700 {???}) line 161
nsXBLWindowKeyHandler::KeyPress(nsXBLWindowKeyHandler * const 0x02ee3d90,
nsIDOMEvent * 0x0307b868) line 177
DispatchToInterface(nsIDOMEvent * 0x0307b868, nsIDOMEventListener * 0x02ee3d90,
unsigned int (nsIDOMEvent *)* 0x014862d0 `vcall'(nsIDOMEvent *), const nsID &
{...}, int * 0x0012ce64) line 127 + 11 bytes
nsEventListenerManager::HandleEvent(nsEventListenerManager * const 0x02ee32f0,
nsIPresContext * 0x03008148, nsEvent * 0x0012f71c, nsIDOMEvent * * 0x0012f390,
nsIDOMEventTarget * 0x02ea69e8, unsigned int 514, nsEventStatus * 0x0012f540)
line 1520 + 35 bytes
nsXULDocument::HandleDOMEvent(nsIPresContext * 0x03008148, nsEvent * 0x0012f71c,
nsIDOMEvent * * 0x0012f390, unsigned int 514, nsEventStatus * 0x0012f540) line 1239
nsXULElement::HandleDOMEvent(nsIPresContext * 0x03008148, nsEvent * 0x0012f71c,
nsIDOMEvent * * 0x0012f390, unsigned int 514, nsEventStatus * 0x0012f540) line
2813 + 51 bytes
nsXULElement::HandleDOMEvent(nsIPresContext * 0x03008148, nsEvent * 0x0012f71c,
nsIDOMEvent * * 0x0012f390, unsigned int 514, nsEventStatus * 0x0012f540) line
2807 + 57 bytes
nsXULElement::HandleDOMEvent(nsIPresContext * 0x03008148, nsEvent * 0x0012f71c,
nsIDOMEvent * * 0x0012f390, unsigned int 514, nsEventStatus * 0x0012f540) line
2807 + 57 bytes
nsXULElement::HandleDOMEvent(nsIPresContext * 0x03008148, nsEvent * 0x0012f71c,
nsIDOMEvent * * 0x0012f390, unsigned int 514, nsEventStatus * 0x0012f540) line
2807 + 57 bytes
nsXULElement::HandleDOMEvent(nsIPresContext * 0x03008148, nsEvent * 0x0012f71c,
nsIDOMEvent * * 0x0012f390, unsigned int 514, nsEventStatus * 0x0012f540) line
2807 + 57 bytes
nsXULElement::HandleDOMEvent(nsIPresContext * 0x03008148, nsEvent * 0x0012f71c,
nsIDOMEvent * * 0x0012f390, unsigned int 514, nsEventStatus * 0x0012f540) line
2807 + 57 bytes
nsXULElement::HandleDOMEvent(nsIPresContext * 0x03008148, nsEvent * 0x0012f71c,
nsIDOMEvent * * 0x0012f390, unsigned int 514, nsEventStatus * 0x0012f540) line
2807 + 57 bytes
nsXULElement::HandleDOMEvent(nsIPresContext * 0x03008148, nsEvent * 0x0012f71c,
nsIDOMEvent * * 0x0012f390, unsigned int 514, nsEventStatus * 0x0012f540) line
2807 + 57 bytes
nsXULElement::HandleChromeEvent(nsXULElement * const 0x033c62cc, nsIPresContext
* 0x03008148, nsEvent * 0x0012f71c, nsIDOMEvent * * 0x0012f390, unsigned int
514, nsEventStatus * 0x0012f540) line 3874 + 35 bytes
GlobalWindowImpl::HandleDOMEvent(nsIPresContext * 0x03008148, nsEvent *
0x0012f71c, nsIDOMEvent * * 0x0012f390, unsigned int 514, nsEventStatus *
0x0012f540) line 913
nsDocument::HandleDOMEvent(nsIPresContext * 0x03008148, nsEvent * 0x0012f71c,
nsIDOMEvent * * 0x0012f390, unsigned int 514, nsEventStatus * 0x0012f540) line 3675
nsGenericElement::HandleDOMEvent(nsIPresContext * 0x03008148, nsEvent *
0x0012f71c, nsIDOMEvent * * 0x0012f390, unsigned int 519, nsEventStatus *
0x0012f540) line 1995 + 52 bytes
PresShell::HandleEventInternal(nsEvent * 0x0012f71c, nsIView * 0x036984e0,
unsigned int 1, nsEventStatus * 0x0012f540) line 6053 + 47 bytes
PresShell::HandleEvent(PresShell * const 0x0347b444, nsIView * 0x036984e0,
nsGUIEvent * 0x0012f71c, nsEventStatus * 0x0012f540, int 1, int & 1) line 5921 +
25 bytes
nsViewManager::HandleEvent(nsView * 0x036984e0, nsGUIEvent * 0x0012f71c, int 0)
line 2187
nsViewManager::DispatchEvent(nsViewManager * const 0x0370e890, nsGUIEvent *
0x0012f71c, nsEventStatus * 0x0012f66c) line 1973 + 20 bytes
HandleEvent(nsGUIEvent * 0x0012f71c) line 79
nsWindow::DispatchEvent(nsWindow * const 0x0349cf04, nsGUIEvent * 0x0012f71c,
nsEventStatus & nsEventStatus_eIgnore) line 1067 + 10 bytes
nsWindow::DispatchWindowEvent(nsGUIEvent * 0x0012f71c) line 1088
nsWindow::DispatchKeyEvent(unsigned int 131, unsigned short 0, unsigned int 114,
long 3997697) line 2978 + 15 bytes
nsWindow::OnKeyDown(unsigned int 114, unsigned int 61, long 3997697) line 3057
nsWindow::ProcessMessage(unsigned int 256, unsigned int 114, long 3997697, long
* 0x0012fc28) line 3899 + 38 bytes
nsWindow::WindowProc(HWND__ * 0x003707d0, unsigned int 256, unsigned int 114,
long 3997697) line 1349 + 27 bytes
USER32! 77e11ef0()
USER32! 77e1204c()
USER32! 77e121af()
nsAppShellService::Run(nsAppShellService * const 0x00f72c58) line 524
main1(int 2, char * * 0x00263fa8, nsISupports * 0x00ebeb28) line 1302 + 32 bytes
main(int 2, char * * 0x00263fa8) line 1779 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 7c58

Comment 6

[rbs]

Attachment: fix

Comment 7

[rbs]

Comment on attachment 147417 [details] [diff] [review]
fix

Simple fix to re-order a call of greater importance.

Comment 8

[Johnny Stenback (:jst)]

Comment on attachment 147417 [details] [diff] [review]
fix

sr=jst

Comment 9

[rbs]

Comment on attachment 147417 [details] [diff] [review]
fix

Fix checked in the trunk.

Asking a= for 1.7f. Simple nice-to-have, polish, no risk. One of these extras
that make the next release better than previous ones "in many ways".

Comment 10

[chris hofmann]

Comment on attachment 147417 [details] [diff] [review]
fix

a=chofmann for 1.7

Comment 11

[rbs]

Fixed on the 1.7 branch.