Closed Bug 1402489 Opened 7 years ago Closed 7 years ago

Allow "switch to tab" to move the target tab into the current window


(Firefox :: Address Bar, enhancement, P5)




Firefox 58
Tracking Status
firefox58 --- fixed


(Reporter: nika, Assigned: nika)



(Whiteboard: [fxsearch])


(1 file, 1 obsolete file)

When the awesomebar gives a suggestion of an existing tab, I often want to be able to load that tab, as it has already been loaded, but I don't want to switch windows over to the window which actually contains the tab which I'm selecting.

I'd love a pref which changes the default behavior of the "switch to tab" option from switching to the window containing the target tab, to instead moving the target tab into the location of the current tab, and closing the current tab.

This might be doable as a webextension, but I'd rather avoid having to type a webextension specific keyword to trigger switch to tab search. The awesomebar's ability to just find what I'm looking for is very nice.
I'm calling this a p5 because we prioritize single window behaviors and it's unlikely we could spend time on this edge case.
If you can suggest a simple enough implementation we'd evaluate it!
Priority: -- → P5
This is first way to implement it which came to mind. I implemented it behind a pref as another option for how the switch to tab urlbar option could be implemented.

It doesn't impact behaviour for single-window usecases, but in multi-window usecases, it changes the code to adopt the tab into the active window, rather than switching windows.

MozReview-Commit-ID: 8Am4UKxF8x5
Attachment #8913425 - Flags: review?(mak77)
Fixed an eslint failure

MozReview-Commit-ID: 8Am4UKxF8x5
Attachment #8913824 - Flags: review?(mak77)
Attachment #8913425 - Attachment is obsolete: true
Attachment #8913425 - Flags: review?(mak77)
Assignee: nobody → nika
Whiteboard: [fxsearch]
Comment on attachment 8913824 [details] [diff] [review]
Add a pref for switch to tab moving the target tab into the current window

Review of attachment 8913824 [details] [diff] [review]:

LGTM, a mochitest-browser test would be the cherry on the cake. browser_bug1104165-switchtab-decodeuri.js could be a good starting point to make one, BrowserTestUtils may help. I'm not going to block on the test considered it's a disabled by default option, but it would be very welcome.

::: browser/base/content/urlbarBindings.xml
@@ +629,5 @@
> +                  let loadOpts = {
> +                    adoptIntoActiveWindow: this._adoptIntoActiveWindow,
> +                  };
> +
> +                  if (switchToTabHavingURI(url, false, loadOpts) && isTabEmpty(prevTab)) {

nit: reindent isTabEmpty into its own line.
Attachment #8913824 - Flags: review?(mak77) → review+
Pushed by
Add a pref for switch to tab moving the target tab into the current window, r=mak
Pushed by
Part 2: Fix formatting nit which I forgot to commit, r=mak
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
I'm a bit baffled how this patch got accepted without making a case for this pref being useful to a wide enough range of users. Obviously adding hidden prefs whenever a single user requests one isn't going to be sustainable...
Flags: needinfo?(mak77)
It has a decent use-case that I see it could be useful to other users, and I don't exclude it could even be evaluated as the default behavior, the code is not scary and it's well contained. In the past I'd have suggested to make an add-on, but now that's no more the case. That doesn't mean we should pick everything, but things that make sense can be evaluated.
Flags: needinfo?(mak77)
This particular one is super useful to me personally, I work with multiple tabs and windows and need to reorganize tabs into different windows all the time. With Firefox Panorama / TabGroups gone, this would fill the gaps for me. I tested it on the latest 58 and it works fine and as expected.
In fact, I'd love it if there was a perf to do it across devices via sync-ed tabs. That would also be useful but it might present some problems for people who want to mirror tabs across devices.
You need to log in before you can comment on or make changes to this bug.