New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 15090 link

Starred by 21 users

Issue metadata

Status: Verified
Owner:
Closed: Oct 2009
Cc:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug
M-4

Blocked on:
issue 24817
issue 24877
issue 24921
issue 25000
issue 25249
issue 25254

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment

While ⌘ (command) key held down, letter keys don't generate key events

Reported by jasneet@chromium.org, Jun 23 2009

Issue description

Chrome version : 3.0.190.0 (Official Build 18973)

What steps will reproduce the problem?
1. Login to google docs
2. Open any spreadsheet
3. Copy and paste any text using ⌘ + C, ⌘ + V or using menu > edit >copy and paste

What is the expected output?
Copy paste should happen

 What do you see instead?
The text is not copy pasted

Please use labels and text to provide additional information.

 
Labels: Mstone-MacBeta

Comment 2 by Deleted ...@, Jul 21 2009

Same problem, copy/paste is limited to the Google document, so it is separate from the 
desktop copy/paste--the clipboard isn't shared.  That is a non-starter for me, does not 
meet requirements.  

Comment 3 Deleted

Status: Assigned

Comment 5 by a...@chromium.org, Aug 12 2009

mjgoldsmith—

This is an issue with the Mac version of Chromium. If you're having issues with a
non-Mac version, please file a bug.

Comment 6 by a...@chromium.org, Aug 12 2009

alhartmann—

The copy/paste that you get with a right click is not addressed in this bug. That is
a design issue with Google Docs.

Comment 7 by a...@chromium.org, Aug 12 2009

OK, this seems to be a pasting issue. The content is copied properly using tabs and
returns, and we can copy from Chromium and paste in Safari just fine. However,
pasting tab-delimited content is failing.

Comment 8 Deleted

Comment 9 by a...@chromium.org, Aug 13 2009

Status: Started

Comment 10 by a...@chromium.org, Aug 13 2009

Summary: While ⌘ key held down, letter keys don't generate key events
Repro:
0. Go to http://unixpapa.com/js/testkey.html
1. Hold down the command key
>> See keydown event
2. Press the V key
>> Should see keydown/keypress, but don't see anything
3. Release the command key
>> See keyup event

BTW, pasting into Google Spreadsheets from the menu is broken due to a bug on their
side; it doesn't work in any Mac browser.

Comment 11 by a...@chromium.org, Aug 14 2009

Here's what's going on:

Command keys are handled differently than any other type of keystroke. While
experienced Carbon hands would expect that command keys would propagate like any
other key event, and that when it hit the application event handler would be turned
into a menu event, that's not the case at all.

Command keys are handled by sending them down the performKeyEquivalent: call on the
view. If no one handles it then it is processed by the menu.

WebKit handles it in WebHTMLView.mm, where they send the event to WebCore, and only
pass it through if it wasn't handled. We can't quite do that since we have IPC to do.

The plan is, then, in RWHV to always return YES to performKeyEquivalent: (to indicate
we're handling the event) and then to pass it to WebCore. When TCV gets the message
to handle an unhandled event from WebCore, it will set a flag around a call to
process the event:

processingKeyEquivalent_ = YES;
[NSApp sendEvent:event]
processingKeyEquivalent_ = NO;

and then have a performKeyEquivalent: override in TCV that just returns NO if
processingKeyEquivalent is YES, so that we don't infinitely loop.

In implementing this idea, I found that WebCore actually eats ⌘-C if it sees it. It's
never had the chance to in Chromium before, so we were triggering the menu which
fires off an editor action. But if we actually route command key events into WebCore,
deep down it eats the ⌘-C and does a paste, which is very surprising. From the WebKit
code, it would seem that it wouldn't do that, but it is. I'm still digging up what's
going on.

Comment 12 by a...@chromium.org, Aug 14 2009

In event processing, the event gets shipped off to the editor's handleKeyboardEvent,
which figures out that it's a paste and pastes. That's the code path that the Windows
version takes, but we don't want to take it. We want to ignore it, re-propagate it up
the event tree, and let the menu handle it.

In WebKit, handleKeyboardEvent calls into the same type of code. I'm not yet sure why
it's not doing this too.

Comment 13 by a...@chromium.org, Aug 14 2009

Summary: While ⌘ (command) key held down, letter keys don't generate key events
It's not entirely clear why WebKit's editor code isn't executing a command; it seems
like it's doing a lookup and not finding anything.

The real killer here is that feeding the event back into the system with sendEvent:
doesn't actually fire off the shortcutted menu item. Whee.

Comment 14 by ben@chromium.org, Aug 17 2009

 Issue 19533  has been merged into this issue.
Labels: -Area-Misc Area-WebKit

Comment 17 by jon@chromium.org, Sep 2 2009

Labels: ReleaseBlock-Beta
This is a release blocker for Mac Beta.

Comment 18 by jon@chromium.org, Sep 2 2009

Labels: Mstone-4
We are deprecating the MacBeta milestone in favor of ReleaseBlock-Beta and 
a milestone.

Comment 19 by a...@chromium.org, Sep 11 2009

Even better, arrow keys are handled by performKeyEquivalent: too. Whee. :(
Labels: -Pri-2 Pri-1
 Issue 23198  has been merged into this issue.

Comment 22 by suzhe@chromium.org, Sep 28 2009

Comment 23 by james...@gmail.com, Sep 29 2009

Any progress on this issue?
I'm just wondering if it's necessary to override NSApplication's sendEvent: method to
send all key events to RWHV, then in TCV, feeds all unhanded key events back to the
original sendEvent: implementation for cmd- key bindings handling?
+jam
Status: Assigned
James: Please read avi's comments, there's some fighting with Cocoa going on here. He sent me his work-in-progress 
patch, which iirc looks exactly like what you described.

Anyway, I'll look into this once I've made cmd-left/right work for history.

Comment 26 by suzhe@chromium.org, Sep 29 2009

Thanks for your feedback. I'm not familiar with Cocoa programming, actually I'm learning it these days. Feel free 
to ignore my comment if it makes no sense :-)

Does avi's WIP patch work? Can you send it to me?
suzhe: I've uploaded avi's patch to http://codereview.chromium.org/242069 . I can't 
see the "The real killer here is that feeding the event back into the system with 
sendEvent: doesn't actually fire off the shortcutted menu item." problem Avi describes, 
cmd-t seems to work if it's not captured but it _is_ captured correctly at 
http://www.quirksmode.org/js/keys.html . The patch (intentionally) DCHECK()s when 
arrow keys are pressed, but it's a good start.

Comment 28 by suzhe@chromium.org, Sep 30 2009

Thanks. I just read it and so good that it looks rather simple. However, this approach 
is different than what I proposed: to override NSApplication's sendEvent: method and 
dispatch key events to RWHV directly, then feed unhandled key events back into NSApp's 
original sendEvent: method in TCV. This approach can avoid feeding a key event into 
NSApp's sendEvent: twice, which looks weird.

Disclaimer: I'm not sure if my proposal is feasible, which is just an idea in my mind. 
Hope it help.
Useful reading for anyone following this thread:

<http://developer.apple.com/mac/library/documentation/Cocoa/Conceptual/EventOverview/EventArchitecture/EventArchitecture.html#//apple_ref/doc/uid/10000060i-CH3-SW10>

(in particular Figure 1-5). If I understand (and recall) correctly, key equivalents
are dispatched before |-sendEvent:|, and no standard Cocoa application ever sees
normal key events for Cmd-letter_key combinations. We can fake key-down and key-press
events when Cmd is held down, but not do key-up. (This is the case for all Mac
browsers I've tested.)
Another good material:
http://developer.apple.com/mac/library/documentation/Cocoa/Conceptual/TextEditing/Concepts/OverviewEditi
ng.html#//apple_ref/doc/uid/20000929-BBCFEBHA

See Figure 1.

Comment 31 by jam@chromium.org, Oct 2 2009

Note: whoever implements this will also want to implement 
BrowserWindowCocoa::GetCommandId which returns a command id given a native keyboard 
event.  This enables a subset of keyboard shortcuts to bypass the renderer, see issue 
5496 and r27814 for more information.
Status: Started
http://codereview.chromium.org/242069 kinda works modulo the change requested 
by jam. It's based on avi's approach. It doesn't send keyups for cmd+key, like other 
os x browsers. It suppresses the default action only if keydown() is suppressed but 
not if keypress() is suppressed – this matches chrome on windows and linux and IE8, 
but doesn't match safari and firefox. Matching chrome on win/linux sounds like the 
right thing.

I'd like to write a version based on james's suggestion. I think it would make it 
possible to send keyups for cmd+key (although we probably shouldn't do that 
anyway, for web compat), and it might be a bit cleaner.
http://codereview.chromium.org/271054 is the sendEvent-based patch. It's a bit 
simpler, but both Safari ( 
http://trac.webkit.org/browser/trunk/WebKit/mac/WebView/WebHTMLView.mm ) 
and Firefox ( http://mxr.mozilla.org/mozilla-
central/source/widget/src/cocoa/nsChildView.mm#5436 ) use the 
performKeyEquivalent-based approach. Since WebKit needs to work when embedded 
into arbitrary os x apps, it's unlikely that they'll ever use the sendEvent-based 
approach, and hence they will probably never send keyup()s to the renderer for cmd-
keypresses (one of the major advantages of the approach).

Not sure which patch is better. Opinions?
Labels: -Area-WebKit Area-BrowserUI
Status: Fixed
The sendEvent-based one is cleaner, simpler, and more versatile (but in ways we don't 
need). The pKE-based one is closer to safari's and firefox's code, less intrusive, and less 
surprising (someone new to the project wouldn't know that keyboard handling for 
rwhvcocoa is special-cased in the app).

Chatted with Avi, and we both like the pKE-based version better, so I checked that in. 
Both patches are pretty small, so we can switch to the other version in the future, 
should we have to.

Comment 36 by suzhe@chromium.org, Oct 13 2009

I'm ok with it. Thanks a lot.
Status: Started
Dang, I forgot to implement that method in BrowserWindowCocoa that jam mentioned 
above – not fixed.
CL for the missing part: http://codereview.chromium.org/273032
Blockedon: 24817 24877 24921 25000
Blockedon: 25249
Blockedon: 25254
Status: Fixed
For the records: We went with the sendEvent-based approach after all, because with the 
pKE-approach the key view loop processing swallowed ctrl-tab (see  issue 24921 ).

This bug is fixed with http://src.chromium.org/viewvc/chrome?
view=rev&revision=30032 . One of the bugs this is blocked on ( issue 25254 ) has a CL 
out and is nearly fixed, and the other blocking issue isn't really related to this bug, so 
I'm marking this as fixed.
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=30032 

------------------------------------------------------------------------
r30032 | thakis@chromium.org | 2009-10-25 21:40:12 -0700 (Sun, 25 Oct 2009) | 12 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/blocked_popup_container_interactive_uitest.cc?r1=30032&r2=30031
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/cocoa/browser_window_cocoa.mm?r1=30032&r2=30031

Do not send some keyboard shortcuts to the renderers

Walking the whole menu on every keypress seems ridiculous. Linux does this too :-/ (Caching is made hard because the user can change key equivalents in system preferences at every point in time, and we're not notified of that. And people only hit max 5 keys/second, so it's not all that ridiculous).

There's a UI test for this, but the interactive UI tests are not enabled on OS X, so it's not executed.

Menu walking code based on code from CocoatechCore.

BUG= 5496 , 15090 , 24877 
TEST=Go to http://unixpapa.com/js/testkey.html , check "keydown", focus the textbox, and make sure that cmd-t still opens tabs, cmd-shift-[ still switches tabs, cmd-w still closes tabs. however, cmd-L should not focus url bar and cmd-1 should not go to the first tab.

Review URL: http://codereview.chromium.org/273032
------------------------------------------------------------------------

Status: Verified
4.0.225.0 (Official Build 30036)
 Issue 15846  has been merged into this issue.
Project Member

Comment 46 by bugdroid1@chromium.org, Oct 12 2012

Blockedon: -chromium:24817 -chromium:24877 -chromium:24921 -chromium:25000 -chromium:25249 -chromium:25254 chromium:24817 chromium:24877 chromium:24921 chromium:25000 chromium:25249 chromium:25254
Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.
Project Member

Comment 47 by bugdroid1@chromium.org, Mar 10 2013

Labels: -Mstone-4 M-4
Project Member

Comment 48 by bugdroid1@chromium.org, Mar 13 2013

Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue

Sign in to add a comment