Closed Bug 951618 Opened 11 years ago Closed 11 years ago

Always show the close button on the last tab

Categories

(Firefox :: Tabbed Browser, defect)

28 Branch
x86
All
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 31
Tracking Status
firefox31 --- verified

People

(Reporter: phlsa, Assigned: vtsatskin)

References

Details

(Whiteboard: p=2 s=it-31c-30a-29b.3 [qa!])

Attachments

(1 file, 2 obsolete files)

It is already possible to close the last tab using the menu or the keyboard shortcut. Firefox should be consistent here and also offer a close button on the last tab of a window.
Whiteboard: [triage]
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → DUPLICATE
No longer blocks: fxdesktopbacklog
Whiteboard: [triage]
There should be renewed discussion about this issue. Bug 724555 is a wontfix that references bug 455990. The main argument there is that removing the close button was consistent with Safari and a bunch of text editors back in 2008. Today, both IE and Chrome show a close button at all times and close the window when it is clicked. But above all, hiding the button is an inconsistency within Firefox. Under these circumstances I think there are enough reasons to rethink this decision. I'm not sure if reopening the bug is the right thing to do in terms of process. Dão, perhaps you can help me out here. @Marco, does this mean that we should include the bug into the triage again?
Status: RESOLVED → REOPENED
Flags: needinfo?(mmucci)
Flags: needinfo?(dao)
Resolution: DUPLICATE → ---
Flags: needinfo?(mmucci)
Whiteboard: [triage]
No longer blocks: fxdesktopbacklog
Whiteboard: [triage]
Whiteboard: [feature] p=0
Assignee: nobody → vtsatskin
Attached patch 951618.patch (obsolete) (deleted) — Splinter Review
I've gone ahead and created a new preference called browser.tabs.closeButtonOnLastTab. When the preference is enabled, the close button will always be shown on the last tab, otherwise the opposite is true, the close button will never be shown on the last tab. Background and previous behaviour: Through implementing the preference, I discovered there were four different modes of displaying close buttons. The mode is controlled by the browser.tabs.closeButtons preference. The modes are: - close buttons only shown on active tab (closeButtons == 0) - close buttons shown on all tabs (closeButtons == 1) - no close buttons at all (closeButtons == 2) - close buttons at end of tabstrip (closeButtons == 3) Previously if closeWindowWithLastTab was set to true, the close tab button would be hidden when only one tab was present. If it was set to false, the button would be shown but pressing it would close the tab and open a new "new tab" page. In closeButtons modes 2 and 3, the close tab button will never be shown, even if the closeButtonOnLastTab is set to true. Changed Behaviour: Previously in mode 0 and 1, the close button being shown on the last would be controlled by the closeWindowWithLastTab preference (as described earlier). Now however, closeWindowWithLastTab only controls if the window will close when closing the last tab (or a new tab page appears instead). closeButtonOnLastTab will control the showing the close button on the last tab. If closeButtons are in mode 2 or 3, closeButtonOnLastTab will have no effect on the tab close button because none should be shown. Test cases have been written to handle all combinations of these preferences, so if my explanation is not clear, the test should define the behaviour more explicitly.
Other patch notes: - I've removed a bunch of rouge hidden characters in firefox.js - The browser.tabs.closeButtonOnLastTab is set to false by default to align with current behaviour. If we choose to go through with the changes, it will be a simple line change.
Attachment #8390963 - Flags: review?(mconley)
Attachment #8390963 - Flags: review?(dao)
Attachment #8390963 - Flags: review?(bmcbride)
I think the best way to go forward from here is to land a patch that always shows the button and where the button closes the window on Nightly for some time and see if there are any reactions. Unless the feedback is hugely negative I'd move forward with it because of the external consistency gain.
Can we avoid adding a pref here? Doesn't seem necessary to make this pref-controllable.
Comment on attachment 8390963 [details] [diff] [review] 951618.patch (In reply to :Gavin Sharp (email gavin@gavinsharp.com) from comment #6) > Can we avoid adding a pref here? Doesn't seem necessary to make this > pref-controllable. yep
Attachment #8390963 - Flags: review?(mconley)
Attachment #8390963 - Flags: review?(dao)
Attachment #8390963 - Flags: review?(bmcbride)
Attachment #8390963 - Flags: review-
Flags: needinfo?(dao)
Attached patch 951618.no.pref.patch (obsolete) (deleted) — Splinter Review
I've removed the proposed pref entirely. Close buttons will always be shown in closeButton modes 0 and 1, never for 2 and 3.
Attachment #8393697 - Flags: review?(gavin.sharp)
Attachment #8393697 - Flags: review?(dao)
Comment on attachment 8393697 [details] [diff] [review] 951618.no.pref.patch Please remove this rule: http://hg.mozilla.org/mozilla-central/annotate/d6e549d77ab2/browser/themes/osx/browser.css#l2721 It's dead code now. Otherwise this looks good. I only reviewed the tabbrowser.xml change, don't think there's much value in the test.
Attachment #8393697 - Flags: review?(dao) → review-
Attachment #8390963 - Attachment is obsolete: true
Attached patch 951618.patch (deleted) — Splinter Review
Attachment #8393697 - Attachment is obsolete: true
Attachment #8393697 - Flags: review?(gavin.sharp)
Attachment #8394384 - Flags: review?(dao)
Comment on attachment 8394384 [details] [diff] [review] 951618.patch Again, I only reviewed the code changes, not the test.
Attachment #8394384 - Flags: review?(dao) → review+
Whiteboard: [feature] p=0 → [feature] p=2
No longer blocks: fxdesktopbacklog
Flags: firefox-backlog+
Whiteboard: [feature] p=2 → p=2
Flags: in-testsuite-
Blocks: 995626
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 31
Whiteboard: p=2 → p=2 s=it-31c-30a-29b.3 [qa?]
(In reply to Dão Gottwald [:dao] from comment #13) > Landed without the test: > https://hg.mozilla.org/integration/fx-team/rev/267197d42c90 Dao, do you need a test for this? We can get one created if you think it's valuable. Florin, please make sure this gets verified fixed.
QA Contact: florin.mezei
Whiteboard: p=2 s=it-31c-30a-29b.3 [qa?] → p=2 s=it-31c-30a-29b.3 [qa+]
(In reply to Dão Gottwald [:dao] from comment #13) > Landed without the test: > https://hg.mozilla.org/integration/fx-team/rev/267197d42c90 What was the rationale for removing the test? The test was created to define the behaviour of permutations of closeButtons and closeWindowWithLastTab. I see that as value enough to include it. (In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #15) > (In reply to Dão Gottwald [:dao] from comment #13) > > Landed without the test: > > https://hg.mozilla.org/integration/fx-team/rev/267197d42c90 > > Dao, do you need a test for this? We can get one created if you think it's > valuable. As per the above discussion, there is a test created for this bug.
(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #15) > Dao, do you need a test for this? No. (In reply to Valentin Tsatskin [:vt] from comment #16) > (In reply to Dão Gottwald [:dao] from comment #13) > > Landed without the test: > > https://hg.mozilla.org/integration/fx-team/rev/267197d42c90 > > What was the rationale for removing the test? Because I didn't think reviewing it was a good use of my time. > The test was created to define the behaviour of permutations of closeButtons Tests aren't free, they take time to run on every OS for every code change and they often need manual maintenance. I think we'd rather remove the hidden closeButtons pref than spending these resources on it. > and closeWindowWithLastTab. I see that as value enough to include it. This code doesn't even check closeWindowWithLastTab anymore. I don't see how this could change accidentally.
QA Contact: florin.mezei → paul.silaghi
(In reply to Philipp Sackl [:phlsa] from comment #0) > It is already possible to close the last tab using the menu What menu?
Tab right click context menu/close tab is still grayed out. Shouldn't be active ?
Flags: needinfo?(vtsatskin)
(In reply to Paul Silaghi, QA [:pauly] from comment #19) > Tab right click context menu/close tab is still grayed out. Shouldn't be > active ? Yes, it should. Please file a new bug.
Flags: needinfo?(vtsatskin)
(In reply to Paul Silaghi, QA [:pauly] from comment #18) > (In reply to Philipp Sackl [:phlsa] from comment #0) > > It is already possible to close the last tab using the menu > What menu? I was talking about the OS X menu.
Depends on: 996632
Verified fixed FF 31.0a1 (2014-04-16) Win 7, OS X 10.8, Ubuntu 13.04.
Status: RESOLVED → VERIFIED
Whiteboard: p=2 s=it-31c-30a-29b.3 [qa+] → p=2 s=it-31c-30a-29b.3 [qa!]
Wouldn't be nice to also close the last tab with middle click on the X button or anywhere in the tab area as it happens with the second tab ?
Flags: needinfo?(dao)
(In reply to Paul Silaghi, QA [:pauly] from comment #24) > Wouldn't be nice to also close the last tab with middle click on the X > button or anywhere in the tab area as it happens with the second tab ? -> bug 997681
Flags: needinfo?(dao)
Depends on: 997681
Thanks, Dao.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: