Closed Bug 351289 Opened 18 years ago Closed 18 years ago

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

Categories

(SeaMonkey :: UI Design, defect)

1.8 Branch
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
seamonkey1.1beta

People

(Reporter: sgautherie, Assigned: csthomas)

References

Details

(Keywords: fixed-seamonkey1.1b, regression, Whiteboard: [verified-seamonkey1.1b])

Attachments

(1 file, 1 obsolete file)

[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 !?
(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...
Summary: Don't show the tab bar when clicking the close box (["Hide tab bar when only one tab is open"] pref is ignored) → Don't show the tab bar when using Ctrl-F4 on single tab (["Hide tab bar when only one tab is open"] pref is ignored)
Workaround:
3. Ctrl-T to open another blank tab.
4. Ctrl-F4 to close it.
r. The tab bar is hidden too.
(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.
Interesting.  I'll try to fix this today.  ->me.
Assignee: guifeatures → cst
Attached patch patch (obsolete) — Splinter Review
If you show the tab bar:
 Ctrl+F4 => close tab

If you hide the tab bar:
 Ctrl+F4 => ignored
Attachment #236765 - Flags: superreview?(neil)
Attachment #236765 - Flags: review?(neil)
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.
Attachment #236765 - Flags: superreview?(neil)
Attachment #236765 - Flags: superreview-
Attachment #236765 - Flags: review?(neil)
Attachment #236765 - Flags: review-
(In reply to comment #0)
> Could this be caused by bug 156082 fix ?

(For the record, Chris replied to me that "It was true".)
(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?
(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
Summary: Don't show the tab bar when using Ctrl-F4 on single tab (["Hide tab bar when only one tab is open"] pref is ignored) → Using ctrl-F4 to close the only remaining tab is broken
v2
Attachment #236765 - Attachment is obsolete: true
Attachment #237221 - Flags: superreview?(neil)
Attachment #237221 - Flags: review?(neil)
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?
Attachment #237221 - Flags: superreview?(neil)
Attachment #237221 - Flags: superreview+
Attachment #237221 - Flags: review?(neil)
Attachment #237221 - Flags: review+
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.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Attachment #237221 - Flags: approval-seamonkey1.1a?
Comment on attachment 237221 [details] [diff] [review]
patch
[Checkin: Comment 12]

Remove whichever approval you don't need ;-)
Attachment #237221 - Flags: approval-seamonkey1.1b+
Attachment #237221 - Flags: approval-seamonkey1.1a?
Attachment #237221 - Flags: approval-seamonkey1.1a+
Attachment #237221 - Flags: approval-seamonkey1.1a+
Attachment #237221 - Attachment description: patch → patch [Checkin: Comment 12]
Checked in for 1.1b.

Serge, in the future it is not necessary to change flags or summaries of my attachments.
[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).
Status: RESOLVED → VERIFIED
OS: Windows 98 → All
Hardware: PC → All
Whiteboard: [verified-seamonkey1.1b]
Component: XP Apps: GUI Features → UI Design
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: