Bug 11628 - REGRESSION (r17597): Command-return in text fields doesn't open a new tab or window
Summary: REGRESSION (r17597): Command-return in text fields doesn't open a new tab or ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Darin Adler
URL:
Keywords: Regression
Depends on:
Blocks:
 
Reported: 2006-11-17 04:20 PST by Daniele Metilli
Modified: 2006-12-17 09:29 PST (History)
2 users (show)

See Also:


Attachments
patch (48.83 KB, patch)
2006-12-01 14:14 PST, Darin Adler
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniele Metilli 2006-11-17 04:20:09 PST
In the old fields, command-click allowed the user to open a new tab or window. In the new ones, this doesn't work anymore.
Comment 1 Alexey Proskuryakov 2006-11-18 02:04:16 PST
Could you please provide an example? I'm afraid I don't understand what the issue is.
Comment 2 Daniele Metilli 2006-11-18 02:26:56 PST
Oops, I meant Command-return, not Command-click!

Example:

1 - Go to imdb.com
2 - Type something in the field on the left
3 - Command-return

The resulting page will be loaded in the same tab, and it shouldn't.
Comment 3 Alexey Proskuryakov 2006-11-18 13:01:28 PST
Confirmed.
Comment 4 mitz 2006-11-21 09:12:17 PST
The regression is due to r17597, specifically "added DOM Event parameters to various functions so that the handlers can use the DOM Event instead of the global "current NSEvent";".

The problem is that when a Command-Return keyboard event triggers a simulated click event, the click event doesn't inherit the modifiers from the mouse event. This affects not only form fields but also focused links, and breaks to the Option modifier as well. I think everything funnels through HTMLElement::click(), and in the relevant cases the caller to click() still has the original keyboard event which it could pass along if click() were modified to take it.
Comment 5 Darin Adler 2006-11-28 17:11:16 PST
Sounds like a good approach. I'll try to fix this.
Comment 6 Darin Adler 2006-12-01 13:44:03 PST
Got a patch -- writing release notes for review.
Comment 7 Darin Adler 2006-12-01 14:14:45 PST
Created attachment 11706 [details]
patch
Comment 8 mitz 2006-12-01 14:51:00 PST
Comment on attachment 11706 [details]
patch

r=me
Comment 9 Darin Adler 2006-12-01 14:58:24 PST
Committed revision 17976.
Comment 10 Darin Adler 2006-12-01 14:58:44 PST
Note that this actually had nothing to do with native text fields!
Comment 11 mitz 2006-12-02 13:00:09 PST
Oops, Command-Return on a focused link is still broken.
Comment 12 mitz 2006-12-02 13:32:52 PST
(In reply to comment #11)
> Oops, Command-Return on a focused link is still broken.
> 

Two reasons for that. One is that HTMLAnchorElement::defaultEventHandler() neglects to pass the original event to dispatchSimulatedClick(). The other is that even if it did, findKeyStateEvent() stops at the simulated click event, which doesn't have the correct modifier state.

I'm reopening this bug, but if you feel that a new bug is needed, just close this one again and I'll file a new one.
Comment 13 mitz 2006-12-02 13:51:26 PST
(Forgot to reopen)
Comment 14 mitz 2006-12-02 13:54:59 PST
Comment on attachment 11706 [details]
patch

Clearing the review flag to keep the bug from showing in the commit queue.
Comment 15 mitz 2006-12-04 22:42:42 PST
Darin fixed the "focused link" case in r18012 (even though he would have preferred having a separate bug for that case!). Thanks, Darin!
Comment 16 mitz 2006-12-17 09:29:49 PST
Command-clicking a form's submit button is still broken. I will file a separate bug on that :-)