Closed
Bug 554300
Opened 15 years ago
Closed 14 years ago
"close tab" context menu item should be enabled for last tab when browser.tabs.closeWindowWithLastTab=false
Categories
(Firefox :: Tabbed Browser, defect)
Firefox
Tabbed Browser
Tracking
()
VERIFIED
FIXED
Firefox 4.0b2
People
(Reporter: eyalgruss, Assigned: mkohler)
References
Details
Attachments
(2 files, 3 obsolete files)
|
(deleted),
patch
|
dao
:
review+
|
Details | Diff | Splinter Review |
|
(deleted),
patch
|
dao
:
review-
|
Details | Diff | Splinter Review |
now that Bug 501714 is fixed, the "close tab" context menu item should be enabled for the last tab when browser.tabs.closeWindowWithLastTab=false
|
Reporter | |
Updated•15 years ago
|
Blocks: cuts-cruft
|
Assignee | |
Comment 1•14 years ago
|
||
|
||
Comment 2•14 years ago
|
||
Comment on attachment 456123 [details] [diff] [review]
Patch v1
For the item to always be enabled when it should, I think you need to remove tbattr="tabbrowser-multiple" from context_closeTab in browser.xul and use this:
> document.getElementById("context_closeTab").disabled = disabled && closeWithLastTab;
instead of:
>+ if (!closeWithLastTab && disabled)
>+ document.getElementById("context_closeTab").removeAttribute("tbattr");
>+ else
>+ document.getElementById("context_closeTab").setAttribute("tbattr", "tabbrowser-multiple");
Attachment #456123 -
Flags: review?(dao) → review-
|
|
Assignee | |
Comment 3•14 years ago
|
||
(In reply to comment #2)
> (From update of attachment 456123 [details] [diff] [review])
> For the item to always be enabled when it should, I think you need to remove
> tbattr="tabbrowser-multiple" from context_closeTab in browser.xul and use this:
>
> > document.getElementById("context_closeTab").disabled = disabled && closeWithLastTab;
This is simpler, yes.
Attachment #456123 -
Attachment is obsolete: true
Attachment #456153 -
Flags: review?(dao)
|
|
||
Comment 4•14 years ago
|
||
Comment on attachment 456153 [details] [diff] [review]
Patch v2
>+ // Enable the "Close Tab" menuitem when the window doesn't close with the last tab.
>+ var closeWithLastTab = gPrefService.getBoolPref("browser.tabs.closeWindowWithLastTab");
>+ document.getElementById("context_closeTab").disabled = disabled && closeWithLastTab;
Could also be done without accessing the actual pref:
document.getElementById("context_closeTab").disabled =
disabled && gBrowser.tabContainer._closeWindowWithLastTab;
Attachment #456153 -
Flags: review?(dao) → review+
|
|
Assignee | |
Comment 5•14 years ago
|
||
Nit addressed.
Attachment #456153 -
Attachment is obsolete: true
Attachment #456859 -
Flags: review?(dao)
|
|
||
Comment 6•14 years ago
|
||
Comment on attachment 456859 [details] [diff] [review]
Patch v3
I don't see a difference between this and v2.
Attachment #456859 -
Flags: review?(dao)
|
|
Assignee | |
Comment 7•14 years ago
|
||
sorry, I attached the wrong patch. Now it should be the correct one.
Attachment #456859 -
Attachment is obsolete: true
Attachment #456867 -
Flags: review?(dao)
|
|
||
Updated•14 years ago
|
Attachment #456867 -
Flags: review?(dao) → review+
|
|
Assignee | |
Updated•14 years ago
|
Keywords: checkin-needed
|
|
||
Comment 8•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b2
|
||
Comment 9•14 years ago
|
||
Michael, could you also write an automated test for it?
Flags: in-testsuite?
|
|
||
Comment 10•14 years ago
|
||
Verified with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; en-US; rv:2.0b2pre) Gecko/20100718 Minefield/4.0b2pre
Status: RESOLVED → VERIFIED
|
|
Assignee | |
Comment 11•14 years ago
|
||
Attachment #458768 -
Flags: review?(dao)
|
|
||
Comment 12•14 years ago
|
||
Comment on attachment 458768 [details] [diff] [review]
Test v1
>+function test() {
>+ // Open a tab and see wether the "Close Tab" menuitem is enabled
>+ // if browser.tabs.closeWindowWithLastTab is set to false and disabled
>+ // if it is set to true.
>+ var tab = gBrowser.addTab();
This is going to be the second tab in that window, which makes the test not test what it intends to test...
>+ var prefs = Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefService);
Services.prefs
>+ prefs.setBoolPref("browser.tabs.closeWindowWithLastTab", false);
>+ EventUtils.synthesizeMouse(gBrowser.tabs[0], 1, 1, { type : "contextmenu", button: 0 });
>+ ok(!document.getElementById("context_closeTab").disabled, "'Close Tab' menuitem is enabled.");
>+
>+ prefs.setBoolPref("browser.tabs.closeWindowWithLastTab", true);
You should reset the pref to its original value, even if that happened to be false.
Attachment #458768 -
Flags: review?(dao) → review-
|
||
Updated•14 years ago
|
No longer blocks: cuts-cruft
You need to log in
before you can comment on or make changes to this bug.
Description
•