Always show the close button on the last tab

VERIFIED FIXED in Firefox 31

Status

()

Firefox
Tabbed Browser
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: phlsa, Assigned: vt)

Tracking

28 Branch
Firefox 31
x86
All
Points:
---
Dependency tree / graph
Bug Flags:
firefox-backlog +
in-testsuite -

Firefox Tracking Flags

(firefox31 verified)

Details

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

Attachments

(1 attachment, 2 obsolete attachments)

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.

Updated

4 years ago
Blocks: 950073
Whiteboard: [triage]

Updated

4 years ago
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 724555

Updated

4 years ago
No longer blocks: 950073
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 → ---

Updated

4 years ago
Blocks: 950073
Flags: needinfo?(mmucci)
Whiteboard: [triage]

Updated

4 years ago
No longer blocks: 950073
Whiteboard: [triage]

Updated

4 years ago
Blocks: 950073
Whiteboard: [feature] p=0
(Assignee)

Updated

4 years ago
Assignee: nobody → vtsatskin
(Assignee)

Comment 3

4 years ago
Created attachment 8390963 [details] [diff] [review]
951618.patch

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.
(Assignee)

Comment 4

4 years ago
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.
(Assignee)

Updated

4 years ago
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)
(Assignee)

Comment 8

4 years ago
Created attachment 8393697 [details] [diff] [review]
951618.no.pref.patch

I've removed the proposed pref entirely. Close buttons will always be shown in closeButton modes 0 and 1, never for 2 and 3.
(Assignee)

Updated

4 years ago
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-

Updated

4 years ago
Attachment #8390963 - Attachment is obsolete: true
(Assignee)

Comment 10

4 years ago
Created attachment 8394384 [details] [diff] [review]
951618.patch
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+

Updated

4 years ago
Duplicate of this bug: 987632

Updated

4 years ago
Whiteboard: [feature] p=0 → [feature] p=2

Updated

4 years ago
No longer blocks: 950073
Flags: firefox-backlog+
Whiteboard: [feature] p=2 → p=2
Landed without the test:
https://hg.mozilla.org/integration/fx-team/rev/267197d42c90
Flags: in-testsuite-

Updated

4 years ago
Blocks: 995626
https://hg.mozilla.org/mozilla-central/rev/267197d42c90
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 31

Updated

4 years ago
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.
status-firefox31: --- → fixed
QA Contact: florin.mezei
Whiteboard: p=2 s=it-31c-30a-29b.3 [qa?] → p=2 s=it-31c-30a-29b.3 [qa+]
(Assignee)

Comment 16

4 years ago
(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
status-firefox31: fixed → verified

Updated

4 years ago
Duplicate of this bug: 724555
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)

Updated

4 years ago
Depends on: 997681
Thanks, Dao.

Updated

4 years ago
Duplicate of this bug: 919952
You need to log in before you can comment on or make changes to this bug.