296526 - While autocomplete list is visible opening the context menu causes a misplacement, the focus is lost and a crash occurs when trying to load another URL [@nsAutoCompleteController::HandleEnter]

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

This bug describes two issues where the second mayor problem probably could be
eleminated by fixing the first one.

Using the context menu key on your keyboard while the autocomplete list of an
edit field is visible, the context menu will open and the autocomplete list is
moved down-right. It is still open and isn't get closed when displaying the
context menu.

Now if you select an entry with your mouse and the text is autocompleted the
focus is gone forever for this window. You can open new tabs, close them or try
other things. You won't get back the focus. A new window solves it.

I tested with builds of the Aviary-Branch and the Deer Park Alpha 1 release.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

If you are trying to load another URL over the location bar Fx crashes. I can't
send the report at this time because I don't get a connection over the proxy server.

=> crash, critical

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Talkback id: TB6427028K

The crash is similar to bug 280084 which is still be fixed? I can reproduce it
on my trunk build. If it's the same we shouldn't dupe this bug. Instead I'll
remove the crash keyword and this bug only handles the missplacement of the ui
elements.

Stack Signature	 nsAutoCompleteController::HandleEnter b787c5f1
Product ID	FirefoxTrunk
Build ID	2005053112
Trigger Time	2005-06-06 02:47:21.0
Platform	Win32
Operating System	Windows NT 5.0 build 2195
Module	firefox.exe + (003e3438)
URL visited	
User Comments	bug 296526
Since Last Crash	86 sec
Total Uptime	2818 sec
Trigger Reason	Access violation
Source File, Line No.
c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/toolkit/components/autocomplete/src/nsAutoCompleteController.cpp,
line 260
Stack Trace 	
nsAutoCompleteController::HandleEnter 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/toolkit/components/autocomplete/src/nsAutoCompleteController.cpp,
line 260]
XPTC_InvokeByIndex 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp,
line 102]
XPCWrappedNative::CallMethod 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp,
line 2100]
XPC_WN_CallMethod 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp,
line 1348]
js_Invoke 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1182]
js_Interpret 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 3473]
js_Invoke 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1202]
js_InternalInvoke 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1279]
JS_CallFunctionValue 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsapi.c, line 3858]
nsJSContext::CallEventHandler 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/dom/src/base/nsJSEnvironment.cpp,
line 1386]
nsJSEventListener::HandleEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/dom/src/events/nsJSEventListener.cpp,
line 184]
nsXBLPrototypeHandler::ExecuteHandler 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/xbl/src/nsXBLPrototypeHandler.cpp,
line 500]
nsXBLKeyEventHandler::HandleEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/xbl/src/nsXBLEventHandler.cpp,
line 143]
nsEventListenerManager::HandleEventSubType 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/events/src/nsEventListenerManager.cpp,
line 1568]
nsEventListenerManager::HandleEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/events/src/nsEventListenerManager.cpp,
line 1669]
nsXULElement::HandleDOMEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/xul/content/src/nsXULElement.cpp,
line 2194]
nsXULElement::HandleDOMEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/xul/content/src/nsXULElement.cpp,
line 2173]
nsXULElement::HandleDOMEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/xul/content/src/nsXULElement.cpp,
line 2173]
nsGenericElement::HandleDOMEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/base/src/nsGenericElement.cpp,
line 2079]
nsHTMLInputElement::HandleDOMEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/html/content/src/nsHTMLInputElement.cpp,
line 1380]
PresShell::HandleEventInternal 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp,
line 6324]
PresShell::HandleEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp,
line 6167]
nsViewManager::HandleEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/view/src/nsViewManager.cpp,
line 2457]
nsViewManager::DispatchEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/view/src/nsViewManager.cpp,
line 2224]
HandleEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/view/src/nsView.cpp, line
174]
nsWindow::DispatchEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 1180]
nsWindow::DispatchKeyEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 3491]
nsWindow::OnKeyDown 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 3629]
nsWindow::ProcessMessage 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 4495]
nsWindow::WindowProc 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 1472]
USER32.dll + 0x2a3d0 (0x77e2a3d0)
USER32.dll + 0x4605 (0x77e04605)
USER32.dll + 0xa7ba (0x77e0a7ba)
nsAppStartup::Run 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/toolkit/components/startup/src/nsAppStartup.cpp,
line 145]
main 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/browser/app/nsBrowserApp.cpp,
line 61]
KERNEL32.DLL + 0x2893d (0x77e9893d)

Comment 3

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

Is there a regression range for this?

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Should this be a regression? IMHO it's there since autocomplete was integrated.
But I didn't run tests with a version earlier than Fx 1.0.4 so I'm ansure.

Should we remove the crashing part here? If yes, this bug also covers Linux.

Comment 5

[gavin long]

This is biting me pretty hard on Win XP, Firefox 1.5 beta 1 (Mozilla/5.0
(Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4)

Talkback examples TB9247775G, TB9478684Q, TB9479312H, stack trace pretty much as
above.

I can't reproduce at will (it usually takes half an hour or so) but I know that
it usually occurs when:
- I'm typing in the address bar
- There are downloads running in the background

I also have a hunch that it's linked to other applications/tasks stealing focus
from Firefox.

As a precursor to the crash, I usually start to get odd focusing issues.  For
example, I'll focus the address bar and try to type, but my keystrokes go to odd
places (backspace triggering the back button is a common one).  Switching
windows will usually resolve the focus issues. I suppose that could be a
different bug, but it has an uncanny knack of being followed by a crash within a
few minutes.

In any case, I'm pretty sure that I didn't have these issues in 1.0.6 on the
same box.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

There are over 600 talkback ids available. Most of them describing the same
crash issue with autocomplete and especially on google searchbar.

Any possibility to get this fixed soon?

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

List of talkback reports:

http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=1&searchby=stacksig&match=contains&searchfor=nsAutoCompleteController%3A%3AHandleEnter&vendor=All&product=All&platform=All&buildid=&sdate=&stime=&edate=&etime=&sortby=bbid

Comment 8

[Mike Schroepfer]

Brian - can you take a look?

Comment 9

[Brian Ryner (not reading)]

Just for reference - if you don't have a context menu key, you can reproduce
this using shift+F10.

Comment 10

[Benjamin Smedberg]

Attachment: Wallpaper, rev. 1 [fixed on trunk and 1.8 branch]

This is untested, but it looks like what might be happening.

Comment 11

[Brian Ryner (not reading)]

Comment on attachment 198018 [details] [diff] [review]
Wallpaper, rev. 1 [fixed on trunk and 1.8 branch]

This won't solve the problem of the popup moving when the context menu appears,
but is probably a good change anyway.

Comment 12

[Benjamin Smedberg]

Comment on attachment 198018 [details] [diff] [review]
Wallpaper, rev. 1 [fixed on trunk and 1.8 branch]

I'll leave the bug open for the misplaced popup, but this should at least fix
the crash.

Comment 13

[Benjamin Smedberg]

The 1.8 necessary bits of this bug (the topcrash) are fixed, so marking
fixed1.8: the mispositioning is not 1.8 blocker.

Comment 14

[Asa Dotzler [:asa]]

This appears to not be fixed in the beta2 release. It's showing up in the top 10
crashers still.

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to comment #14)
> This appears to not be fixed in the beta2 release. It's showing up in the top 10
> crashers still.

Currently 43 crashes can be found:
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=1&searchby=stacksig&match=contains&searchfor=nsAutoCompleteController%3A%3AHandleEnter&vendor=All&product=Firefox15&platform=Win32&buildid=2005100614&sdate=&stime=&edate=&etime=&sortby=bbid

Comment 16

[Mark Mentovai]

Henrik, can you reproduce the crash according to your original steps in comment
0 (and 1)?  I couldn't on Windows or Linux, although on Windows, the steps are
successful in killing focus.  If the patch above prevented this from crashing,
then something else is responsible for the 215 crashes in Talkback.

Comment 17

[gavin long]

I just got hit by this again on Beta 2.  Talkback: TB10993742Q

The stack trace looks to be superficially the same as that in comment 2, with some line number differences.

Same as before, I was editing a URL in the address bar, hit enter, -> crash.

I used to see this a LOT in beta 1, but this is the first time it's given me this crash with beta 2 in a couple of weeks.  I don't *think* I've modified my behaviour much, but I could be wrong.

Comment 18

[Benjamin Smedberg]

Comment 17 is a different crash, probably due to mInput being null at http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/toolkit/components/autocomplete/src/nsAutoCompleteController.cpp&mark=260&rev=#260

And the situations in which mInput are null are different from popup being null. The comment at 361 indicates that there are conditions where we need to null-check mInput also :-(

I could provide a blanket patch for null-checking mInput in lots of places, but I'm not sure I understand the flow enough to do it intelligently.

Comment 19

[gabe]

TB14022778W ive had a crash in the url bar not sure if its related

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to comment #19)
> TB14022778W ive had a crash in the url bar not sure if its related

It's the same stack trace which I posted in comment 2. Benjamin it seems that the crash isn't fully fixed by your wallpaper patch.

Looking at talkback gives you a list of 1788 crash reports within the last days! This should be a blocker for the next security release.

Comment 21

[Martijn Wargers (dead)]

Attachment: patch

Well, this fixes the original bug report as mentioned in comment 0, so I think it would also fix the crash issue.
The patch adds a context menu listener, which - when a context menu appears - calls handleEscape to make the autocomplete popup disappear.

Comment 22

[Martijn Wargers (dead)]

One nasty side-effect of the patch is that a context menu resets the selection in the url bar, so I need to change the patch to use closePopup or something probably.

Comment 23

[Daniel Veditz [:dveditz]]

Too late for 1.8.0.1. Need a trunk-tested fix before we can consider taking on the stability/security branch

Comment 24

[Martijn Wargers (dead)]

Attachment: patch2

Ok, this works well without (known) nasty side-effects.

Comment 25

[Daniel Veditz [:dveditz]]

Comment on attachment 209090 [details] [diff] [review]
patch2

Brian, can you review this, and give us input on whether it's appropriate for the 1.8.0 branch

Comment 26

[Brian Ryner (not reading)]

Comment on attachment 209090 [details] [diff] [review]
patch2

Looks ok, but get rid of the unneeded whitespace changes.

I think this would be acceptable and low-risk for the branch.

Comment 27

[Martijn Wargers (dead)]

I don't need supperreview here, right?

Comment 28

[Brian Ryner (not reading)]

Comment on attachment 209090 [details] [diff] [review]
patch2

please go ahead and land this

Comment 29

[Daniel Veditz [:dveditz]]

Comment on attachment 209090 [details] [diff] [review]
patch2

marking bryner's 1.8.1 approval as noted in bug, approved for 1.8.0 branch, a=dveditz for drivers

Comment 30

[Martijn Wargers (dead)]

Attachment: trunk patch without white-space changes

This is the patch without the unnecessary white-space changes, which I'm going to check in.

Comment 31

[Brian Ryner (not reading)]

Oops, I didn't realize you were ready to check this in, and just did so on the trunk and both branches.  I didn't fix up the whitespace change, sorry.

Comment 32

[Martijn Wargers (dead)]

No problem. Thank you for checking it in! (I was indeed about to check it in)

Comment 33

[Tracy Walker [:tracy]]

verified with:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.5) Gecko/20060620 Firefox/1.5.0.5