332271 - One cause of wrong tab closure identified in tab closure handling (Ctrl+F4 bubbles although being handled)

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Fox2]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1

I don't know if this is the cause of all the time I've had a desired tab close when I wasn't expecting it, but it's a confirmed cause of many of them and needs fixing.

Suppose you open a new tab. A blank tab appears and the focus is taken to the address bar for entry of the desired address. Whilst entering the address you decide actually you don't need that page after all. So you close it, with the normal key combination for "close tab".

Then not only your new blank tab is closed, but also the NEXT tab is closed as well. If you only had one tab open prior to the above, FF itself shuts down too.

It is obvious that logically what has happened is that the edge case where the current tab is newly displayed blank but no web page has yet loaded into it, is not correctly processed in the "close tab" function.

Reproducible: Always

Steps to Reproduce:
See above.
Actual Results:  
See above. 
A second tab is closed that was not directed to be closed. Data is lost.

Expected Results:  
Existing valid tabs should not close when the current tab is closed.

I have also had many cases (harder to reproduce) where a current tab being worked on has closed and pathologically, a second previous tab has closed too. Most of these were not new tabs. So probably there is a second bug in this function with similar effect. Please when rechecking this function look for other cases by which a 2nd tab might accidentally be closed when the close-tab function is used.

Comment 1

[Phil Ringnalda (:philor)]

Works for me: ctrl+w closes the newly opened tab, not any other tab. Try it in Firefox's safe mode ( http://kb.mozillazine.org/Safe_mode ) to be sure you don't have a buggy extension causing the problem.

Comment 2

[Fox2]

Testing that now. I'm using userChrome to put the tabbar on the left instead of vertigo, will update if this seems to fix it. It might be a fault in vertigo.

Naturally, doing so has instead revealed a likely further bug in drag & drop drawing and function of such vertical boxes via userChrome (#332298)........

Comment 3

[Fox2]

Intersting result. I have a confirmed bug in keyconfig where there shouldn't by rights be a bug (!)

Can someone test if this makes it reproducable:

1. Open up FF with no tabs
2. Install and enable extension keyconfig
3. Allocate ctrl-F4 to "CLOSE TAB" (FILE)
4. Once that's done, open one tab to this page (for example)
5. Open up a 2nd (new) blank tab. The focus goes to the address bar.
6. Start typing an address in the address bar but don't press ENTER.....

7a. With the focus still in the address box, now close the current tab with CTRL-F4. Both the new tab AND the previous one close and FF itself closes down.

7b. Exact same process but this time while the focus is in the address box, now close the current tab avoiding keyboard shortcuts, using FILE --> CLOSE TAB. It works correctly.

On my system this is 100% reproducable. It raises the question though, why would the same function accessed via a menu or via an equivalent keyboard shortcut, work correctly in one case and not in the other? This suggests there may be a problem with the keyboard to function mapping going on somewhere. Core or extension isn't clear.

Verification of the issue please?

Comment 4

[Fox2]

... and I think I know why.

FF *says* on its menu, and in keyconfig, that CTRL-W is the shortcut for "close tab". But it seems that CTRL-F4 is a "hidden" second shortcut. Not knowing this, when a user uses keyconfig to allocate "close tab" from CTRL-W to CTRL-F4, the function probably gets hooked twice, once from its original CTRL-F4 undocumented keyboard shortcut, once from the keyconfig'ed CTRL-W allocated to CTRL-F4 as well.

If so, is this more a problem in how keyboard shortcuts are allocated, that the same shortcut can be allocated to two functions, once in the background and once visible to the user?

can someone verify if the above seems correct?

Comment 5

[Phil Ringnalda (:philor)]

Yep, that would be an extension bug: if you're going to let people remap keys which already have a function, it's probably a good idea to be sure you stop the default action from happening (or, maybe not: maybe the extension author thinks it's more useful to let you map one key combo to more than one thing).

Comment 6

[Dorando]

(In reply to comment #5)
> Yep, that would be an extension bug: if you're going to let people remap keys
> which already have a function, it's probably a good idea to be sure you stop
> the default action from happening (or, maybe not: maybe the extension author
> thinks it's more useful to let you map one key combo to more than one thing).
> 
keyconfig only adds/modifies <key>s and for these the system already seems to "stop the default action from happening" ( http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/xbl/src/nsXBLPrototypeHandler.cpp&rev=1.101#393 ).

In this case the problems seems to be that the <tabbrowser>._keyEventHandler (which receives the event before it reaches the <key>) handles this shortcut but doesn't call preventDefault...

Comment 7

[Fox2]

Seems from comments 5 + 7 theres a question if this is a bug in the core, within the _keyEventHandler handler. 

At least, thats what it looks like it's saying..... can someone check? Marking it as reopened until confirmed what the issue is and where it should be fixed.

Comment 8

[Simon Bünzli]

Attachment: prevent the [Ctrl]+[F4] event from bubbling

Although I'm not sure whether it wouldm't be better to handle Ctrl+F4 as a <key> in browser.xul, this will fix the issue at hand (preventing Ctrl+F4 to be assigned to any other action).

Comment 9

[Fox2]

Might it be worth looking at the key handling, to just recheck that this isnt a systemic issue which could covertly affect other keys? Now we know there is an issue that some keys can be hooked and not prevented from being rehooked? 

I would have thought that was an important thing to double-check couldn't happen.

Comment 10

[Simon Bünzli]

I doubt that this issue is systemic in any way. A brief search through the sources didn't turn up any other occurrence of this issue. Should you still find one, please just file another bug.

Comment 11

[Mike Connor [:mconnor]]

Comment on attachment 216971 [details] [diff] [review]
prevent the [Ctrl]+[F4] event from bubbling

If we had this as a <key> in browser, anyone else using <tabbrowser/> would have to reimplement it.

Comment 12

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

mozilla/toolkit/content/widgets/tabbrowser.xml 	1.103.2.33
mozilla/toolkit/content/widgets/tabbrowser.xml 	1.142