This is a recent regression between 2008040704 and 2008040804. After using Flash (giving it focus), I can no longer cmd-T to open a new tab until after defocusing Flash. STR: 1. Open a YouTube video 2. Click the pause button 3. cmd-T AR: Nothing happens. ER: A new tab opens.

Regression range: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2008-04-07+04&maxdate=2008-04-08+04&cvsroot=%2Fcvsroot My money's on bug 398514 or bug 420527.

Same results with Quicktime (using the trailers at http://www.apple.com/trailers) and with Java. I find that cmd-c, cmd-v and cmd-a still work ... but almost none of the other key combinations. Same results in trunk Camino.

When the focus is in a text-input field in a plugin, you can see that the key commands which fail are interpreted as text. So, for example, pressing cmd-t causes a 't' to appear.

Flash example for comment #3 (from bug 333881): http://www.playercore.com/bugfiles/146162/AddReturnHTML.html

I never would have expected this but after debugging and talking to some people, it turns out this isn't really a bug, at least it isn't Gecko's bug. *groan* This exact thing happens on Windows and Linux. On Mac OS X we used to always execute native menu item commands instead of letting them properly flow through Gecko's event system, and thus things like "cmd-w" always had their menu command executed regardless of what plugins wanted. Somehow this didn't cause problems that we picked up on as significant enough to fix in Gecko 1.8, but it still wasn't *correct*, at least not in terms of Gecko event handling as I understand it. We lucked out getting the behavior we wanted most of the time, especially concerning plugins, but it was totally accidental and now that our code makes sense it doesn't behave the way that we're used to. The only way to fix this (without some cross-platform solution) is to go back to executing native menu item command content all the time again, but that isn't right so I'm pretty hesitant to do that. This is an annoying predicament.

How does this bug relate to bug 78414? Did that bug previously not affect Mac?

(In reply to comment #7) > How does this bug relate to bug 78414? Did that bug previously not affect Mac? Based on comment 6, yes. These are probably the same bug except that it's a "regression" on Mac from the behavior it used to be.

This bug is marked as a blocker but bug 78414 is not. That doesn't seem right to me.

(In reply to comment #9) > This bug is marked as a blocker but bug 78414 is not. That doesn't seem right > to me. This bug is a recent regression, that one isn't. It may be that regression means bug 78414 has worsened, and blocking on not shipping in that worse state is different than blocking on fixing the old behavior (which we've shipped with before).

This might work, I'm not done figuring out if this is the best we can do so I'm holding off on requesting review for a bit. Basically works though.

JFYI: This is working for me by following the STR in comment 0 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008042122 Minefield/3.0pre

A try server build with "fix v1.0". I'm having trouble doing debug testing of my patch because of bug 429771. Plugins only receive keyboard events in optimized builds (?!). https://build.mozilla.org/tryserver-builds/2008-04-21_13:20-josh@mozilla.com-1208809134/josh@mozilla.com-1208809134-firefox-try-mac.dmg

(In reply to comment #13) > A try server build with "fix v1.0". I'm having trouble doing debug testing of > my patch because of bug 429771. Plugins only receive keyboard events in > optimized builds (?!). Josh, this was exactly the issue why I'm not able to see it. I run a debug build yesterday. Now I can reproduce it with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008041804 Minefield/3.0pre ID:2008041804 Perhaps Boris or Benjamin could help to answer this question.

Uh... which question? If it's something about focus or plug-ins, I'm not the right man to be asking....

Comment on attachment 316168 [details] [diff] [review] fix v1.0 I need to add a comment explaining my change, I'll do that on checkin or in any updated patch I post. Basically we have to invoke menu commands when a plugin has focus. Obviously this is wrong and we risk double processing events but we don't have a choice and it worked out alright with a very similar system for the past many years. The good news is that double processing doesn't happen all that often and when it does it tends not to matter much (for example, if cmd-w gets double-processed the window closes so whatever happened *in* the window doesn't really matter, same deal for cmd-q).

Double-processing Cmd+W could cause two tabs to close when you only intended to close one...

No, because we only double process for plugins with my patch. The tab would get closed and the plugin would eat the second event. If the plugin did anything (like insert a "w" into a text field) it wouldn't matter because the tab it was in would be wiped out. If the plugin didn't eat the event we wouldn't have this bug.

I just noticed that this bug is listed as Mac OS X only, when it obviously effects Windows also (at least it has always effected me, go to Youtube, click a video, and ctrl-t and ctrl-w will no longer do anything) is there a corresponding bug for Windows or will this patch also fix the problem on Windows?

(In reply to comment #19) > I just noticed that this bug is listed as Mac OS X only, when it obviously > effects Windows also (at least it has always effected me, go to Youtube, click > a video, and ctrl-t and ctrl-w will no longer do anything) See comment 7 through comment 10.

Um, Josh, we have a problem :-( I can't get your patch to work at all. I tried in this bug's URL and also in all three of the examples listed in bug 357670 comment #25. I get exactly the same symptoms as before the patch. Just to make sure the code is working as expected, I put in extra logging. I found that mIsPluginView _is_ true when the focus is in a plugin, and performKeyEquivalent: _does_ get called (also that the opposite happens when the focus isn't in a plugin). But I still didn't get the right results. I tested on OS X 10.5.2. With an opt build (though it has debug symbols).

I'm not going to attempt to explain the bizarre set of circumstances that led me to believe the previous patch worked :)

Josh, I've started testing (on OS X 10.5.2), and have found one minor problem -- though Cmd-i (Page Info) and Cmd-j (Downloads) work with the focus in a plugin, 'i' and 'j' also appear in the plugin's text-input field (testing with the examples from bug 357670 comment #25). I suspect this isn't a killer, but you should probably try to think of a way to fix it.

That's the expected downside of this bug. We could try going a step further and only send key up like on the 1.8 branch, but I think if we need to take that step we should do it in a point release.

Sounds reasonable. Another problem, potentially more severe: Your patch doesn't solve this problem at all in Camino (or presumably in other embedders). Still testing on 10.5.2. Will move to 10.4.11.

Comment on attachment 317594 [details] [diff] [review] fix v2.0 This looks fine to me. I didn't find any new problems testing on OS X 10.4.11. Too bad about Camino (and the other embedders) ... but this patch doesn't make their behavior any worse. I don't think that problem should block this patch.

Should fix Camino.

fix silly mistake

Comment on attachment 317744 [details] [diff] [review] fix v2.2 This makes your patch also work in Camino (and presumably in other embedders). Interestingly, in Camino you _don't_ see an 'i' in a plugin text-input box after you do Cmd-i while it has the focus. I don't know why ... though of course I'm not objecting :-)

Comment on attachment 317744 [details] [diff] [review] fix v2.2 a=mconnor on behalf of 1.9 drivers

landed on trunk

verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008042904 Minefield/3.0pre. I verified using the STR in Comment 0.

https://litmus.mozilla.org/show_test.cgi?id=5302 added to Litmus.

(In reply to comment #33) > https://litmus.mozilla.org/show_test.cgi?id=5302 added to Litmus. Marcia, shouldn't this be in-litmus+ instead of in-testsuite+?

This fix resulted in Komodo getting two events for a key command when the plugin was focused. If the menu system calls doCommand and sends a keyevent for that command, then we get it twice. Our keybindings system handles calling doCommand for key commands and is unaware (I don't know how to make it aware) that the underlying menu system has already called doCommand.