1100862 - Compatibility issue: The Win+V shortcut used by PlainText doesn't work in Firefox

Status: RESOLVED FIXED

(Core :: XBL, defect)

Description

[Mikael Falkvidd]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:33.0) Gecko/20100101 Firefox/33.0
Build ID: 20141113143407

Steps to reproduce:

1. Start Plain Text (http://plaintext.codeplex.com/) v 1.5
2. Start Firefox
3. Copy any text to the clipboard (ctrl+C)
4. In any text area (address bar, search box, html text/textarea field, etc), press Win+V




Actual results:

Nothing at all


Expected results:

The text on the clipboard should be pasted without HTML formatting.
Pasting using Win+V works in all other applications that I know of, including Windows WordPad, Chrome, IE 11.

Related issue: https://bugzilla.mozilla.org/show_bug.cgi?id=819899#c25

Comment 1

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

PlainText emulates Ctrl + V when user presses Win + V (perhaps, it modifies the clipboard text before that). However, the generated 'V' key event has both Ctrl and Win (OS) modifiers. Therefore, perhaps, nsXBLPrototypeHandler fails to do the expected command.

As far as I tested on Windows, native applications ignore Win key status at handling shortcut keys. I think that we need to ignore OS modifier state in nsXBLPrototypeHandler on Windows even if "os" isn't specified with "any".
https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/key#a-modifiers

How do you think, Enn?

Comment 2

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

On gedit, Ctrl + Super + X doesn't work as Ctrl + X. So, I think that this is Windows only issue.

Comment 3

[Olli Pettay [:smaug][[e-mail]] (vacation-ish -> Aug 1)]

So does PlainText create its own ctrl+V event and expect that to cause paste?
(If so, wouldn't it be just relying on a blink bug where they allow web pages to do all sorts of things with untrusted events?)

Comment 4

[Boris Zbarsky [:bzbarsky]]

My understanding of comment 1 was that PlainText generates Windows key events, not DOM key events.

Comment 5

[Olli Pettay [:smaug][[e-mail]] (vacation-ish -> Aug 1)]

Ah yeah, probably.

Comment 6

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

Yes. Probably it uses SendInput() API and that causes WM_KEYDOWN, WM_CHAR and WM_KEYUP. We will dispatch DOM key events for them. So, they *expect* Ctrl+v causes "Paste". (Although, it should use WM_PASTE, instead)

Comment 7

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

Okay, this test build works for me.
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-825eedbcee71/try-win32/

However, I need to modify or add test for this before requesting review.
https://hg.mozilla.org/try/rev/ba2e50eff625

Comment 8

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

FYI: On Windows 8.1, Win+V doesn't cause WM_KEYDOWN and WM_CHAR. Therefore, PlainText doesn't work on it. You need to test it on Windows 7 or earlier.

Comment 9

[Mikael Falkvidd]

It does not work for me when testing with http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-825eedbcee71/try-win32/firefox-36.0a1.en-US.win32.zip

Same problem as before, nothing happens when pressing Win+V . Using Windows 7 pro version 6.1 build 7601, service pack 1

Comment 10

[Mikael Falkvidd]

Sorry, my bad. The old FF was still running so starting the new build just gave me a new window of the old version.

Pasting with PlainText using Win+V now works in address bar, search field and html text fields.

Great work, thanks a lot!

Comment 11

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

Attachment: Shortcut key handlers should ignore Windows-Logo key state if no shortcut keys match a key event and the key is pressed because native applications do so

Okay, let's take this approach.

Native applications on Windows don't honor Windows key state at least for Ctrl+foo or Alt+bar. So, when Win key (OS key) is pressed, we should ignore the Win key state if there is no shortcut key handler which exactly matches Win+foo+bar.

I.e., on Windows, modifiers attribute of XUL key element always include "os any" implicitly. However, if there is a shortcut key handler which exactly matches a key event including OS key state, we should prefer it.

Comment 12

[Neil Deakin]

Comment on attachment 8526823 [details] [diff] [review]
Shortcut key handlers should ignore Windows-Logo key state if no shortcut keys match a key event and the key is pressed because native applications do so

>+#ifdef XP_WIN
>+  // Windows native applications ignore Windows-Logo key state at checking
>+  // shortcut keys even if the key is pressed.  Therefore, if there is no

'key state at checking' -> 'key state when checking'


>-  if (mKeyMask & cOSMask) {
>+  if ((mKeyMask & cOSMask) &&
>+      (!aIgnoreModifierState.mOSIfActive || !inputEvent->IsOS())) {
>     if (inputEvent->IsOS() != ((mKeyMask & cOS) != 0)) {
>       return false;

Why is the !inputEvent->IsOS() necessary here? The caller already checks if the key is pressed.

You should also just change the name mOSIfActive to mOS to mirror the mShift property.

Comment 13

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

Thanks!

https://hg.mozilla.org/integration/mozilla-inbound/rev/84bb72174c24

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/84bb72174c24