Closed
Bug 114170
Opened 23 years ago
Closed 22 years ago
ctrl+pgup/pgdn (to switch tabs) doesn't work with focus in url bar
Categories
(SeaMonkey :: Tabbed Browser, defect)
SeaMonkey
Tabbed Browser
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla1.2alpha
People
(Reporter: jruderman, Assigned: neil)
References
(Blocks 1 open bug)
Details
(Keywords: access, polish, Whiteboard: [adt3])
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
asa
:
approval+
|
Details | Diff | Splinter Review |
Steps to reproduce:
1. Open an extra tab (Ctrl+Tab). Focus should now be in the location bar of
the new tab.
2. Press Ctrl+pgdn or Ctrl+pgup.
Result: autocomplete dropdown appears
Expected: switch tabs (but don't change focus?)
Comment 1•23 years ago
|
||
->bryner?
also see this on mac and linux.
Comment 2•23 years ago
|
||
Probably i have to open new bug for this, but may be that isn't useful for now.
I really think that CTRL-left and CTRL-right are better shortcuts for swithing
between tab.
There are already ALT-left/right for going back and forward...
Comment 4•23 years ago
|
||
nsbeta1- per ADT triage team, ->1.2. Is this even a defect? cc Marlon for UE input
Comment 5•23 years ago
|
||
*** Bug 132261 has been marked as a duplicate of this bug. ***
Comment 6•23 years ago
|
||
*** Bug 134972 has been marked as a duplicate of this bug. ***
Comment 7•23 years ago
|
||
*** Bug 140706 has been marked as a duplicate of this bug. ***
could this be a variation of the same thing as bug 102831?
Comment 9•22 years ago
|
||
It seems to me so.
Comment 10•22 years ago
|
||
nominating...
Comment 11•22 years ago
|
||
Nav triage team: nsbeta1+/adt3
Reporter | ||
Comment 12•22 years ago
|
||
This fixes:
- Can't ctrl+pgdn in browser address bar (this bug).
- Can't ctrl+pgdn in page info when Help button has focus (bug 130644).
- Can't ctrl+pgdn just after opening the mail folder properties dialog.
Comment 14•22 years ago
|
||
Comment on attachment 94107 [details] [diff] [review]
patch
this can work, but does that mean we should have plan for change all these kind
of handler
<handler event="keypress" keycode="vk_tab" modifiers="control,shift">
to xbl event listerner.
Theoretically, what will happen if there are 2 tabbox in one xul document.
Reporter | ||
Comment 15•22 years ago
|
||
Hmm, the patch above lets you switch tabs while a menu is dropped down. That's
not good.
Assignee | ||
Comment 16•22 years ago
|
||
Ah, possibly I misused a capturing instead of a bubbling event listener...
Reporter | ||
Comment 17•22 years ago
|
||
If we use bubbling instead of capturing (to fix the menu problem), can we still
block the default behavior of Ctrl+Tab, which is to switch panes/frames?
Assignee | ||
Comment 18•22 years ago
|
||
Yes, and better still it fixes the problem of 2 tabboxes - the tabbrowser gets
ctrl+tab directed when the urlbar has focus, but the DOM inspector sidebar gets
ctrl+tab when it has focus. I could create a new patch but I'm not sure whether
to remove the existing event listeners and rely on all the events bubbling up to
the document.
Reporter | ||
Comment 19•22 years ago
|
||
I think neil's patch (bug 136915 comment 76) is better than the one I posted
above, so I'm going to work from his patch.
Neil's suggestion to use bubble rather than capture does work. It requires a
small change to the address bar because the address bar unintentionally grabs
ctrl+pgdn.
Reporter | ||
Comment 20•22 years ago
|
||
Attachment #94107 -
Attachment is obsolete: true
Reporter | ||
Comment 21•22 years ago
|
||
r=jesse on neil's part of the patch.
Assignee | ||
Updated•22 years ago
|
Attachment #95436 -
Flags: review+
Assignee | ||
Comment 22•22 years ago
|
||
Comment on attachment 95436 [details] [diff] [review]
patch based on neil's patch
> I could create a new patch but I'm not sure whether to remove the existing event listeners and rely on all the events bubbling up to the document.
Well I would have created almost exactly this patch if you had decided the
issue for me. r=neil@parkwaycc.co.uk on all your changes except the line of
four spaces after </implementation>
Comment 23•22 years ago
|
||
This patch looks okay, though there will be a problem (multiple tabboxes
switching when you ctrl-tab) when you have something like:
<window>
<tabbox/>
<tabbox/>
</window>
where two (or more) tabboxes are in the same document. I can't think of any
place that we have such code, but I'm not sure I want to disallow this either.
Perhaps we should allow the XUL author to specify on which node the tabbox
should register the listener, defaulting to the tabbox itself. That would allow
us to do the best thing in each case I think.
Comment 24•22 years ago
|
||
i agree with jag.
I suggest that if there are more than one tabbox, they should be some kind of
sort. And when we reach the last tab of a tabbox, ctrl-tab should make mozilla
to switch to the next tabbox's first tab.
Assignee | ||
Comment 25•22 years ago
|
||
With Jesse's changes only one set of tabs will grab ctrl+tab because the events
will be stopped from reaching the second set of tabs. Unfortunately this also
applies when focus is inside the second set of tabs, which is why I was
considering leaving the <handler>s in (but additionally stopping event propogation).
Comment 26•22 years ago
|
||
Neil, you're right. There's still a problem though: which tabbox gets the
ctrl+tab? With luck it's "the first one in the document", but I think it's
pretty much undefined.
Assignee | ||
Comment 27•22 years ago
|
||
This creates a property called eventNode which can be set in JS to whichever
node the tabbox needs to listen for events. It defaults to the document,
window, parent or element depending on the initial value of the attribute
eventNode (which should be one of those values).
jag, if that's too flexible, completely remove the property and add
readonly="true" to the field.
Or, if that's not flexible enough, I could make it so that the tabbox walks up
the tree looking for a node whose localName matches the attribute.
Attachment #95436 -
Attachment is obsolete: true
Comment 28•22 years ago
|
||
Does this also fix CTRL-F4 shortcut (to close the current tab)?
Comment 29•22 years ago
|
||
Comment on attachment 96180 [details] [diff] [review]
Patch to address jag's comment
This looks good to me. We can make it more flexible if need be (e.g.
eventnode="id=foopy"). Btw, attribute names are typically all lowercase in our
code, can you change yours (I know some places break with that style, tsk tsk
to them)?
Also, could you indent the body of the switch with two spaces?
sr=jag
Attachment #96180 -
Flags: superreview+
Assignee | ||
Comment 30•22 years ago
|
||
Attachment #96180 -
Attachment is obsolete: true
Reporter | ||
Comment 31•22 years ago
|
||
r=jesse
Comment 32•22 years ago
|
||
Comment on attachment 96449 [details] [diff] [review]
Fixed switch and eventnode attribute
a=asa (on behalf of drivers) for checkin to 1.2a
Attachment #96449 -
Flags: approval+
Assignee | ||
Comment 33•22 years ago
|
||
Fix was checked in by bz.
Assignee | ||
Comment 34•22 years ago
|
||
Really resolving fixed...
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Comment 35•22 years ago
|
||
looks good! vrfy'd fixed with 2002.09.16.08 comm trunk builds.
Status: RESOLVED → VERIFIED
Updated•16 years ago
|
Product: Core → SeaMonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•