166704 - Alt+tabbing to another app & back can disable typeaheadfind

Status: VERIFIED FIXED

(SeaMonkey :: Find In Page, defect, P1)

Description

[Aaron Leventhal]

If you use Alt+Tab, but let go of the alt before the tab, typeaheadfind gets
disabled.

This is because typeaheadfind receives a DOMMenuBarActive event, but not a
DOMMenuBarInactive event. It turns out that ToggleMenuActiveState() uses
SetActive(PR_TRUE) to turn on menus, but it doesn't use SetActive(PR_FALSE) to
turn them off. SetActive is where the DOMMenuBar* events are fired.

This occurs on Windows, possibly on Linux.

Comment 1

[Aaron Leventhal]

Attachment: Patches nsMenuBarFrame.cpp, changes ToggleMenuActiveState()

Should get review from hyatt, he's the only one who's touched this code.
Otherwise, please tell me who should review it now.

Comment 2

[timeless]

i'm not sure which bug tracks it. but after i copy from browser and paste into
mailnews composer i can't actually type letters into it. I have to alt-tab back
to navigator, press a key and alt-tab back to mailnews composer. (w32 2002090408)

Comment 3

[Aaron Leventhal]

Timeless, I can't dup that bug. It's different than this bug anyway. You can
file a new bug if you like. Could you include more details of the behavior -
does it start searching for what you type while you're in mailcompose? Does the
status bar say anything?

Comment 4

[Kyle Yuan]

Comment on attachment 97855 [details] [diff] [review]
Patches nsMenuBarFrame.cpp, changes ToggleMenuActiveState()

looks good to me. there is one nit: seems InstallKeyboardNavigator() was called
twice. could you fix this too?

Comment 5

[Aaron Leventhal]

Attachment: Gets rid of unneeded call to InstallKeyboardNavigator(), and fixes comment to say we use the menuactive attribute

Seeking r=

Comment 6

[Kyle Yuan]

Comment on attachment 97910 [details] [diff] [review]
Gets rid of unneeded call to InstallKeyboardNavigator(), and fixes comment to say we use the menuactive attribute

r=kyle

Comment 7

[timeless]

fwiw my concern was bug 166955 which has been fixed

Comment 8

[Aaron Leventhal]

Kyle, could you test this on Linux or Solaris for me, and make sure that we
never lose the caret when Alt+Tabbing or using the menubar, and that everything
related to the menubar still works?

Comment 9

[Aaron Leventhal]

Attachment: Annotated patch describing each change

Comment 10

[Aaron Leventhal]

I tried all kinds of things on Windows, and couldn't see any difference. No
surprises there. Kyle is testing on Unix.

Also have the annotated patch describing each change attached to this bug.

Comment 11

[Johnny Stenback (:jst)]

Comment on attachment 97910 [details] [diff] [review]
Gets rid of unneeded call to InstallKeyboardNavigator(), and fixes comment to say we use the menuactive attribute

sr=jst

Comment 12

[Aaron Leventhal]

Asa gave a= on IRC

Comment 13

[Kyle Yuan]

tested this patch on Solaris, it works fine. 

aaron, some sr may dislike to put 1-line |if|/|else| bodies into a {} block (in 
SetActive()).

Comment 14

[Aaron Leventhal]

checked in

Comment 15

[sairuh (rarely reading bugmail)]

looks fixed on win2k and linux rh7.2, 2002.09.24.08 mozilla trunk.