1640665 - Use `inBackground` parameter of duplicateTab in duplicateTabIn

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, enhancement)

Description

[Rob Wu [:robwu]]

When the patch for bug 1376088 lands, the duplicateTab method has a new inBackground option that defaults to true. Let's set that option to false (to set the duplicated tab as the active tab) instead of manually doing that at https://searchfox.org/mozilla-central/rev/501eb4718d73870892d28f31a99b46f4783efaa0/browser/base/content/browser.js#8542-8543

By doing so, the tab duplication logic of extension code and frontend code will behave identical, and also trigger the same kinds of events. Bugs and fixes for the one would automatically apply to the other.

There will be a slight difference in behavior: The inBackground option will only change the selected tab shortly after initializing the tab, whereas the current logic immediately switches to the newly duplicated tab, even before the restoration of its contents has started.

Comment 1

[agentcoulsonphil3]

Hi, I would like to contribute to this. Could you help me get started on this?

Comment 2

[Rob Wu [:robwu]]

To get started, you will need to have a copy of the source code, and install build dependencies. Since you only need to change frontend code (JavaScript) and no native code (e.g. C++, Rust, ...), you can get started more quickly by using artifact builds.

To get started, see https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html
Documentation on artifact builds is at https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Artifact_builds

While you are waiting for the source code to be cloned, it would be good to get familiar with the code that you need to change. In my original report, I linked to the relevant code on Searchfox. If you prefer live debugging over reading code, then you can use the Browser Toolbox, open browser.js and set a breakpoint at the relevant line, and then interact with Firefox to trigger that breakpoint.

To trigger this code, right-click on a browser tab, and choose "Duplicate Tab". The first comment describes how the code should be changed (and still work).

Good luck, and feel free to ask more questions if needed.

Comment 3

[agentcoulsonphil3]

I will get started on this.

Comment 4

[Harsh]

Hi Rob,
Is this fixed yet? If not, maybe I could take this up.

Comment 5

[Rob Wu [:robwu]]

(In reply to agentcoulsonphil3 from comment #3)

I will get started on this.

Are you still working on this? If so, please let us know where you are at.
If we don't get a reply by the end of the week, consider the bug to be up for grabs.

Comment 6

[Harsh]

From what I understand from your link, the line gBrowser.selectedTab = newTab; manually switches our duplicated tab to active. I believe this line needs to be removed and the parameter of inBackground needs to passed as false in the SessionStore.duplicateTab() in the preceding line?

Comment 7

[Rob Wu [:robwu]]

(In reply to Harsh from comment #6)

From what I understand from your link, the line gBrowser.selectedTab = newTab; manually switches our duplicated tab to active. I believe this line needs to be removed and the parameter of inBackground needs to passed as false in the SessionStore.duplicateTab() in the preceding line?

Yes, that's correct. Patches are welcome :)

Comment 8

[Harsh]

Attachment: Bug 1640665 - Use `inBackground` parameter of duplicateTab in duplicateTabIn. r=robwu

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d9e15f6cc350
Use `inBackground` parameter of duplicateTab in duplicateTabIn. r=robwu

Comment 10

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/d9e15f6cc350