Last Comment Bug 679255 - “Close” Button on Tabs Reacts to Spacebar but Performs No Corresponding Action
: “Close” Button on Tabs Reacts to Spacebar but Performs No Corresponding Action
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Tabbed Browser (show other bugs)
: Trunk
: All All
: -- minor (vote)
: Firefox 9
Assigned To: Dão Gottwald [:dao]
:
:
Mentors:
Depends on: 684444
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-15 19:21 PDT by eviljoel
Modified: 2011-09-04 01:27 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
stop making the close button focusable (2.89 KB, patch)
2011-08-17 12:30 PDT, Dão Gottwald [:dao]
no flags Details | Diff | Splinter Review
stop making the close button focusable, v2 (3.65 KB, patch)
2011-08-29 00:15 PDT, Dão Gottwald [:dao]
enndeakin: review+
Details | Diff | Splinter Review

Description eviljoel 2011-08-15 19:21:00 PDT
This bug originated in Ubuntu's tracker as Bug #779581 (https://bugs.launchpad.net/ubuntu/+source/firefox/+bug/779581).  The original issue was that if the close button on a tab is in focus and the spacebar is pressed, the tab does not close.  One might believe that the tab is suppose to close because the close button responds as though it is being pressed.  However, upon further investigation, it is apparent that the erronous behavior is that either the close button becomes in focus or that once the button is in focus it responds to the spacebar.  This behavior was tested in Mozilla Firefox for Ubuntu 3.6.18, Mozilla Firefox for Ubuntu 4.0.1, official Mozilla build of Firefox 5.0.1 for Linux and with some success on an official Mozilla build of Firefox 5.0.1 for Windows.

In Windows the behavior is a little different.  Here you can get similar results by clicking and holding the close button (to bring focus to it), and then pressing spacebar (without releasing the mouse button).  Like the Ubuntu versions, the close button has the clicking animation but does not close the tab (until the mouse button is released).

Thus far, no one has tested this issue in Mac OSX.

I dug into the code in the http://hg.mozilla.org/releases/mozilla-2.0/ (63475:5216dd412535) repository to try to find the problem.  On line 20 of browser/base/content/tabbrowser.css there is an interesting comment:

.tab-close-button[selected="true"] {
  /* Make this button focusable so clicking on it will not focus the tab while
     it's getting closed */
  -moz-user-focus: normal;
}

Changing this value to 'none' or 'ignore' results in the tab getting focus when the close button is pressed.  It might be better to just let the tab have focus but this is a less than an optimal solution.  

One might also want to look at line 3507 of browser/base/content/tabbrowser.xml:

          <xul:toolbarbutton anonid="close-button"
                             xbl:inherits="fadein,pinned,selected"
                             tabindex="-1"
                             clickthrough="never"
                             class="tab-close-button"/>

My knowledge of this codebase is not so great, so any futher guildance in resolving this issue will be appreciated.

In my opinion, having a response to the keyboard that does not result in a useful action is not appropriate. (Of course, this situation is so hard to find that it may not be worth fixing.)  

Please let me know if you need more information.  Thanks.
Comment 1 (mostly gone) XtC4UaLL [:xtc4uall] 2011-08-16 16:51:00 PDT
Confirmed against Mozilla/5.0 (Windows NT 5.1; rv:8.0a1) Gecko/20110816 Firefox/8.0a1 ID:20110816100414
Comment 2 Dão Gottwald [:dao] 2011-08-17 12:30:48 PDT
Created attachment 553868 [details] [diff] [review]
stop making the close button focusable
Comment 3 Neil Deakin (away until Oct 3) 2011-08-26 17:14:56 PDT
Comment on attachment 553868 [details] [diff] [review]
stop making the close button focusable

Is this reverting back 462289 or is that fixed in some other way?

What effect does removing stopPropagation have here?
Comment 4 Dão Gottwald [:dao] 2011-08-27 01:09:23 PDT
(In reply to Neil Deakin from comment #3)
> Comment on attachment 553868 [details] [diff] [review]
> stop making the close button focusable
> 
> Is this reverting back 462289 or is that fixed in some other way?

It's still there, I'm only folding the three handlers with button="0"/"1"/"2" into one.

> What effect does removing stopPropagation have here?

It was needed to prevent the tab getting focused when clicking the close button, pre-bug 462289. Maintaining that branch now would regress bug 462289, since we wouldn't set MozUserFocus = 'ignore' in that case.
Comment 5 Neil Deakin (away until Oct 3) 2011-08-28 17:25:33 PDT
Where does the close button get -moz-user-focus set to normal, that we need to set it to 'ignore'?

Can we remove the rule for '.tab-close-button:focus' from windows browser.css?
Comment 6 Dão Gottwald [:dao] 2011-08-28 23:58:27 PDT
(In reply to Neil Deakin from comment #5)
> Where does the close button get -moz-user-focus set to normal, that we need
> to set it to 'ignore'?

We don't set it to 'ignore' for the close button but for the tab.

> Can we remove the rule for '.tab-close-button:focus' from windows
> browser.css?

Oh, yes.
Comment 7 Dão Gottwald [:dao] 2011-08-29 00:15:13 PDT
Created attachment 556483 [details] [diff] [review]
stop making the close button focusable, v2
Comment 8 Marco Bonardo [::mak] 2011-08-31 02:02:33 PDT
http://hg.mozilla.org/mozilla-central/rev/c706c08c24ee

Note You need to log in before you can comment on or make changes to this bug.