401425 - Enter key sometimes handled twice.

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, defect, P2)

Description

[jag (Peter Annema)]

Regression from bug 385292.

In the browser, if I search (cmd+F) for text that doesn't appear in the document and hit enter (instead of clicking the Find button) I get the "Text not found" sheet twice. If I search in History and hit enter (instead of clicking the Find button) I get the "Search Results" window twice.

Comment 1

[jag (Peter Annema)]

I have some debug code in my tree that dumps the charCode and keyCode in ConvertMacToGeckoKeyCode. For "find in browser" this is what I see:

WARNING: ConvertMacToGeckoKeyCode: 36, 13: file ../../../../mozilla/widget/src/cocoa/nsChildView.mm, line 3380
WARNING: ConvertMacToGeckoKeyCode: 36, 13: file ../../../../mozilla/widget/src/cocoa/nsChildView.mm, line 3380
++WEBSHELL 0x44172cc0 == 22
++DOMWINDOW == 52
++DOMWINDOW == 53
WARNING: empty langgroup: file ../../../../mozilla/gfx/thebes/src/gfxFont.cpp, line 871
WARNING: empty langgroup: file ../../../../mozilla/gfx/thebes/src/gfxFont.cpp, line 871
WARNING: empty langgroup: file ../../../../mozilla/gfx/thebes/src/gfxFont.cpp, line 871
WARNING: ConvertMacToGeckoKeyCode: 36, 13: file ../../../../mozilla/widget/src/cocoa/nsChildView.mm, line 3380

<--- First "not found" sheet comes up, dismiss it with a click on OK

WARNING: getting z level of unregistered window: file ../../../../mozilla/xpfe/appshell/src/nsWindowMediator.cpp, line 635
WARNING: getting z level of unregistered window: file ../../../../mozilla/xpfe/appshell/src/nsWindowMediator.cpp, line 635
--WEBSHELL 0x44172cc0 == 21
WARNING: ConvertMacToGeckoKeyCode: 36, 13: file ../../../../mozilla/widget/src/cocoa/nsChildView.mm, line 3380
++WEBSHELL 0x44172cc0 == 22
++DOMWINDOW == 54
++DOMWINDOW == 55
WARNING: empty langgroup: file ../../../../mozilla/gfx/thebes/src/gfxFont.cpp, line 871
WARNING: empty langgroup: file ../../../../mozilla/gfx/thebes/src/gfxFont.cpp, line 871
WARNING: empty langgroup: file ../../../../mozilla/gfx/thebes/src/gfxFont.cpp, line 871
--DOMWINDOW == 54
--DOMWINDOW == 53

<--- Second "not found" sheet comes up, dismiss it with a click on OK

WARNING: getting z level of unregistered window: file ../../../../mozilla/xpfe/appshell/src/nsWindowMediator.cpp, line 635
WARNING: getting z level of unregistered window: file ../../../../mozilla/xpfe/appshell/src/nsWindowMediator.cpp, line 635
--WEBSHELL 0x44172cc0 == 21

=====

I'm guessing that the first two ConvertMac... are keydown and keyup, then the third (keydown?) is right before the first "not found" sheet is shown, and the fourth (keyup?) comes after the first sheet goes away, probably triggering the second sheet.

Comment 2

[jag (Peter Annema)]

And this is what's happening in the "find in history" case:

WARNING: ConvertMacToGeckoKeyCode: 36, 13: file ../../../../mozilla/widget/src/cocoa/nsChildView.mm, line 3380
WARNING: ConvertMacToGeckoKeyCode: 36, 13: file ../../../../mozilla/widget/src/cocoa/nsChildView.mm, line 3380
++WEBSHELL 0x3fdafa80 == 6
++DOMWINDOW == 11
++DOMWINDOW == 12
WARNING: ConvertMacToGeckoKeyCode: 36, 13: file ../../../../mozilla/widget/src/cocoa/nsChildView.mm, line 3380
++WEBSHELL 0x40545940 == 7
++DOMWINDOW == 13
++DOMWINDOW == 14
--WEBSHELL 0x40997a00 == 6
Failed to load jar:file:///Users/jag/moz-src/sm-debug/dist/SeaMonkeyDebug.app/Contents/MacOS/chrome/toolkit.jar!/content/global/nsTreeSorting.js
WARNING: ConvertMacToGeckoKeyCode: 36, 13: file ../../../../mozilla/widget/src/cocoa/nsChildView.mm, line 3380
WARNING: empty langgroup: file ../../../../mozilla/gfx/thebes/src/gfxFont.cpp, line 871
WARNING: empty langgroup: file ../../../../mozilla/gfx/thebes/src/gfxFont.cpp, line 871
WARNING: got a low Surrogate but no high surrogate: file ../../../dist/include/string/nsUTF8Utils.h, line 772
WARNING: got a low Surrogate but no high surrogate: file ../../../dist/include/string/nsUTF8Utils.h, line 693
WARNING: got a high Surrogate but no low surrogate: file ../../../dist/include/string/nsUTF8Utils.h, line 763
WARNING: got a High Surrogate but no low surrogate: file ../../../dist/include/string/nsUTF8Utils.h, line 681
<--- snip a bunch more of the surrogate stuff
JavaScript error: chrome://communicator/content/history/history.js, line 111: SortInNewDirection is not defined
<--- snip a bunch more of the surrogate stuff
JavaScript error: chrome://communicator/content/history/history.js, line 111: SortInNewDirection is not defined
WARNING: empty langgroup: file ../../../../mozilla/gfx/thebes/src/gfxFont.cpp, line 871
--DOMWINDOW == 13

So maybe it's the 3rd ConvertMac... (keydown?) that's triggering the second sheet / results window.

Comment 3

[jag (Peter Annema)]

This is the change I made to dump the keyCode and charCode:

@@ -3366,20 +3366,25 @@ static PRUint32 ConvertMacToGeckoKeyCode
...
     default:
+      nsCAutoString foo("ConvertMacToGeckoKeyCode: ");
+      foo.AppendInt((PRUint32)keyCode);
+      foo.Append(", ");
+      foo.AppendInt((PRUint32)charCode);
+      NS_WARNING(foo.get());
       // if we haven't gotten the key code already, look at the char code
       geckoKeyCode = GetGeckoKeyCodeFromChar(charCode);

Comment 4

[Jeremy Baron]

I've seen this (sounds like the same thing) when I click Create Profile in the profile manager and then hit enter, I end up seeing the profile name choosing screen flash by for a sec and then see the main profile manager screen again.

This has been happening for at least 2 weeks with the profile manager but I've only recently started using trunk again so it could be a much older regression.

This might have happened to me on Windows but I'm not sure.  (I'm seldom on Windows and I remember seeing this when I was using Windows and OS X side by side)

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Is this bug on Camino? Cannot reproduce on Fx?

Comment 6

[philippe (part-time)]

I don't have any problems like that in Camino trunk. So far :-).
No problems either with command+F in Minefield.

Comment 7

[jag (Peter Annema)]

I see this in SeaMonkey trunk on Intel Mac. Backing out the patch from bug 385292 fixes it.

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv1.0

The two key events are raised from:
1. keyDown
2. commandBySelector

Therefore, we should not raise the event if the super class will raise the command.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv1.0.1

This variable name is better, the previous one is misleadable.

Comment 10

[jag (Peter Annema)]

Isn't there already a way for the command (oncommand?) handler to let us know it handled a mouse click / keyboard event? How do other platforms do this?

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I think that it is no. Because the Korean IME uses insertNewline command, see bug 376093.

Comment 12

[Josh Aas]

+    NS_ERROR("The KEY_PRESS event is not rised, and will not be handled by command.");

"No KEY_PRESS event will be sent" is probably fine.

+    case kReturnKeyCode:
+    case kEnterKeyCode:
+    case kPowerbookEnterKeyCode:

Masayuki - can you explain why you chose these keys? insertText: sendsNS_KEY_PRESS events for other types of keys, why did you single out those three?

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #12)
> +    case kReturnKeyCode:
> +    case kEnterKeyCode:
> +    case kPowerbookEnterKeyCode:
> 
> Masayuki - can you explain why you chose these keys? insertText:
> sendsNS_KEY_PRESS events for other types of keys, why did you single out those
> three?

I chose the line-feed inputing keys. I'm not sure this is correct, but I don't have another idea...

Comment 15

[Josh Aas]

Jag - does the first patch on 396832 fix this bug?

Comment 16

[jag (Peter Annema)]

(In reply to comment #15)
> jag - does the first patch on 396832 fix this bug?

Yes it does. I was hoping it would be something a little simpler than the patch in this bug.

Comment 17

[Josh Aas]

fixed by patch for bug 396832

Comment 18

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

Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b2pre) Gecko/2007111222 Minefield/3.0b2pre

Comment 19

[Tony Chung [:tchung]]

This is fixed in the nightly, but not on b1.  The other thing to note for beta1 release notes, is that double entries of New Folder is created on the bookmarks toolbar - when creating a new bookmarks folder.

Comment 20

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

(In reply to comment #19)
> release notes, is that double entries of New Folder is created on the bookmarks
> toolbar - when creating a new bookmarks folder.

Not only folders. Also creating new bookmarks shows this issue. Places Organizer is also affected. But have a look at the dependencies list of all tracked issues.

Comment 25

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

This one has reappeared with the latest nightly builds. Shall I file a new bug for or should this be reopened?

Comment 26

[jag (Peter Annema)]

Please open a new bug. Regression from bug 358379?

Comment 27

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

This is already bug 420502.