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)

defect
Not set
normal

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)

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?)
Blocks: 55416
->bryner? also see this on mac and linux.
Assignee: hyatt → bryner
Keywords: nsbeta1
OS: Windows 98 → All
Hardware: PC → All
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...
-> jag
Assignee: bryner → jaggernaut
nsbeta1- per ADT triage team, ->1.2. Is this even a defect? cc Marlon for UE input
Keywords: nsbeta1nsbeta1-
Target Milestone: --- → mozilla1.2
*** Bug 132261 has been marked as a duplicate of this bug. ***
*** Bug 134972 has been marked as a duplicate of this bug. ***
*** Bug 140706 has been marked as a duplicate of this bug. ***
could this be a variation of the same thing as bug 102831?
It seems to me so.
Depends on: 102831
nominating...
Keywords: nsbeta1-access, nsbeta1
Nav triage team: nsbeta1+/adt3
Keywords: nsbeta1nsbeta1+
Whiteboard: [adt3]
Attached patch patch (obsolete) (deleted) — Splinter Review
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.
Taking.
Assignee: jaggernaut → jruderman
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.
Hmm, the patch above lets you switch tabs while a menu is dropped down. That's not good.
Ah, possibly I misused a capturing instead of a bubbling event listener...
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?
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.
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.
Attached patch patch based on neil's patch (obsolete) (deleted) — Splinter Review
Attachment #94107 - Attachment is obsolete: true
r=jesse on neil's part of the patch.
Attachment #95436 - Flags: review+
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>
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.
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.
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).
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.
Attached patch Patch to address jag's comment (obsolete) (deleted) — Splinter Review
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
Keywords: patch, polish, review, ui
Does this also fix CTRL-F4 shortcut (to close the current tab)?
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+
Attachment #96180 - Attachment is obsolete: true
r=jesse
Assignee: jruderman → neil
Blocks: Ctrl-Tab
No longer depends on: 102831
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+
Fix was checked in by bz.
Really resolving fixed...
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
looks good! vrfy'd fixed with 2002.09.16.08 comm trunk builds.
Status: RESOLVED → VERIFIED
Product: Core → SeaMonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: