351289 - Using ctrl-F4 to close the only remaining tab is broken

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1b2) Gecko/20060902 SeaMonkey/1.1b] (nightly) (W98SE)

0. (Start SeaMonkey (browser))
1. Ctrl-N to open new browser [mine opens with <about:>]
2. Ctrl-F4 to close current "tab"
r. A new blank tab replaces the previous one, but the tab bar shows up :-(

Could this be caused by bug 156082 fix ?

There could also be a (unexpected) different behavior between Ctrl-F4 and Ctrl-W !?

Comment 1

[[e-mail]]

(In reply to comment #0)
>There could also be a (unexpected) different behavior between Ctrl-F4 and Ctrl-W !?
^W is expected to close the window when there is only one tab, ^F4 is not.

I wonder what Firefox does...

Comment 2

[Serge Gautherie (:sgautherie)]

Workaround:
3. Ctrl-T to open another blank tab.
4. Ctrl-F4 to close it.
r. The tab bar is hidden too.

Comment 3

[Serge Gautherie (:sgautherie)]

(In reply to comment #1)
> I wonder what Firefox does...

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1b2) Gecko/2006090304 BonEcho/2.0b2] (nightly) (W98SE)

On multiple tabs,
Ctrl-F4 closes the tab. (as SeaMonkey does)

On single tab,
Ctrl-F4 does nothing; (as SeaMonkey used to do !?)
and Ctrl-W closes the window.

Comment 4

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Interesting.  I'll try to fix this today.  ->me.

Comment 5

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: patch

If you show the tab bar:
 Ctrl+F4 => close tab

If you hide the tab bar:
 Ctrl+F4 => ignored

Comment 6

[[e-mail]]

Comment on attachment 236765 [details] [diff] [review]
patch

1. You shouldn't early return, you should reverse your condition instead.
2. You're testing the wrong thing, there are three reasons for a hidden strip.

Comment 7

[Serge Gautherie (:sgautherie)]

(In reply to comment #0)
> Could this be caused by bug 156082 fix ?

(For the record, Chris replied to me that "It was true".)

Comment 8

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

(In reply to comment #6)
> (From update of attachment 236765 [details] [diff] [review] [edit])
> 1. You shouldn't early return, you should reverse your condition instead.
> 2. You're testing the wrong thing, there are three reasons for a hidden strip.
> 

Could you go into a little more detail for issue #2?

Comment 9

[[e-mail]]

(In reply to comment #8)
>Could you go into a little more detail for issue #2?
We hide the tab bar in a popup window, or via View -> Show/Hide -> Tab Bar

Comment 10

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: patch [Checkin: Comment 12]

v2

Comment 11

[[e-mail]]

Comment on attachment 237221 [details] [diff] [review]
patch
[Checkin: Comment 12]

>-            this.tabbrowser.mTabBox.handleCtrlPageUpDown)
>-          this.tabbrowser.removeCurrentTab();
>+              this.tabbrowser.mTabBox.handleCtrlPageUpDown &&
>+              this.tabbrowser.getStripVisibility()) {
>+            this.tabbrowser.removeCurrentTab();
>+          }
Two nits: 1. Why the extra indentation? 2. Why the extra {}s?

Comment 12

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Checked in on trunk.

(In reply to comment #11)
> (From update of attachment 237221 [details] [diff] [review] [edit])
> >-            this.tabbrowser.mTabBox.handleCtrlPageUpDown)
> >-          this.tabbrowser.removeCurrentTab();
> >+              this.tabbrowser.mTabBox.handleCtrlPageUpDown &&
> >+              this.tabbrowser.getStripVisibility()) {
> >+            this.tabbrowser.removeCurrentTab();
> >+          }
> Two nits: 1. Why the extra indentation? 2. Why the extra {}s?

1. So the if conditions line up, and so the if's body is indented two spaces from the "i" in "if"
2. Fixed.

Comment 13

[Ian Neal]

Comment on attachment 237221 [details] [diff] [review]
patch
[Checkin: Comment 12]

Remove whichever approval you don't need ;-)

Comment 14

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Checked in for 1.1b.

Serge, in the future it is not necessary to change flags or summaries of my attachments.

Comment 15

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1) Gecko/20060930 SeaMonkey/1.1b] (nightly) (W98SE)

V.Fixed on MOZILLA_1_8_BRANCH, per comment 5 ...
(Marking bug as verified too, as it was branch only.)

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1) Gecko/2006100103 BonEcho/2.0] (nightly) (W98SE)

... which is what FF does (too).