1128771 - Investigate if plugins should handle key events after web page and browser chrome

Status: RESOLVED WONTFIX

(Core Graveyard :: Plug-ins, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

...this way web page or browser chrome could call preventDefault() and make sure
plugins can't override the behavior web page or browser chrome wants to have.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Problem is things like arrow keys. Should those scroll the page or move a flash video back/forward?

Comment 2

[Doug Turner (:dougt)]

I think basically if the CMD key is down in the keyevent, we shouldn't forward the event to the plugin.  Doing so breaks the browser as-an-application.  For example, CMD-Q (quit) doesn't work if focus is owned by the plugin.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

It is not clear to me why cmd is so special?

On linux OS-button (the Windows button) is important for certain things, and ctrl for others etc.

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away: -9/28)]

On Gnome or Unity, Win+foo is consumed before us if it's a system-wide shortcut key.

IIRC, similarly, some system-wide shortcut keys are not sent to us on Mac too.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away: -9/28)]

And also, our gtk widget ignores Ctrl+Alt+Tab (i.e., not cause DOM events):
http://mxr.mozilla.org/mozilla-central/source/widget/gtk/nsWindow.cpp#3006

Comment 6

[Doug Turner (:dougt)]

Masayuki, when you say 'us' in comment #4, who is 'us'?

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away: -9/28)]

(In reply to Doug Turner (:dougt) from comment #6)
> Masayuki, when you say 'us' in comment #4, who is 'us'?

I meant Gecko (and native applications on those platforms).

Comment 8

[Doug Turner (:dougt)]

On the mac, gecko does get keyevents with CMD modifiers.  

https://github.com/dougt/gecko-dev/commit/43f932678d258488714c5f228c60ac7cd1c3f9ff

This is the horrible hack that I do to prevent keyevents with CMD going into flash.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away: -9/28)]

(In reply to Doug Turner (:dougt) from comment #8)
> On the mac, gecko does get keyevents with CMD modifiers.  
> 
> https://github.com/dougt/gecko-dev/commit/
> 43f932678d258488714c5f228c60ac7cd1c3f9ff
> 
> This is the horrible hack that I do to prevent keyevents with CMD going into
> flash.

Then, the users cannot use Cmd+C, Cmd+V, etc, right? It's bad idea to cut the key events which are not handled by our chrome.

I guess that it's the best approach we to think that sending key events is a default action. However, perhaps, we need to hack XBL prototype handler. It may eat some shortcut keys which is necessary to be dispatched to plugin, e.g., "copy" (Accel + C).

Comment 10

[Doug Turner (:dougt)]

I would be happy to just stop the hijacking of the top level page navigation by plugins.  Thoughts?

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away: -9/28)]

(In reply to Doug Turner (:dougt) from comment #10)
> I would be happy to just stop the hijacking of the top level page navigation
> by plugins.  Thoughts?

But plugins may need to handle some shortcut keys like Cmd+[XCVZ]. So, I don't think we can fix this simply.

Comment 12

[Doug Turner (:dougt)]

What if we did something as simple as preventing the following 4 events from being passed into flash on the MAC:

modifier: cmd
key: left-arrow

modifier: cmd
key: right-arrow

modifier: cmd
key: [

modifier: cmd
key: ]

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

For example in Finnish Macbook keyboard there are no specific keys for [ or ].
(I think one needs to use Alt+8 and Alt+9).

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away: -9/28)]

Our chrome key event handlers ignore defaultPrevented of some key events. In bug 1008772, I make following shortcut keys so:

Ctrl+Tab
Ctrl+Shift+Tab
Ctrl+PageDown
Ctrl+PageUp
Ctrl+Shift+PageDown
Ctrl+Shift+PageUp
Ctrl+ArrowUp (except Mac)
Ctrl+ArrowDown (except Mac)
Ctrl+ArrowLeft (except Mac)
Ctrl+ArrowRight (except Mac)
Ctrl+Home (except Mac)
Ctrl+End (except Mac)
Command+ArrowUp (Mac)
Command+ArrowDown (Mac)
Command+ArrowLeft (Mac)
Command+ArrowRight (Mac)
Command+Home (Mac)
Command+End (Mac)
Command+Option+ArrowLeft (Mac)
Command+Option+ArrowRight (Mac)

Basically, Tab/Window navigation shouldn't be blocked by anybody.

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away: -9/28)]

If shortcut key is defined by XUL <key> element, it's handled by keypress event but keypress event may not be fired (if it's default is prevented). So, I think that <key> elements should handle keydown events instead of keydown event. In such case, keypress events which is defined as shortcut key will be never fired. I guess that this causes some incompatibility with some our internal code but not also with most web contents because other browsers don't dispatch keypress events if the key event doesn't cause text input.

Additionally, XUL <key> should have |ignoreDefaultPrevented| attribute for ignoring the defaultPrevented value?

Comment 16

[Doug Turner (:dougt)]

what are the next steps here?

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away: -9/28)]

Not sure. We should decide what we should do from following options:

1. Whether we make sending events to plugin one of default action or keep current behavior (this bug)
2. Whether we make XUL <key> handling before sending events to plugins (I guess that we shouldn't do this, it may consume some key events which are necessary on plugins).
3. Whether we improve XUL <key> handling as I mentioned above (I believe that we need this anyway, let's separate to another bug).

Anyway, I don't have much time for now. I'm working on rewriting all mochitests which are using synthesizeKey() or something in EventUtils.js with new API. I need a couple of weeks or more for that. After that, I can work on the #3 if it won't cause unexpected regressions.

Comment 18

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away: -9/28)]

I'm not sure if this is possible to fix on Linux, I'll work on only Windows and Mac for now.

Comment 19

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away: -9/28)]

FYI: when bug 1154183 is fixed, this bug is fixed in windowless mode on Windows.

Comment 20

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away: -9/28)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=380c0bc03f81

Comment 21

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away: -9/28)]

I confirmed that bug 1154183 completely fixes this bug on Mac. I guess that this should (needs to) treat only the bug on Windows.

Comment 22

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away: -9/28)]

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #21)
> I confirmed that bug 1154183 completely fixes this bug on Mac.

I changed the approach to fix bug 1154183. Then, Mac becomes some specific patches needed.

E.g., Cmd+T is pressed on Flash Player, the keyboard event is NOT sent to plugin, but not executed. I'm still investigating the reason, but I honestly have no idea. Loos like somebody stops propagation of the system event group or prevents default but neither Event::Stop*Propagation() nor Event::PreventDefault() is called...

Comment 23

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away: -9/28)]

Okay, I should make this bug a meta bug. I'll file new bugs for each platform.

Comment 24

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away: -9/28)]

I think that we shouldn't work on plugins anymore.