595762 - CTRL + Space opening new "Group Your Tabs" should be preventable

Status: RESOLVED INVALID

(Firefox Graveyard :: Panorama, defect, P4)

Description

[Jaroslav Benc]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b5) Gecko/20100101 Firefox/4.0b5
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b5) Gecko/20100101 Firefox/4.0b5

CTRL + Space opening new "Group Your Tabs" should be preventable so we can have our own application key handler. 



Reproducible: Always

Steps to Reproduce:
1. add a keydown handler on CTRL+SPACE and call preventDefault when handled
2. press CTRL + SPACE
Actual Results:  
"Group Your Tabs" view is shown

Expected Results:  
"Group Your Tabs" shouldn't be shown

This is related to Bug 534121 and Bug 435164 as well as CTRL+Space opens context menu on Mac. 

Whatever it does, it should be preventable.

Comment 1

[Boris Zbarsky [:bzbarsky]]

This seems like it'll cause web compat issues...

Does using preventDefault on the keypress handler work?

Comment 2

[Johnathan Nightingale [:johnath]]

Blocking on this speculatively, need this to be confirmed (and Boris' question answered).

Jaroslav - can we get a reduced html test case?

Comment 3

[Jaroslav Benc]

> Does using preventDefault on the keypress handler work?

No, that's the problem, event.cancelable is true but event.preventDefault() doesn't prevent the TabCandy from being opened. Try attached example.

Comment 4

[Jaroslav Benc]

Attachment: press CTRL + space - this should prevent opening the TabCandy

Comment 5

[Jaroslav Benc]

There is another bug I found when creating this example but I've filed a new bug for it - Bug 598809

Comment 6

[Boris Zbarsky [:bzbarsky]]

> Try attached example.

The attached example is preventing default action for keydown.  You said that in comment 0, and I believe you that it doesn't work.  My question was about _keypress_.

Comment 7

[Jaroslav Benc]

Well, keypress is not even fired when CTRL + space is pressed. TabCandy seems to be activated on keydown and prevents all other key events being fired when TabCandy is active (which make sense).

Comment 8

[Jaroslav Benc]

Attachment: Try this example in FF3 and then FF4beta6 (or other beta with the TabCandy)

Comment 9

[Boris Zbarsky [:bzbarsky]]

Ah, so tabcandy does in fact use a keypress listener (see _setBrowserKeyHandlers in browser-tabview.js).  However:

1)  It uses a capturing listener in the non-system group.  Why, exactly?  Seems
    like this is what the system event group is for.
2)  It doesn't pay attention to whether the default action has been prevented
    (which it _may_ be even at the beginning of the capturing phase of the normal
    event group by preventing default on keydown; some of that stuff is weird)

Comment 10

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

(In reply to comment #9)
> 1)  It uses a capturing listener in the non-system group.  Why, exactly?  Seems
>     like this is what the system event group is for.
Capturing listener in default event group for things which chrome wants to
handle before content. Sounds like TabCandy shouldn't use that,
but capturing listener in system event group.
(or if it is ok that .stopPropagation() prevents TabCandy, then bubble phase
in default event group is also ok)

> 2)  It doesn't pay attention to whether the default action has been prevented
>     (which it _may_ be even at the beginning of the capturing phase of the
> normal
>     event group by preventing default on keydown; some of that stuff is weird)
Yeah, TabCandy listener should check event.getPreventDefault()

Comment 11

[Dão Gottwald [:dao]]

Bug 592183 should fix this by using a standard <key> element and getting rid of the custom event listener.

Comment 12

[:Gavin Sharp [email: [e-mail]]]

Indeed, I think this is now resolved by bug 592183, per comment 11.