Add support for kioclient and x-www-browser

[ssokolow]

An implementation of the changes discussed in issue #16

I tried to preserve the existing code structure as much as possible in the face of needing to check std::env for whether kioclient or gvfs-open and gnome-open should take precedence.

I did some quick checks on my end by unsetting BROWSER, and/or commenting out the other entries and everything appears to be working.

For a more robust means of integration testing, one approach I've used before is to mock the commands as follows:

1. Create folders containing mock commands that log success to file (eg. shell scripts with lines like echo "SUCCESS: $0" > result.log and equivalent batch files)
2. Run cargo test --no-run
3. Run the ./target/debug/webbrowser-* binary manually, with PATH set to contain only the folder full of whichever set of mocks that particular test requires.
4. Tests can now be written which reliably detect both failure due to missing commands and which specific command resulted in success.

[ssokolow]

Oops. Sorry about that failed Travis-CI run. I forgot to run cargo fmt.

(I don't have it set up to run automatically because there are certain aspects of my coding style where rustfmt.toml isn't expressive enough and I can't apply rustfmt::skip until the compiler stops saying things like error: attributes are not yet allowed on if expressions... mostly relating to breaking really short expressions across far too many lines, hurting code readability.)

[ssokolow]

Hmm. It looks like adding kioclient exec and x-www-browser altered which browser is getting picked up under Travis-CI to produce something that sits, waiting for input, until the build times out.

I can think of various ways to work around that (eg. setting BROWSER to something that will exit on its own, having the test prepend a directory full of mocks onto PATH, etc.) but I'll need to know what approach you'd prefer to take.

[amodm]

Consistency of behaviour (currently non-blocking) is important, so if some of those commands invoke a blocking behaviour, can you figure out how to make them comply with the existing behaviour? Manually running the commands on your terminal may indicate if they are blocking in nature or not.

Modifying tests to mock or skip wouldn't be productive because it may hide behavioural inconsistencies of this kind.

[ssokolow]

Ahh. It's probably the x-www-browser then. That's a symbolic link to the browser binary and the initial launch of a browser is blocking. They only become non-blocking when operating in remote/open-URL-in-existing-process mode.

[ssokolow]

...and, looking at your BROWSER code, it looks like you're already getting blocking behaviour... you just don't have your tests set anything for BROWSER to trigger it.

(Setting BROWSER=x-www-browser on Travis-CI should trigger the same test timeout in your existing code... or, for that matter, setting BROWSER=firefox or BROWSER=chromium-browser in a desktop where there isn't already a session of that browser running.)

EDIT: Yeah. If I invoke BROWSER=chromium-browser cargo test when only my usual Firefox session is running, each test opens up a new Chromium process that blocks the test until I manually close it.

[ssokolow]

...and looking at xdg-open's code, it looks like it doesn't try to ensure non-blocking operation either. When it's using BROWSER or falling back to parsing the file associations itself, it just invokes the browser without any apparent attempt to background it.

(The other options are to invoke things like gvfs-open and, since those are compiled tools, it'll take me a bit longer to check what they do.)

[ssokolow]

Hmm. It looks like gvfs-open is using g_app_info_launch_default_for_uri, which has this to say in its docs:

The D-Bus–activated applications don't have to be started if your application terminates too soon after this function. To prevent this, use g_app_info_launch_default_for_uri_async() instead.

The _async() variant has this to say:

This version is useful if you are interested in receiving error information in the case where the application is sandboxed and the portal may present an application chooser dialog to the user.

This is also useful if you want to be sure that the D-Bus–activated applications are really started before termination and if you are interested in receiving error information from their activation.

In other words, gvfs-open seems to not provide the reliability guarantee you're after if running in a sandbox.

...and gnome-open uses the same g_app_info_launch_default_for_uri function, so it's also unreliable in sandboxed contexts.

[ssokolow]

While poking through xdg-open's source, I found that it also tries these other tools I wasn't aware of:

KDE:

• kde-open
• kfmclient openURL

MATE:

• mate-open

Xfce:

• exo-open

I don't want to go poking through every tool out there, but the source to kioclient appears to show it waiting until the finished() or error() signals, which would indicate that it's going to block if there wasn't already an instance of the browser open to dispatch a request to and then quit early.

I believe mate-open is a forked and renamed copy of gnome-open, so it'd probably behave the same way as gvfs-open and gnome-open do.

[ssokolow]

To make a long story short:

1. I didn't introduce blocking into webbrowser-rs, I just exposed some that your tests weren't triggering.
2. Your rationale in Please support non-GTK/GNOME-family options in the non-xdg-utils fallbacks #16 for not wanting #[cfg(all(unix, not(target_os = "macos")))] is a futile effort because the *-open and the "Browser remotely invoking an existing instance of itself" layers of the stack both independently introduce more unreliability than the assumption that "If we're Unixy and we've got xdg-open, it'll work"... and that's not a Linux-specific thing either. (You can break file/url associations on Windows and browsers sometimes being flaky in their implementations of single-instance/remote-control functionality isn't platform-specific.)

As for your question, I can certainly make webbrowser-rs invoke browsers in a non-blocking way... but doing that while still preserving the ability to have Err() mean anything more than "Couldn't find a supported means of invoking a browser" is another story.

The best you can do is introduce a race condition by polling on Command::try_wait and then considering it a success if the process is still running after X number of seconds. (I have personal experience with it being a very visible race condition because I have machines where, when the nightly backups blow away the disk cache, it can take up to half a minute to pull things back in sufficient to xdg-open a tab in an existing browser session.)

Fundamentally, there's no common standard for applications to report "I successfully finished starting up" to their parents that browsers implement and there's also no standard way to make it predictable whether invoking a browser will block. Firefox and Chromium, for example, will both exit quickly if they decide to open the URL in a pre-existing browser session or block until the browser is closed if not.

[amodm]

Thanks for bringing out potentially blocking behaviour of xdg-open in certain circumstances. It's probably tolerable for xdg-open, as it's a user tool. But it's not ok for a library, so I'll treat this as an issue for webbrowser-rs, given that the choice to use xdg-open is being made by the library, not the user.

I also agree with your assessment of the potential blocking behaviour of BROWSER env var, but given that it's not the library's decision, I feel it's ok. To me, it's in the same boat as other programming choices, e.g. if a programmer makes a blocking call inside the render path of a rendering library, leading to janky UI, it's not the rendering library's fault.

For the time being, let's focus on bringing this PR to a closure. We can discuss the philosophy piece on #16

[ssokolow]

My point was to illustrate that blocking vs. non-blocking has to be solved across all of them. It was a lurking issue all along.

• Whether BROWSER blocks is up to the browser
• Whether xdg-open blocks is up to the browser
• Whether x-www-browser blocks is up to the browser

The documentation isn't 100% clear on what gvfs-open and gnome-open do, so I'd have to set up some tests to determine that.

As for browsers:

• Firefox blocks unless there's already a window open (and, even then, it may block on the Profile Manager dialog if it's uncertain which one you want to open the URL in)
• Chrome/Chromium blocks unless there's already a window open
• Arora blocks unless there's already a window open
• Midori blocks unless there's already a window open
• Luakit blocks unless there's already a window open
• Konqueror always blocks
• Terminal-based browsers like lynx and links2 have no remote functionality and block unless explicitly asked otherwise (in those two cases, with the -dump flag)

Those are all the browsers I happen to have installed at the moment.

In fact, the proper way to think about it is "Like most processes (gvim excepted), it's default for browsers to block, but a subset of browsers implement "open in a pre-existing browser window" functionality that causes the subprocess you launched to exit early... that exception to the rule just happens to occur commonly.

[amodm]

All good points. I'm going to noodle over this issue until I've developed a consistent thought process on the blocking part, as well as fix the tests. Until then, the PR remains open.

[ssokolow]

Understood. Poke me when you're ready.

[amodm]

I'll recommend you modify this to something like

.or_else(|e| -> Result<ExitStatus> {
                if let Ok(child) = Command::new("x-www-browser").arg(url).spawn() {
                    return Ok(ExitStatusExt::from_raw(0));
                }
                Err(e)
            })

to allow for the tests to pass for now. It'll allow me to accept the PR. I'm realising that a larger fix for making the behaviour consistent, and reflecting those in the tests, will take me some time. So let's focus on closing this out for now.

[ssokolow]

OK. I'm just winding down for bed but I should have a commit for you tomorrow.

[ssokolow]

I pushed the change but it's still blocking on the browser.

I have a headache and want to get to bed early, so I'll have to look into it further tomorrow, but I know it's not kioclient exec because, if I temporarily set my KDE handler for http:// and https:// URLs to something that's not already running, kioclient exec will launch the expected browser but will not block.

That leaves two possibilities I can think of off the top of my head:

1. The code snippet you proposed is insufficient to keep it from waiting on a spawned child. I'll have to experiment tomorrow to rule this out.
2. Travis-CI's configuration has changed since you last ran a test and, if you were to ask it to re-test your current HEAD, that would timeout and fail too.

I'm fairly certain it's not a difference between how my and Travis-CI's copies of commands behave, because we're both running systems based on Ubuntu Xenial (16.04 LTS).

I can see three ways option 2 might come about:

1. Travis-CI might have started waiting for background processes to exit before considering a test run finished.
2. Travis-CI might have changed the default browser.
3. Travis-CI might have upgraded to a version of the browser which behaves differently under your test conditions.

[amodm]

I ran a new build on master HEAD just now, and it passed.

[amodm]

A few updates. Firstly, xdg-open seems to open a text browser, which would block obviously. No clue yet why the tests still seem to pass on master. Secondly, when I take your branch and push it on a branch in my repo, the tests pass !?

I'll be merging this PR, and figure it out from there. Regarding the issues with the tests, I feel your contribution has only brought out the root problem, than caused it. So the onus is on me to figure it out.

Thanks for the contribution, as well as other insights.

[ssokolow]

No problem and good to know. I was starting to worry that Travis-CI just wanted to try to drive me crazy.

[ssokolow]

Oh, I just noticed that I forgot to update the README to acknowledge kioclient exec and x-www-browser. Are you going to do that or should I?

[amodm]

I'll do that, after figuring out why the tests produce different outcomes in different scenarios, and fixing them.

[ssokolow]

Speaking of fixing tests, what I'd suggest as the ideal solution for the end-to-end testing you seem to want is to add something like Hyper as a dev dependency and have the tests spin up an HTTP server which will service one request and then exit.

Then, you can even do a true verification that the URLs fed to the function are what are getting requested by the browsers.

[amodm]

Makes sense. Thanks.