266822 - crash if right-clicking on a text box, wait for text box to go away, press escape [@ nsMenuListener::KeyPress]; affects find bar in Firefox

Status: VERIFIED FIXED

(Core :: DOM: Events, defect)

Description

[Ori Avtalion (salty-horse)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20041026 Firefox/1.0RC1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20041026 Firefox/1.0RC1

This crash was originally reported in the second comment of bug 258781 by Jordan
Liggit. Since it didn't get a response from devs (it was out of context, and the
actual reported bug was dismissed), I now create a new entry for it.


Reproducible: Always
Steps to Reproduce:
1. Navigate to any website
2. Make the Find Toolbar appear either by searching something.
3. When the toolbar appears, right-click in the textbox to display the 
context menu
4. Wait for the toolbar to disappear
5. Hit the 'Esc' key

Actual Results:  
Browser crashes.


I only tested this on Windows XP

Comment 1

[Phil Ringnalda (:philor)]

Talkback ID 1636197

Comment 2

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

I crashed here:
http://lxr.mozilla.org/aviarybranch/source/layout/xul/base/src/nsMenuListener.cpp#203

handled = 0
theChar = 27
mMenuParent = 0x02bf3448
 nsISupports = {...}
  __vfpt = 0xdddddddd
this = 0x02c8ffe0
 (the rest of "this" looks fine)

Comment 3

[Ori Avtalion (salty-horse)]

(In reply to comment #0)

After more reseach and realizing I had some typos:

1) It only works when the timeout is enabled, which means you can't use ctrl+F.
2) Possible fix:
 toolkit/components/typeaheadfind/content/findBar.js :
 When right clicking on the find text-box, set gFindMode to FIND_NORMAL.
 It is logical, because if the user wants to edit something in the text box, it
should not disappear in the middle of the task, and it will prevent the crash.

Comment 4

[Asaf Romano (gone)]

only ADT has the right to + bugs. You can set ?

Comment 5

[Asaf Romano (gone)]

err, i wasn't clear. you can set "?"

Comment 6

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

The problem appears to be that somebody (nsMenuPopupFrame?) creates a
nsMenuListener, and after the timer expires the parent of the menu listener is
deleted, so the next time the listener dereferences it's parent, crash.

Comment 7

[Boris Zbarsky [:bzbarsky]]

As I recall, trunk got some patches to handle menu teardown properly in the 1.8a
cycle.  So this is probably fixed on trunk, if someone can test it (I'm not sure
the find toolbar is on trunk, of course).

Comment 8

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

->Browser
Testcase coming in a moment.

Comment 9

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: Testcase

Comment 10

[[e-mail]]

Attachment: Proposed patch

Comment 11

[[e-mail]]

Fix checked in.

Comment 12

[Mike Kaply [:mkaply]]

Comment on attachment 164119 [details] [diff] [review]
Proposed patch

Looks like a nobrainer for 1.7. email aviary for aviary

Comment 13

[Asa Dotzler [:asa]]

Comment on attachment 164119 [details] [diff] [review]
Proposed patch

a=asa for aviary checkin. Time is very short for Firefox 1.0 so please land
this quickly.

Comment 14

[Steffen Wilberg]

Checked into aviary.

Comment 15

[sairuh (rarely reading bugmail)]

what happens using 2004110609-0.11 on linux fc2:

1. bring up find toolbar (I hit the / FAYT command)
2. bring up context menu for textfield in find toolbar
3. wait a few seconds for FAYT to time out.

results: both the find toolbar *and* context menu go away (no chance to hit Esc
key to dismiss context menu). is this expected now?

Comment 16

[Ori Avtalion (salty-horse)]

(In reply to comment #15)
> results: both the find toolbar *and* context menu go away (no chance to hit Esc
> key to dismiss context menu). is this expected now?

To this problem, my suggestion on comment #3 would suffice: Whenever the user
interacts with a find toolbar that has a timer (one which was invoked using
FAYT), disable it, and turn the toolbar into a regular CRTL+F one (gFindMode =
FIND_NORMAL in the findBar.js code).
It is not a necessity, just a UI improvement.

Comment 17

[Steffen Wilberg]

> results: both the find toolbar *and* context menu go away (no chance to hit Esc
> key to dismiss context menu). is this expected now?
I think so. If the find bar is gone, there is nothing to cut/copy/paste anymore.
Ori is right IMHO: The find bar shouldn't go away in the first place if you
opened a context menu. But that's another (find-bar) bug.

Comment 18

[sairuh (rarely reading bugmail)]

Steffen, thanks for the clarification! vrfy'ing this particular bug as fixed.

Comment 19

[Ori Avtalion (salty-horse)]

(In reply to comment #17)
Opened bug #268305

Comment 20

[Adam Guthrie]

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