420502 - Using enter/return in dialogs runs the underlying command twice

Status: VERIFIED FIXED

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

Description

[Keng Susumpow]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.5; en-US; rv:1.9b4pre) Gecko/2008030111 Minefield/3.0b4pre
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.5; en-US; rv:1.9b4pre) Gecko/2008030111 Minefield/3.0b4pre

Problem:
Duplicated bookmark folder creation on Minefiled nightly build


Reproducible: Always

Steps to Reproduce:
1.  Right click on Bookmark Toolbar and select "New Folder..."
2. Create new Bookmark folder by put Name: "Test" then click "Add"
3. You will see 2 bookmark folders named "test" on the Bookmark Toolbar
Actual Results:  
Duplicated bookmark folder creation

Expected Results:  
Bookmark folder should be created only one

Tested on Mac OS X 10.5.2 with Minefield build 2008030111

Comment 1

[Keng Susumpow]

Attachment: screenshot of duplicated bookmark folders creation

screenshot of duplicated bookmark creation

Comment 2

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

This is a recent regression. Probably raised by the patch on bug 358379. Haven't seen this the days before. 

Comment 3

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

This doesn't only happen for new folders or bookmarks. Even the creation of new profiles during start-up is affected by this regression. When using enter/return to finally create the profile runs the command twice which results in two new profile entries. Updating summary...

Comment 4

[jag (Peter Annema)]

Bug 401425 might be of help in understanding/fixing this bug.

Comment 5

[philippe (part-time)]

I didn't meant to reset the blocking flag; bad Bugzilla mid-air collision.

Comment 6

[Josh Aas]

In the repro steps here, step 2 is incorrect - clicking add does not repro but hitting enter does.

Comment 7

[Josh Aas]

Attachment: quick fix v1.0

I can't work on this more tonight, but here is a quick fix. I don't necessarily endorse this as something we should try to land, but it gets us somewhere. I'll do more tomorrow.

Comment 8

[Josh Aas]

I should have been more clear - that patch does fix the problem for me and I didn't find any regressions in the 2 minutes of incomplete testing that I did. I just haven't taken the time to figure out if that is the right thing to do yet.

Comment 9

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

No, it's wrong. It looks like creating IME events. You should check mSentKeyPress in the if block. And if it's true, we should return.

Comment 11

[Michael Schonfeld]

Attachment: v2

Patch based on Masayuki Nakano comment #9, only instead of mSentKeyPress, it's mKeyPressSent.

BTW, this bug also affects the feed subscription. See (dupe) bug 420467.

Comment 12

[Josh Aas]

Comment on attachment 306854 [details] [diff] [review]
v2

makes sense, we should take this for b4

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 306854 [details] [diff] [review]
v2

a1.9b4=beltzner

Josh: we can take this, but does it block b4? If not, please change the TM to be "mozilla1.9"

Comment 14

[Reed Loden [:reed]]

Checking in widget/src/cocoa/nsChildView.mm;
/cvsroot/mozilla/widget/src/cocoa/nsChildView.mm,v  <--  nsChildView.mm
new revision: 1.314; previous revision: 1.313
done

Comment 15

[Josh Aas]

This should block b4, thanks for checking it in Reed.

Comment 16

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

Works fine. Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b4pre) Gecko Minefield/3.0b4pre ID:2008030308

Comment 17

[Keng Susumpow]

Seems this bug has been fixed.

Verified with Nightly Build on Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.5; en-US; rv:1.9b4pre) Gecko/2008030304 Minefield/3.0b4pre