Closed
Bug 997681
Opened 10 years ago
Closed 10 years ago
Allow closing the last tab with a middle click
Categories
(Firefox :: Tabbed Browser, defect)
Firefox
Tabbed Browser
Tracking
()
VERIFIED
FIXED
Firefox 31
Tracking | Status | |
---|---|---|
firefox31 | --- | verified |
People
(Reporter: dao, Assigned: dao)
References
Details
(Whiteboard: [good first bug][mentor=dao][lang=js])
Attachments
(1 file)
4.13 KB,
patch
|
Gijs
:
review+
|
Details | Diff | Splinter Review |
This goes back to bug 456382. Bug 995626 made us show the close button on the last tab. Should the same change be made for middle click?
Flags: needinfo?(philipp)
Woah. Middle-clicking that is for clearing that tab to restart with about:newtab At least when browser.tabs.closeWindowWithLastTab is false or browser.tabs.closeButtons is set to 2
Assignee | ||
Comment 2•10 years ago
|
||
This is about browser.tabs.closeWindowWithLastTab being true (the default setting).
By "restart" I didn't mean restart the browser, just as a healthy mental state :)
(In reply to Dão Gottwald [:dao] from comment #2) > This is about browser.tabs.closeWindowWithLastTab being true (the default > setting). Ok, as long as the non-default setting behaviour isn't changed by this.
Comment 5•10 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #0) > This goes back to bug 456382. > > Bug 995626 made us show the close button on the last tab. Should the same > change be made for middle click? Yes, that makes sense.
Flags: needinfo?(philipp)
Assignee | ||
Comment 6•10 years ago
|
||
The code that needs to change is here: http://hg.mozilla.org/mozilla-central/annotate/c962bde5ac0b/browser/base/content/tabbrowser.xml#l4264 We need to remove the "this.childNodes.length > 1 || !this._closeWindowWithLastTab" check.
Keywords: uiwanted
Whiteboard: [good first bug][mentor=dao][lang=js]
Assignee | ||
Comment 7•10 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #0) > Bug 995626 made us show the close button on the last tab. Actually, bug 951618 did that.
Assignee | ||
Updated•10 years ago
|
Flags: firefox-backlog?
Assignee | ||
Comment 8•10 years ago
|
||
Assignee: nobody → dao
Status: NEW → ASSIGNED
Attachment #8412590 -
Flags: review?(gijskruitbosch+bugs)
Comment 9•10 years ago
|
||
Comment on attachment 8412590 [details] [diff] [review] patch Review of attachment 8412590 [details] [diff] [review]: ----------------------------------------------------------------- This looks sane to me. ::: browser/base/content/tabbrowser.xml @@ -3473,5 @@ > - tabContainer: this, > - > - observe: function (subject, topic, data) { > - switch (data) { > - case "browser.tabs.closeWindowWithLastTab": Uh, interesting. On m-c this still has another case for which we're observing (close buttons), but it seems that got removed in bug 865826... @@ +4226,5 @@ > if (event.button != 1) > return; > > if (event.target.localName == "tab") { > + this.tabbrowser.removeTab(event.target, {animate: true, byMouse: true}); Animating if this is the last tab seems useless. Maybe pass animate: this.childNodes.length == 1 ? (This is pretty orthogonal to the patch, so I don't much care if you do or don't (want to) do this, just an observation)
Attachment #8412590 -
Flags: review?(gijskruitbosch+bugs) → review+
Assignee | ||
Comment 10•10 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #9) > > + this.tabbrowser.removeTab(event.target, {animate: true, byMouse: true}); > > Animating if this is the last tab seems useless. Maybe pass animate: > this.childNodes.length == 1 ? removeTab won't allow the animation to happen for the last tab. https://hg.mozilla.org/integration/fx-team/rev/09863b96e772
Comment 11•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/09863b96e772
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 31
Comment 12•10 years ago
|
||
Verified fixed 31.0a1 (2014-04-28) Win 7
Status: RESOLVED → VERIFIED
status-firefox31:
--- → verified
Assignee | ||
Updated•10 years ago
|
Flags: firefox-backlog?
You need to log in
before you can comment on or make changes to this bug.
Description
•