1570845 - Bookmarklet passes parameter to external program, but in background mode

Status: VERIFIED FIXED

(External Software Affecting Firefox :: Other, defect, P1)

Description

[therube]

Bookmarklet passes parameter to external program, but in background mode...

No idea where this belongs...
Best I can do is describe what has changed.

I use a bookmarklet that passes a highlighted string (from a browser window) to an external program, Everything (Search).

In the past, you run the bookmarklet, the string is gathered & passed, & Everything is brought to focus with that string passed as its' search parameter.

Currently, while the string is still passed, most often, Everything opens "in the background", its' Windows taskbar icon blinking.

Difference is in focus or not.
Prior behavior, Everything always taking focus was/is desired.

Current behavior, you have to manually click the Everything (taskbar) icon to bring it to focus, before being able to see the search results it returned.

Works: (through) FF 70.0a1, 20190729213839 (& every Mozilla version prior to that).

Fails (behavior change): Builds after that.

Bookmarklet:

javascript:Qr=document.getSelection();if(Qr)location.href='es:'+(Qr);void(0)

Everything:

https://www.voidtools.com/
https://www.voidtools.com/downloads/
(ZIP is fine. I have always used x86 version. FF x64, Win7.)
In Everything | Options, you need to enable "URL Protocol", Tools | Options -> General --> URL protocol.
Forum thread, http://www.voidtools.com/forum/viewtopic.php?f=2&t=2423.
(Older thread, https://www.voidtools.com/forum/viewtopic.php?f=10&t=739.)

Comment 1

[therube]

Works: (through) FF 70.0a1, 20190729213839 (& every Mozilla version prior to that).

That's actually an incorrect date (too new).
At the time, it looked to be OK, but further testing show it not to be.
Then I checked some other builds, day before... but they too failed.

Pulled a build from 7-12-2019, & that is fine.

I'll get a better date once I figure out a better way to get it ;-).

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

Yeah, this kind of functionality is hard to find a component for.

That said, the code that directly launches external protocols/application handlers (https://searchfox.org/mozilla-central/source/uriloader/exthandler/win/nsMIMEInfoWin.cpp) hasn't had any meaningful changes in years, so I would expect the change in behavior you're seeing is due to changes in Windows or the application in question. Windows itself is responsible for focus management and owns the protocol-handler mapping which seems to self-describe from the Windows registry (https://searchfox.org/mozilla-central/source/uriloader/exthandler/win/nsOSHelperAppService.cpp#72) so it might be worth looking into whether you can add some more registry keys to force a specific behavior.

That said, it may also be more a problem of app coordination. Frequently in cases where an executable is invoked to open a document/URL and the executable is already open, the new app instance needs to communication with the pre-existing app instance to tell it to take an action, which then includes requesting that the app foregrounds itself. The operating system may inhibit this request to foreground itself and instead take the action you're seeing, making the icon blink to let you know that the application wants your attention but has been prevented from forcing itself to the foreground. (This is how Firefox does things, for example.)

I'm moving this to Core :: Networking, but again, I think the problem is likely outside of Firefox so that component's triagers may close the bug given that the problem seems unrelated to Firefox.

Comment 3

[therube]

IMHO, this is certainly FF.

FF 70a1

Works: 20190723155748
Broken: 20190724095428

.
Firefox Setup 69.0b9.exe is also broken. (That was the latest available at the time when I downloaded to check Beta.)
(FF 69, nightly, at least up to the time to switchover to FF 70, nightly, was OK.
All earlier "Mozilla" were fine.)

I'll note that while the bookmarklet worked in earlier Quantum, it only became feasible - for me, once this landed:
Bug 1478037 Allow bookmarklets to run even when the CSP on the page would normally block javascript: execution

So maybe it's something on the security end, a "clearclick"-like "enhancement"?
(There are a couple security bugs during that period that are not public yet.)

Comment 4

[therube]

app_name: firefox
build_date: 2019-07-24 00:00:01.069000
build_file: E:\Users\RUBEN.mozilla\mozregression\persist\e7d500f650cc-shippable--autoland--target.zip
build_type: inbound
build_url: https://queue.taskcluster.net/v1/task/QHkTl_RbRw6fxDFq4k8KOg/runs/0/artifacts/public%2Fbuild%2Ftarget.zip
changeset: e7d500f650ccfe119d46a36392fa641b47c142a2
pushlog_url: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=d4e621b5c51bbfb6d27cd9e307360821c447a06b&tochange=e7d500f650ccfe119d46a36392fa641b47c142a2
repo_name: autoland
repo_url: https://hg.mozilla.org/integration/autoland
task_id: QHkTl_RbRw6fxDFq4k8KOg

Looks like a good bet, Bug 1567614 "Entry Point Not Found" message box when opening Skype for Business URL?

s/enhancement/mitigation/

Comment 5

[Andrew Sutherland [:asuth] (he/him)]

Excellent detective work on the bisection! Marking the regression and moving to the regressing bug's component (and leaving the needinfo).

Comment 6

[(No longer employed by Mozilla) Aaron Klotz]

Thanks for the report. This is going to need some trial-and-error to see if I can coax the right behaviour out of everything.

(Note to RelMan: This does not block the uplift of bug 1567614 to 68release/68ESR)

Comment 7

[therube]

I "tested" [man, profile handling is so confusing now] on a different Win7 system & a Win10 (1809) system, & it appears that maybe ? only Win7 is affected by this?
Win10 looks to work with both the 0723 & 0724 (FF 70, Nightly) builds.

Comment 8

[therube]

Not sure what happened, but this started working again - at least by last nights Nightly.

Comment 9

[(No longer employed by Mozilla) Aaron Klotz]

We'll get to this, but we have higher priorities at the moment.

Comment 10

[(No longer employed by Mozilla) Aaron Klotz]

Over to Toshihito. Lets call CoAllowSetForegroundWindow on the interface right before we call IShellDispatch2::ShellExecute. We should MOZ_ASSERT if the call fails, but we should still allow the function to proceed in release builds since a CoAllowSetForegroundWindow failure is non-fatal.

Comment 11

[Toshihito Kikuchi [:toshi]]

Confirmed the repro and adding CoAllowSetForegroundWindow solved this issue. Preparing a patch.

Comment 12

[Toshihito Kikuchi [:toshi]]

Attachment: Bug 1570845 - Call CoAllowSetForegroundWindow before ShellExecute to launch an application in the foreground. r=aklotz

Comment 13

[(No longer employed by Mozilla) Aaron Klotz]

Once this patch merges to mozilla-central, please request uplift to beta and ESR68.

Comment 14

[(No longer employed by Mozilla) Aaron Klotz]

I have pushed this patch, but it is queued up until autoland reopens.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e3a4af3c20e2
Call CoAllowSetForegroundWindow before ShellExecute to launch an application in the foreground. r=aklotz

Comment 16

[Cosmin Sabou [:CosminS]]

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

Comment 17

[(No longer employed by Mozilla) Aaron Klotz]

Please request uplift to Beta and ESR68.

Comment 18

[Toshihito Kikuchi [:toshi]]

Comment on attachment 9088294 [details]
Bug 1570845 - Call CoAllowSetForegroundWindow before ShellExecute to launch an application in the foreground. r=aklotz

Beta/Release Uplift Approval Request

• User impact if declined: This patch is a fix for a recent regression on Firefox/Thunderbird for Windows. Without this fix, when a user clicks a link to launch an external application, such as the default mailer from Firefox or the default browser from Thunderbird, the application is launched in the background, thus she needs to take an extra action (clicking the taskbar icon) to bring it to the foreground to see it, which is not a good user experience.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The fix is to add a single function call to CoAllowSetForegroundWindow before launching an external application. Even though that function fails for some unknown reason, we continue to launch the application, keeping the current behavior.
• String changes made/needed: None

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: With this regression, an external application is launched in the background while it used to be launched in the foreground before. This behavior may require enterprise users to take an extra action to do some business operation, or break some automation.
• User impact if declined: This patch is a fix for a recent regression on Firefox/Thunderbird for Windows. Without this fix, when a user clicks a link to launch an external application, such as the default mailer from Firefox or the default browser from Thunderbird, the application is launched in the background, thus she needs to take an extra action (clicking the taskbar icon) to bring it to the foreground to see it, which is not a good user experience.
• Fix Landed on Version: 70.0a1
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The fix is to add a single function call to CoAllowSetForegroundWindow before launching an external application. Even though that function fails for some unknown reason, we continue to launch the application, keeping the current behavior.
• String or UUID changes made by this patch: None

Comment 20

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/releases/mozilla-esr68/rev/7797030cd4fa86394e71bb57b879df9464a7ddee on THUNDERBIRD_68_VERBRANCH for >= 68.1
Early uplift for TB 68.1 ESR. We can't ship it with the windows opening in the background.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9088294 [details]
Bug 1570845 - Call CoAllowSetForegroundWindow before ShellExecute to launch an application in the foreground. r=aklotz

Per comment 6, I don't think this needs to drive any RC respins, but let's keep this on the radar as a ride-along for that or a future dot release.

Comment 22

[Jorg K (CEST = GMT+2)]

FYI: Works nicely in the TB 68.1 where I included this (comment #20).

Comment 23

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 9088294 [details]
Bug 1570845 - Call CoAllowSetForegroundWindow before ShellExecute to launch an application in the foreground. r=aklotz

regression fix, approved for 68.2

Comment 24

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-esr68/rev/521a40f7a7cb

Comment 25

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9088294 [details]
Bug 1570845 - Call CoAllowSetForegroundWindow before ShellExecute to launch an application in the foreground. r=aklotz

Fixes a new regression in Fx69 causing links to be opened in a background window. Approved for 69.0.1.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/20efc009d67e

Comment 27

[Ryan VanderMeulen [:RyanVM]]

Added to the 69.0.1 relnotes:

Fixed external programs launching in the background when clicking a link from inside Firefox to launch them

Comment 28

[Emil Ghitta, QA [:emilghitta]]

I have managed to reproduce this issue using Firefox 70.0a1 (BuildId:20190801214227)

This issue is verified fixed using Firefox 70.0b7 (BuildId:20190916074538), Firefox 69.0.1 (BuildId:20190917135527) and Firefox 68.2.0esr (BuildId:20190917230445) on Windows 10 64bit