1560218 - [WebExtensions] Add the ability to duplicate tab to a specified index

Status: RESOLVED FIXED

(WebExtensions :: Frontend, enhancement, P5)

Description

[ariasuni]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

browser.tabs.duplicate(tabId)

Actual results:

I can’t choose where a tab is been duplicated.

Expected results:

Firefox can duplicate tab to any index when drag-and-droping a tab. The WebExtensions API doesn’t let me choose where it should be opened and I should resort to an ugly hack in Tab Center Reborn to duplicate the native Firefox feature.

Comment 1

[Rob Wu [:robwu]]

This is quite a specific use case, and already covered by the tabs.move API.

Comment 2

[ariasuni]

It may be a bit specific, be it is not already covered by the tabs.move() API.

Notably, in tab sidebar add-ons, one may want to replicate the “duplicate on ctrl+drop” feature of Firefox. Obviously, moving the tab after the fact is just going to cause ugly jumps, in both Firefox tab bar and the tab sidebar. And duplicating a tab is working a bit differently than opening a whole new tab with the same URL.

Any other property can be set after the fact, but changing position is always going to make tabs jumps… Obviously I can hack with setTimeout() to avoid jumps in my extension but it adds a lot of complexity.

I guess that a few other tab managements add-ons may want to do that too.

I understand you may think it’s too much of a specific case, but there’s no way to properly work around this limitation of the API.

Comment 3

[ariasuni]

Any thought on this use case?

Comment 4

[Rob Wu [:robwu]]

We have previously sped up the resolution of tabs.duplicate() in bug 1394376. Now it resolves as soon as the new tab has started to be restored. Does that still not address your use case?

If your concern is the moving of tabs, then I can think of two alternatives:

• move the tab before duplicate to the desired position, then move it back.
• Use tabs.onCreated to detect the new tab and move it.

There are a few of other bug reports/feature requests on tabs.duplicate, so if we end up adding an extra dictionary on those (e.g. for bug 1376088), then I would be more willing to also support an index property. However, with the current implementation I still expect a jump because of the internal wait between creating the tab and restoring it: https://searchfox.org/mozilla-central/rev/f1f75f0a2d995241fbc454edfab4be1064c544e1/browser/components/extensions/parent/ext-tabs.js#1108,1115-1118

Patches are welcome.

Comment 5

[ariasuni]

We have previously sped up the resolution of tabs.duplicate() in bug 1394376. Now it resolves as soon as the new tab has started to be restored. Does that still not address your use case?

I’m the one who did the patch. The change is nice, but it makes this case worse because I can’t fix the tab position before the focus change (and I still can’t control the focus change, anyway).

If your concern is the moving of tabs, then I can think of two alternatives:

If I want to replicate Firefox’s behavior regarding duplicating with ctrl+drag, either way it’s gonna make tabs jump.

It can even sometimes scroll back and forth if the original and duplicated tab can’t be displayed in the same view, e.g. if a Firefox’s window is docked to a side of the screen (in this case a sidebar can show much more tabs than an horizontal tab bar), and that a tab at the top of the sidebar is dropped at the bottom of it, it probably won’t fit the native tab bar — not even talking about scrolling before drop-duplicating the tab… Obviously this is a worst case but still.

• move the tab before duplicate to the desired position, then move it back.

1. I move the tab (may scroll if the tab that we want to duplicate is active)
2. I duplicate the tab (may scroll if it didn’t in step 1)
3. I move back the original tab
4. I put focus back to the previously active tab (may scroll)
5. Scroll to newly created tab if it is now out of view (it will still be out of view in the native tab bar)

• Use tabs.onCreated to detect the new tab and move it.

1. I duplicate the tab (may scroll)
2. I put focus back to the previously active tab
3. I move the duplicated tab
4. Scroll to newly created tab if it is now out-of-view (it will still be out of view in the native tab bar)

There are a few of other bug reports/feature requests on tabs.duplicate, so if we end up adding an extra dictionary on those […]

In both cases, it would mean I could use browser.tabs.duplicate(tabId, { index: newIndex, active: false }) and be done with it, without the 5 hackish steps with the current implementation.

Patches are welcome.

This bug is marked as WONTFIX and my time is limited, so I didn’t take the risk to waste it for something that wasn’t gonna be accepted.

However, if they are chances that it would be accepted, I’m willing to try to implement a new parameter with an index and active property that would allow to cleanly replicate the tab duplicating capabilities of Firefox’s native tab bar. In this case, I guess the gBrowser.duplicateTab() function needs to be modified.

Comment 6

[kernp25]

(In reply to ariasuni from comment #5)

This bug is marked as WONTFIX and my time is limited, so I didn’t take the risk to waste it for something that wasn’t gonna be accepted.

However, if they are chances that it would be accepted, I’m willing to try to implement a new parameter with an index and active property that would allow to cleanly replicate the tab duplicating capabilities of Firefox’s native tab bar. In this case, I guess the gBrowser.duplicateTab() function needs to be modified.

If he attaches a patch on this bug, you will change the status of this bug?

Comment 7

[ariasuni]

Attachment: Bug 1560218 - browser.tabs.duplicate() can now specify index and active property

Comment 8

[Rob Wu [:robwu]]

If he attaches a patch on this bug, you will change the status of this bug?

I'll bring this up during the next triage or team meeting. The main point for considering to re-open is that it infeasible to use the browser.tabs.duplicate API to implement tab duplication to a specific position without messing up the UI.

The implementation of active hasn't had opposition yet, so I suggest to attach a patch that fixes the issue of active to bug 1376088. Then, if there is already an optional object parameter, it would be a smaller step to also support index.

Supporting index is not as straightforwards as adding extra parameters to the extension API, so I will also need to consult with other Firefox frontend folks (if the API is accepted by the other members of the extension team).

Comment 9

[ariasuni]

I made a patch that do both, but it’s easy to split.

Comment 10

[ariasuni]

Can someone review my patch? What should I do now?

Comment 11

[Shane Caraveo (:mixedpuppy)]

We will have to consider it before moving it to review. I've put it in our agenda to discuss this week. You request reviewers in phabricator.

Comment 12

[ariasuni]

OK, thank you. :)

Comment 13

[Rob Wu [:robwu]]

Re-opening bug because the feature request has been accepted.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8accdd6152c6
browser.tabs.duplicate() can now specify index and active property r=robwu

Comment 15

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/8accdd6152c6

Comment 16

[Richard Bloor]

Documentation changes have now been made:

• compatibility data changes submitted in PR #6164
• tabs.duplicate parameter details updated
• item added to the release notes

Please let me know if everything is in order or if any changes are needed. Thank you.

Comment 17

[Rob Wu [:robwu]]

Excellent as usual, thanks :)

Comment 18

[ariasuni]

I marked duplicateProperties as optional on MDN.