1068282 - Keypress for Ctrl+Any key not fired

Status: RESOLVED INVALID

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[dtrunk90]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:32.0) Gecko/20100101 Firefox/32.0
Build ID: 20140830210550

Steps to reproduce:

Developing an addon with the following JavaScript snippet:
window.addEventListener('load', function ()  {
    document.addEventListener('keypress', function (e) {
        console.error(e);
    }, true);
}, false);

And look into the browser console whats the output when pressing Ctrl+Tab.


Actual results:

No output as keypress is not fired. I need to catch tab switching in my firefox addon. It worked some versions ago. But since any version (don't know till which version it worked) it stopped working.


Expected results:

I should get a console output to handle the event.

Comment 1

[Loic]

Could you attach a simple self-contained testcase, please.

Comment 2

[dtrunk90]

Attachment: 1068282.xpi

Comment 3

[dtrunk90]

Hope that's enough as a testcase

Comment 4

[Loic]

(In reply to dtrunk90 from comment #3)
> Hope that's enough as a testcase

I needed to modify install.rdf because you set minVersion and maxVersion to 32, so it blocks testing with older and newer versions of Firefox (it's better to allow from FF4 to FF35).

Secondly, I tested your add-on with the old FF17, I'm not able to see some console messages when pressing Ctrl+T, Ctrl+V or Ctrl+Tab in Firefox.

So what are the steps to reproduce once the add-on is installed? Which message should I see in the console? Did you test your add-on?

Comment 5

[dtrunk90]

(In reply to Loic from comment #4)
> (In reply to dtrunk90 from comment #3)
> > Hope that's enough as a testcase
> 
> I needed to modify install.rdf because you set minVersion and maxVersion to
> 32, so it blocks testing with older and newer versions of Firefox (it's
> better to allow from FF4 to FF35).
> 
> Secondly, I tested your add-on with the old FF17, I'm not able to see some
> console messages when pressing Ctrl+T, Ctrl+V or Ctrl+Tab in Firefox.
> 
> So what are the steps to reproduce once the add-on is installed? Which
> message should I see in the console? Did you test your add-on?

You're right. My fault.

I don't know if FF17 is able to print console.error without importing resource://gre/modules/devtools/Console.jsm: Components.utils.import('resource://gre/modules/devtools/Console.jsm');
My addon worked with older versions and triggered keypress for Ctrl+Tab which isn't working anymore.

I don't know exactly till which FF version my addon stopped working, but you should see the event information in your console when pressing only Tab but not when pressing Ctrl+Tab (which is the bug)

Comment 6

[dtrunk90]

Well. I noticed it works on my virtual Windows 7 machine with FF30.
I'm getting the following output on my browser console there: keypress Control { target: <richlistbox#categories>, key: "Tab", charCode: 0, keyCode: 9 } browser.js:3

I'll do an update to FF32 now to see if it still works there.

Comment 7

[Loic]

I'm able to reproduce it, I needed to open the Browser Console, not the Web Console. :D

Comment 8

[dtrunk90]

Lol. That's what I said: Browser console ;-)
I'm currently downloading FF31 (including all beta releases) to locate the first "buggy" version.

Comment 9

[Loic]

Regression range:
good=2014-06-05
bad=2014-06-06
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=51b428be6213&tochange=c8288d0c7a15

Suspected bug:
Masayuki Nakano — Bug 1008772 part.1 tabs should handle non-printable keys with keydown event handlers in the system group rather than keypress event handlers in the normal group r=smaug+enndeakin

mayasuki, do you know if it's a valid regression?

Comment 10

[Loic]

Attachment: 1068282.xpi (v1.0.1 with correct install.rdf)

Comment 11

[dtrunk90]

My test results: It worked till 31.1.0esr and then I've installed 32.0b1 and it stopped working.
Your mentioned fix could be the reason.

Comment 12

[dtrunk90]

So I guess I need to change my code to use keydown then?

Comment 13

[Loic]

My skills of analysis stop here. :)
Maybe wait for the opinion of a Moz dev.

Comment 14

[dtrunk90]

My intention is to prevent FF from switching tabs as my addon does it (it excludes pinned tabs from switching with Ctrl+Tab). And I guess FF uses keypress event for it. Now e.stopPropagation(); and e.preventDefault(); doesn't do what I want and it switches two tabs now.

Comment 15

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

I'm working on conforming to D3E keyboard event behavior. For that,

* a call of preventDefault() of keydown event prevents following keypress event. This was already done.
* non-printable keys shouldn't cause keypress event. I'm still working on this.

Therefore, now, default action handler which should NOT be blocked by web contents like tab switching needs to listen keydown event rather than keypress event.

Therefore, Ctrl+Tab and some other tab navigation shortcut keys are handled at keydown event and consumes the event (calling preventDefault() for preventing other actions). Therefore, keypress event for them won't be fired.

If your addon attempts to override the default behavior of tab navigation, you should handle keydown event *before* our event handlers. And note that if you don't use system event group, your addon's behavior is blocked by a call of event.stopPropagation() of web apps. This is perhaps unexpected behavior for you. So, ideal event handler for implementing default action should be:

1. listen keydown event for non-printable keys (except Enter), listen keypress event for printable keys if it doesn't ignore Event.defaultPrevented value.
2. listen in system event group.
3. ignore events whose defaultPrevented is true because the event was already handled by other event handlers and they don't want defaultaction to occur. (For security reason, some default action handlers like tab navigation ignore it, though)

Comment 16

[dtrunk90]

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) (away: 9/20 - 9/23, JST) from comment #15)
> I'm working on conforming to D3E keyboard event behavior. For that,
> 
> * a call of preventDefault() of keydown event prevents following keypress
> event. This was already done.
> * non-printable keys shouldn't cause keypress event. I'm still working on
> this.
> 
> Therefore, now, default action handler which should NOT be blocked by web
> contents like tab switching needs to listen keydown event rather than
> keypress event.
> 
> Therefore, Ctrl+Tab and some other tab navigation shortcut keys are handled
> at keydown event and consumes the event (calling preventDefault() for
> preventing other actions). Therefore, keypress event for them won't be fired.
> 
> If your addon attempts to override the default behavior of tab navigation,
> you should handle keydown event *before* our event handlers. And note that
> if you don't use system event group, your addon's behavior is blocked by a
> call of event.stopPropagation() of web apps. This is perhaps unexpected
> behavior for you. So, ideal event handler for implementing default action
> should be:
> 
> 1. listen keydown event for non-printable keys (except Enter), listen
> keypress event for printable keys if it doesn't ignore
> Event.defaultPrevented value.
> 2. listen in system event group.
> 3. ignore events whose defaultPrevented is true because the event was
> already handled by other event handlers and they don't want defaultaction to
> occur. (For security reason, some default action handlers like tab
> navigation ignore it, though)

So why my addon switches two tabs now as it looks like preventDefault gets ignored by the followed keypress event? (I'll attach my addon)

Comment 17

[dtrunk90]

Attachment: apptabsplus-1.6.2.xpi

Comment 18

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

> So why my addon switches two tabs now as it looks like preventDefault gets ignored by the
> followed keypress event? (I'll attach my addon)

Yes, preventDefault() of keydown is always ignored when tabbox handles keydown events as switching tabs.

Looks like your addon listens keydown events in default event group. Then, it's impossible to prevent the default actions. If you want to override the default action, you should use system event group for listening keydown event and call stopPropagation() in it. See around here:
https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIEventListenerService

Comment 19

[dtrunk90]

Thanks for your information.