Bug 1420601 - Let Accel+W in a pinned tab select the first unpinned tab. r?daleharvey
MozReview-Commit-ID: DNhOuW4BL3P
--- a/browser/base/content/browser-sets.inc
+++ b/browser/base/content/browser-sets.inc
@@ -26,17 +26,17 @@
<command id="Browser:SavePage" oncommand="saveBrowser(gBrowser.selectedBrowser);"/>
<command id="Browser:SendLink"
oncommand="MailIntegration.sendLinkForBrowser(gBrowser.selectedBrowser);"/>
<command id="cmd_pageSetup" oncommand="PrintUtils.showPageSetup();"/>
<command id="cmd_print" oncommand="PrintUtils.printWindow(window.gBrowser.selectedBrowser.outerWindowID, window.gBrowser.selectedBrowser);"/>
<command id="cmd_printPreview" oncommand="PrintUtils.printPreview(PrintPreviewListener);"/>
- <command id="cmd_close" oncommand="BrowserCloseTabOrWindow()"/>
+ <command id="cmd_close" oncommand="BrowserCloseTabOrWindow(event);"/>
<command id="cmd_closeWindow" oncommand="BrowserTryToCloseWindow()"/>
<command id="cmd_toggleMute" oncommand="gBrowser.selectedTab.toggleMuteAudio()"/>
<command id="cmd_CustomizeToolbars" oncommand="gCustomizeMode.enter()"/>
<command id="cmd_quitApplication" oncommand="goQuitApplication()"/>
<commandset id="editMenuCommands"/>
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -2365,23 +2365,34 @@ function BrowserOpenFileWindow() {
nsIFilePicker.filterImages | nsIFilePicker.filterXML |
nsIFilePicker.filterHTML);
fp.displayDirectory = gLastOpenDirectory.path;
fp.open(fpCallback);
} catch (ex) {
}
}
-function BrowserCloseTabOrWindow() {
- // If we're not a browser window, just close the window
+function BrowserCloseTabOrWindow(event) {
+ // If we're not a browser window, just close the window.
if (window.location.href != getBrowserURL()) {
closeWindow(true);
return;
}
+ // Keyboard shortcuts that would close a tab that is pinned select the first
+ // unpinned tab instead.
+ if (event &&
+ (event.ctrlKey || event.metaKey || event.altKey) &&
+ gBrowser.selectedTab.pinned) {
+ if (gBrowser.visibleTabs.length > gBrowser._numPinnedTabs) {
+ gBrowser.tabContainer.selectedIndex = gBrowser._numPinnedTabs;
+ }
+ return;
+ }
+
// If the current tab is the last one, this will close the window.
gBrowser.removeCurrentTab({animate: true});
}
function BrowserTryToCloseWindow() {
if (WindowIsClosing())
window.close(); // WindowIsClosing does all the necessary checks
}
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -206,52 +206,16 @@
if (this._statusPanel) {
let browser = this.selectedBrowser;
let browserContainer = this.getBrowserContainer(browser);
browserContainer.insertBefore(this._statusPanel, browser.parentNode.nextSibling);
}
]]></body>
</method>
- <method name="_setCloseKeyState">
- <parameter name="aEnabled"/>
- <body><![CDATA[
- let keyClose = document.getElementById("key_close");
- let closeKeyEnabled = keyClose.getAttribute("disabled") != "true";
- if (closeKeyEnabled == aEnabled)
- return;
-
- if (aEnabled)
- keyClose.removeAttribute("disabled");
- else
- keyClose.setAttribute("disabled", "true");
-
- // We also want to remove the keyboard shortcut from the file menu
- // when the shortcut is disabled, and bring it back when it's
- // renabled.
- //
- // Fixing bug 630826 could make that happen automatically.
- // Fixing bug 630830 could avoid the ugly hack below.
-
- let closeMenuItem = document.getElementById("menu_close");
- let parentPopup = closeMenuItem.parentNode;
- let nextItem = closeMenuItem.nextSibling;
- let clonedItem = closeMenuItem.cloneNode(true);
-
- parentPopup.removeChild(closeMenuItem);
-
- if (aEnabled)
- clonedItem.setAttribute("key", "key_close");
- else
- clonedItem.removeAttribute("key");
-
- parentPopup.insertBefore(clonedItem, nextItem);
- ]]></body>
- </method>
-
<method name="pinTab">
<parameter name="aTab"/>
<body><![CDATA[
if (aTab.pinned)
return;
if (aTab.hidden)
this.showTab(aTab);
@@ -259,19 +223,16 @@
this.moveTabTo(aTab, this._numPinnedTabs);
aTab.setAttribute("pinned", "true");
this.tabContainer._unlockTabSizing();
this.tabContainer._positionPinnedTabs();
this.tabContainer._updateCloseButtons();
this.getBrowserForTab(aTab).messageManager.sendAsyncMessage("Browser:AppTab", { isAppTab: true });
- if (aTab.selected)
- this._setCloseKeyState(false);
-
let event = document.createEvent("Events");
event.initEvent("TabPinned", true, false);
aTab.dispatchEvent(event);
]]></body>
</method>
<method name="unpinTab">
<parameter name="aTab"/>
@@ -283,19 +244,16 @@
aTab.removeAttribute("pinned");
aTab.style.marginInlineStart = "";
this.tabContainer._unlockTabSizing();
this.tabContainer._positionPinnedTabs();
this.tabContainer._updateCloseButtons();
this.getBrowserForTab(aTab).messageManager.sendAsyncMessage("Browser:AppTab", { isAppTab: false });
- if (aTab.selected)
- this._setCloseKeyState(true);
-
let event = document.createEvent("Events");
event.initEvent("TabUnpinned", true, false);
aTab.dispatchEvent(event);
]]></body>
</method>
<method name="previewTab">
<parameter name="aTab"/>
@@ -1365,18 +1323,16 @@
this.mIsBusy = false;
this._callProgressListeners(null, "onStateChange",
[webProgress, null,
nsIWebProgressListener.STATE_STOP |
nsIWebProgressListener.STATE_IS_NETWORK, 0],
true, false);
}
- this._setCloseKeyState(!this.mCurrentTab.pinned);
-
// TabSelect events are suppressed during preview mode to avoid confusing extensions and other bits of code
// that might rely upon the other changes suppressed.
// Focus is suppressed in the event that the main browser window is minimized - focusing a tab would restore the window
if (!this._previewMode) {
// We've selected the new tab, so go ahead and notify listeners.
let event = new CustomEvent("TabSelect", {
bubbles: true,
cancelable: false,
--- a/browser/base/content/test/general/browser.ini
+++ b/browser/base/content/test/general/browser.ini
@@ -211,18 +211,16 @@ skip-if = toolkit != "cocoa" # Because o
[browser_bug575830.js]
# DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
[browser_bug577121.js]
# DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
[browser_bug578534.js]
# DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
[browser_bug579872.js]
# DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
-[browser_bug580638.js]
-# DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
[browser_bug580956.js]
# DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
[browser_bug581242.js]
# DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
[browser_bug581253.js]
# DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
[browser_bug585785.js]
# DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
@@ -377,18 +375,16 @@ support-files = feed_discovery.html
# DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
[browser_gZipOfflineChild.js]
support-files = test_offline_gzip.html gZipOfflineChild.cacheManifest gZipOfflineChild.cacheManifest^headers^ gZipOfflineChild.html gZipOfflineChild.html^headers^
# DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
[browser_page_style_menu.js]
# DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
[browser_page_style_menu_update.js]
# DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
-[browser_pinnedTabs.js]
-# DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
[browser_plainTextLinks.js]
# DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
[browser_printpreview.js]
skip-if = os == 'win' # Bug 1384127
# DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
[browser_private_browsing_window.js]
# DO NOT ADD MORE TESTS HERE. USE A TOPICAL DIRECTORY INSTEAD.
[browser_private_no_prompt.js]
--- a/browser/base/content/test/tabs/browser.ini
+++ b/browser/base/content/test/tabs/browser.ini
@@ -15,14 +15,16 @@ skip-if = !e10s # Tab spinner is e10s on
skip-if = os == 'mac'
[browser_navigatePinnedTab.js]
[browser_new_file_whitelisted_http_tab.js]
skip-if = !e10s # Test only relevant for e10s.
[browser_new_web_tab_in_file_process_pref.js]
skip-if = !e10s # Pref and test only relevant for e10s.
[browser_opened_file_tab_navigated_to_web.js]
[browser_overflowScroll.js]
+[browser_pinnedTabs.js]
+[browser_pinnedTabs_closeByKeyboard.js]
[browser_positional_attributes.js]
[browser_preloadedBrowser_zoom.js]
[browser_reload_deleted_file.js]
[browser_tabswitch_updatecommands.js]
[browser_viewsource_of_data_URI_in_file_process.js]
[browser_open_newtab_start_observer_notification.js]
rename from browser/base/content/test/general/browser_pinnedTabs.js
rename to browser/base/content/test/tabs/browser_pinnedTabs.js
rename from browser/base/content/test/general/browser_bug580638.js
rename to browser/base/content/test/tabs/browser_pinnedTabs_closeByKeyboard.js
--- a/browser/base/content/test/general/browser_bug580638.js
+++ b/browser/base/content/test/tabs/browser_pinnedTabs_closeByKeyboard.js
@@ -5,55 +5,58 @@
function test() {
waitForExplicitFinish();
function testState(aPinned) {
function elemAttr(id, attr) {
return document.getElementById(id).getAttribute(attr);
}
- if (aPinned) {
- is(elemAttr("key_close", "disabled"), "true",
- "key_close should be disabled when a pinned-tab is selected");
- is(elemAttr("menu_close", "key"), "",
- "menu_close shouldn't have a key set when a pinned is selected");
- } else {
- is(elemAttr("key_close", "disabled"), "",
- "key_closed shouldn't have disabled state set when a non-pinned tab is selected");
- is(elemAttr("menu_close", "key"), "key_close",
- "menu_close should have key_close set as its key when a non-pinned tab is selected");
- }
+ is(elemAttr("key_close", "disabled"), "",
+ "key_closed should always be enabled");
+ is(elemAttr("menu_close", "key"), "key_close",
+ "menu_close should always have key_close set");
}
- let lastSelectedTab = gBrowser.selectedTab;
- ok(!lastSelectedTab.pinned, "We should have started with a regular tab selected");
+ let unpinnedTab = gBrowser.selectedTab;
+ ok(!unpinnedTab.pinned, "We should have started with a regular tab selected");
testState(false);
- let pinnedTab = BrowserTestUtils.addTab(gBrowser, "about:blank");
+ let pinnedTab = gBrowser.addTab();
gBrowser.pinTab(pinnedTab);
// Just pinning the tab shouldn't change the key state.
testState(false);
- // Test updating key state after selecting a tab.
+ // Test key state after selecting a tab.
gBrowser.selectedTab = pinnedTab;
testState(true);
- gBrowser.selectedTab = lastSelectedTab;
+ gBrowser.selectedTab = unpinnedTab;
testState(false);
gBrowser.selectedTab = pinnedTab;
testState(true);
- // Test updating the key state after un/pinning the tab.
+ // Test the key state after un/pinning the tab.
gBrowser.unpinTab(pinnedTab);
testState(false);
gBrowser.pinTab(pinnedTab);
testState(true);
- // Test updating the key state after removing the tab.
+ // Test that accel+w in a pinned tab selects the next tab.
+ let pinnedTab2 = gBrowser.addTab();
+ gBrowser.pinTab(pinnedTab2);
+ gBrowser.selectedTab = pinnedTab;
+
+ EventUtils.synthesizeKey("w", { accelKey: true });
+ is(gBrowser.tabs.length, 3, "accel+w in a pinned tab didn't close it");
+ is(gBrowser.selectedTab, unpinnedTab, "accel+w in a pinned tab selected the first unpinned tab");
+
+ // Test the key state after removing the tab.
gBrowser.removeTab(pinnedTab);
+ gBrowser.removeTab(pinnedTab2);
testState(false);
finish();
}