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

RESOLVED INVALID

Status

Firefox Graveyard
Panorama
P4
normal
RESOLVED INVALID
8 years ago
2 years ago

People

(Reporter: Jaroslav Benc, Unassigned)

Tracking

Details

Attachments

(2 attachments)

(Reporter)

Description

8 years ago
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.
Component: Keyboard: Navigation → TabCandy
Product: Core → Firefox
QA Contact: keyboard.navigation → tabcandy

Updated

8 years ago
Depends on: 435164
OS: Windows 7 → All
Priority: -- → P4
Hardware: x86_64 → All
This seems like it'll cause web compat issues...

Does using preventDefault on the keypress handler work?
blocking2.0: --- → ?
Blocking on this speculatively, need this to be confirmed (and Boris' question answered).

Jaroslav - can we get a reduced html test case?
blocking2.0: ? → betaN+
(Reporter)

Comment 3

8 years ago
> 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.
(Reporter)

Comment 4

8 years ago
Created attachment 477782 [details]
press CTRL + space - this should prevent opening the TabCandy
(Reporter)

Comment 5

8 years ago
There is another bug I found when creating this example but I've filed a new bug for it - Bug 598809
> 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_.
(Reporter)

Comment 7

8 years ago
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).
(Reporter)

Comment 8

8 years ago
Created attachment 477803 [details]
Try this example in FF3 and then FF4beta6 (or other beta with the TabCandy)
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)
Status: UNCONFIRMED → NEW
Ever confirmed: true
(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()
Bug 592183 should fix this by using a standard <key> element and getting rid of the custom event listener.
Depends on: 592183
No longer depends on: 435164
Blocks: 598154
Indeed, I think this is now resolved by bug 592183, per comment 11.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → INVALID
(Assignee)

Updated

2 years ago
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.