Bug 1787979 - keyboard shortcut navigates to the wrong tab from Firefox View tab r=dao

* Remove index: 0 in FirefoxViewHandler.openTab
* Add condition in tabbrowser _selectNextTab for FirefoxView tab
* Fix tests

Differential Revision: https://phabricator.services.mozilla.com/D156688
This commit is contained in:
Sarah Clements 2022-09-14 15:20:01 +00:00
parent 39713de281
commit 204e4fc381
9 changed files with 52 additions and 40 deletions

View File

@ -9934,7 +9934,7 @@ var FirefoxViewHandler = {
);
}
if (!this.tab) {
this.tab = gBrowser.addTrustedTab("about:firefoxview", { index: 0 });
this.tab = gBrowser.addTrustedTab("about:firefoxview");
this.tab.addEventListener("TabClose", this, { once: true });
gBrowser.tabContainer.addEventListener("TabSelect", this);
window.addEventListener("activate", this);

View File

@ -14,11 +14,14 @@ add_task(async function() {
let testTab = BrowserTestUtils.addTab(gBrowser);
let firefoxViewTab = BrowserTestUtils.addTab(gBrowser, "about:firefoxview");
gBrowser.hideTab(firefoxViewTab);
let visible = gBrowser.visibleTabs;
is(visible.length, 3, "3 tabs should be open");
is(visible.length, 3, "3 tabs should be visible");
is(visible[0], pinned, "the pinned tab is first");
is(visible[1], origTab, "original tab is next");
is(visible[2], testTab, "last created tab is last");
is(visible[2], testTab, "last created tab is next to last");
// Only show the test tab (but also get pinned and selected)
is(
@ -37,7 +40,7 @@ add_task(async function() {
is(visible.length, 2, "2 tabs should be visible including the pinned");
is(visible[0], pinned, "first is pinned");
is(visible[1], testTab, "next is the test tab");
is(gBrowser.tabs.length, 3, "3 tabs should still be open");
is(gBrowser.tabs.length, 4, "4 tabs should still be open");
gBrowser.selectTabAtIndex(1);
is(gBrowser.selectedTab, testTab, "second tab is the test tab");
@ -72,8 +75,22 @@ add_task(async function() {
gBrowser.tabContainer.advanceSelectedTab(-1, true);
is(gBrowser.selectedTab, testTab, "next to test tab again");
// Try showing all tabs
gBrowser.showOnlyTheseTabs(Array.from(gBrowser.tabs));
// select a hidden tab thats selectable
gBrowser.selectedTab = firefoxViewTab;
gBrowser.tabContainer.advanceSelectedTab(1, true);
is(gBrowser.selectedTab, pinned, "next to first visible tab, the pinned tab");
gBrowser.tabContainer.advanceSelectedTab(1, true);
is(gBrowser.selectedTab, testTab, "next to second visible tab, the test tab");
// again select a hidden tab thats selectable
gBrowser.selectedTab = firefoxViewTab;
gBrowser.tabContainer.advanceSelectedTab(-1, true);
is(gBrowser.selectedTab, testTab, "next to last visible tab, the test tab");
gBrowser.tabContainer.advanceSelectedTab(-1, true);
is(gBrowser.selectedTab, pinned, "next to first visible tab, the pinned tab");
// Try showing all tabs except for the Firefox View tab
gBrowser.showOnlyTheseTabs(Array.from(gBrowser.tabs.slice(0, 3)));
is(gBrowser.visibleTabs.length, 3, "all 3 tabs are visible again");
// Select the pinned tab and show the testTab to make sure selection updates
@ -94,7 +111,10 @@ add_task(async function() {
visible = gBrowser.visibleTabs;
is(visible.length, 1, "only the original tab is visible");
is(visible[0], testTab, "it's the original tab");
is(gBrowser.tabs.length, 2, "still have 2 open tabs");
is(gBrowser.tabs.length, 3, "still have 3 open tabs");
// Close the selectable hidden tab
gBrowser.removeTab(firefoxViewTab);
// Close the last visible tab and make sure we still get a visible tab
gBrowser.removeTab(testTab);

View File

@ -64,9 +64,8 @@ add_task(async function number_tab_select_shortcut() {
AppConstants.MOZ_WIDGET_GTK ? { altKey: true } : { accelKey: true },
win
);
is(
win.gBrowser.tabContainer.selectedIndex,
1,
ok(
!win.FirefoxViewHandler.tab.selected,
"Number shortcut to select the first tab skipped the Firefox View tab"
);
await BrowserTestUtils.closeWindow(win);

View File

@ -151,11 +151,6 @@ async function waitForVisibleSetupStep(browser, expected) {
function assertFirefoxViewTab(w) {
ok(w.FirefoxViewHandler.tab, "Firefox View tab exists");
ok(w.FirefoxViewHandler.tab?.hidden, "Firefox View tab is hidden");
is(
w.gBrowser.tabs.indexOf(w.FirefoxViewHandler.tab),
0,
"Firefox View tab is the first tab"
);
is(
w.gBrowser.visibleTabs.indexOf(w.FirefoxViewHandler.tab),
-1,
@ -175,7 +170,7 @@ async function openFirefoxViewTab(w) {
w
);
assertFirefoxViewTab(w);
is(w.gBrowser.tabContainer.selectedIndex, 0, "Firefox View tab is selected");
ok(w.FirefoxViewHandler.tab.selected, "Firefox View tab is selected");
await BrowserTestUtils.browserLoaded(w.FirefoxViewHandler.tab.linkedBrowser);
return w.FirefoxViewHandler.tab;
}

View File

@ -384,17 +384,12 @@ function test_newtab(testInfo, browserURL = "about:newtab") {
async function assertFirefoxViewTabSelected(w) {
ok(w.FirefoxViewHandler.tab, "Firefox View tab exists");
ok(w.FirefoxViewHandler.tab?.hidden, "Firefox View tab is hidden");
is(
w.gBrowser.tabs.indexOf(w.FirefoxViewHandler.tab),
0,
"Firefox View tab is the first tab"
);
is(
w.gBrowser.visibleTabs.indexOf(w.FirefoxViewHandler.tab),
-1,
"Firefox View tab is not in the list of visible tabs"
);
is(w.gBrowser.tabContainer.selectedIndex, 0, "Firefox View tab is selected");
ok(w.FirefoxViewHandler.tab.selected, "Firefox View tab is selected");
await BrowserTestUtils.browserLoaded(w.FirefoxViewHandler.tab.linkedBrowser);
}

View File

@ -18,8 +18,8 @@ add_task(async function test_TODO() {
is(
state.windows[0].selected,
1,
"The selected tab is Firefox view tab which is the first tab"
3,
"The selected tab is Firefox view tab which is the third tab"
);
gBrowser.selectedTab = tab;
@ -28,7 +28,7 @@ add_task(async function test_TODO() {
// The FxView tab doesn't get recorded in the session state and when we restore we want
// to open the tab that was previously opened so we record the tab position minus one
is(state.windows[0].selected, 2, "The selected tab is the second tab");
is(state.windows[0].selected, 1, "The selected tab is the first tab");
gBrowser.removeTab(window.FirefoxViewHandler.tab);
gBrowser.removeTab(gBrowser.selectedTab);

View File

@ -112,7 +112,7 @@ async function openFirefoxViewTab(w) {
w
);
assertFirefoxViewTab(w);
is(w.gBrowser.tabContainer.selectedIndex, 0, "Firefox View tab is selected");
ok(w.FirefoxViewHandler.tab.selected, "Firefox View tab is selected");
await BrowserTestUtils.browserLoaded(w.FirefoxViewHandler.tab.linkedBrowser);
return w.FirefoxViewHandler.tab;
}
@ -120,11 +120,6 @@ async function openFirefoxViewTab(w) {
function assertFirefoxViewTab(w) {
ok(w.FirefoxViewHandler.tab, "Firefox View tab exists");
ok(w.FirefoxViewHandler.tab?.hidden, "Firefox View tab is hidden");
is(
w.gBrowser.tabs.indexOf(w.FirefoxViewHandler.tab),
0,
"Firefox View tab is the first tab"
);
is(
w.gBrowser.visibleTabs.indexOf(w.FirefoxViewHandler.tab),
-1,

View File

@ -34,11 +34,6 @@ add_setup(async () => {
function assertFirefoxViewTab(w = window) {
ok(w.FirefoxViewHandler.tab, "Firefox View tab exists");
ok(w.FirefoxViewHandler.tab?.hidden, "Firefox View tab is hidden");
is(
w.gBrowser.tabs.indexOf(w.FirefoxViewHandler.tab),
0,
"Firefox View tab is the first tab"
);
is(
w.gBrowser.visibleTabs.indexOf(w.FirefoxViewHandler.tab),
-1,

View File

@ -758,10 +758,23 @@
advanceSelectedTab(aDir, aWrap) {
let startTab = this.ariaFocusedItem || this.selectedItem;
let newTab = this.findNextTab(startTab, {
direction: aDir,
wrap: aWrap,
});
let newTab = null;
// Handle keyboard navigation for a hidden tab that can be selected, like the Firefox View tab,
// which has a random placement in this.allTabs.
if (startTab.hidden) {
if (aDir == 1) {
newTab = this.allTabs.find(tab => !tab.hidden);
} else {
newTab = this.allTabs.findLast(tab => !tab.hidden);
}
} else {
newTab = this.findNextTab(startTab, {
direction: aDir,
wrap: aWrap,
});
}
if (newTab && newTab != startTab) {
this._selectNewTab(newTab, aDir, aWrap);
}