1667645 - Ctrl+Click from home page focuses on new tab

Status: VERIFIED FIXED

(Firefox :: New Tab Page, defect, P1)

Description

[Sid Vishnoi :sidvishnoi]

Attachment: Ctrl+Click vs Right-click new-tab behavior

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

Steps to reproduce:

1. Ctrl+Click on "Top sites" from the default home page.

Actual results:

The newly opened tab got focused.
"When you open a link in a new tab, switch to it immediately" is disabled in about:preferences

Right click -> open in new tab did not focus on new tab.

mozregression directed me to bug 1599368.

Expected results:

New tab should not have gained focus on Ctrl+Click.

Comment 1

[Sid Vishnoi :sidvishnoi]

Attachment corrupt somehow. Uploaded demo to imgur: https://i.imgur.com/V7euzON.mp4

Comment 2

[Scott [:thecount] Downe]

@gijs Looks like we broke something in bug 1599368.

I've done some investigating, I think I know what's going on, and I think we have some options.

When we went from calling openLinkIn to openTrustedLinkIn we ended up hitting a code path that sets params.fromChrome to true: https://searchfox.org/mozilla-central/source/browser/base/content/utilityOverlay.js#349

It then calls openLinkIn. Later on inside openLinkIn we assume fromChrome means do not open in background: https://searchfox.org/mozilla-central/source/browser/base/content/utilityOverlay.js#585

I think we can either:

1. Refactor openTrustedLinkIn to only use fromChrome when it makes sense. Seems like it's an assumption that either, a trusted link is a chrome link, or an assumption that a chrome link should always be focused when opened.
2. Go back to calling openLinkIn and manually send triggeringPrincipal: Services.scriptSecurityManager.getSystemPrincipal(), but I don't think this is ideal, because what if what we consider to be trusted changes in the future.

Comment 3

[:Gijs (he/him)]

(In reply to Scott [:thecount] Downe from comment #2)

@gijs Looks like we broke something in bug 1599368.

I've done some investigating, I think I know what's going on, and I think we have some options.

When we went from calling openLinkIn to openTrustedLinkIn we ended up hitting a code path that sets params.fromChrome to true: https://searchfox.org/mozilla-central/source/browser/base/content/utilityOverlay.js#349

Bah, sorry for missing that.

It then calls openLinkIn. Later on inside openLinkIn we assume fromChrome means do not open in background: https://searchfox.org/mozilla-central/source/browser/base/content/utilityOverlay.js#585

I think we can either:

1. Refactor openTrustedLinkIn to only use fromChrome when it makes sense. Seems like it's an assumption that either, a trusted link is a chrome link, or an assumption that a chrome link should always be focused when opened.

Yeah, it's the latter, but there are dozens of callsites.

We could probably change openUILinkIn to only set fromChrome if it's not already explicitly set to false, and then explicitly pass false from the callsite in the new tab code?

2. Go back to calling openLinkIn and manually send triggeringPrincipal: Services.scriptSecurityManager.getSystemPrincipal(), but I don't think this is ideal, because what if what we consider to be trusted changes in the future.

I don't think that the concept of "trusted" is likely to change, so I don't think this is that bad an option. Either of these two works for me.

Whichever we pick, we should add a browser mochitest that checks that these links open correctly, so we don't break it in future. https://searchfox.org/mozilla-central/rev/f27594d62e7f1d57626889255ce6a3071d67209f/toolkit/content/tests/browser/browser_license_links.js might be a decent template for how to do something like that.

Comment 4

[Scott [:thecount] Downe]

Attachment: Bug 1667645 - fixing ctrl+click newtab link clicks focus issue

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d175746088d7
fixing ctrl+click newtab link clicks focus issue r=Gijs

Comment 6

[Cristian Brindusan [:cbrindusan]]

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

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1599368

Comment 8

[Robert Martin [:romartin], Ecosystem QA]

I have verified that the topsite opened using "Ctrl+Click" ("Cmd+click" for macOS) is NOT focused using Firefox Nightly 83.0a1 (Build ID: 20201002092536) on Windows 10 x64, macOS 10.15.6, and Ubuntu Linux 20.04 x64.