Hi!

I like the idea of opening the target URL in the user's default browser automatically, but in practice it is not clear to me how to do so while providing a good user experience. For example, in my testing, Python's webbrowser tried to open Chrome (which does not exist on my workstation) and then opened Firefox (which is not the default browser on my macOS workstation):
[…]
So unless there is a reliable way for Python to determine the user's default browser correctly across macOS, Windows, and Linux operating systems, unfortunately I don't think we will be able to achieve the intended objective.

I think the problem you describe is not related with the contribution itself, but with the webbrowser's standard library implementation. Did you set the BROWSER environment variable correctly? According to the module documentation, this could help you fix the very specific problem you encounter.

This PR is bringing more benefits than problems in my opinion, you happen to be unlucky in your setup. It works for me, and I guess it will work for a lot of other people too. If you do not manage to make the webbrowser module work as intended, maybe you should create an issue on the Python tracker — but this is beyond the contribution we are discussing.

Thanks for the review, both comments have been addressed in the updated version.

I think the problem you describe is not related with the contribution itself, but with the webbrowser's standard library implementation.

I understand that the problem is related to how webbrowser behaves. But since the contribution (in its current form) relies upon that module, the contribution thereby inherits that problem. (I sense a new white paper entitled... Transitive Properties in Open Source Software. 😆)

Did you set the BROWSER environment variable correctly? According to the module documentation, this could help you fix the very specific problem you encounter.

I am aware of that workaround. I did not use it because (1) I have never had to do so previously, and (2) that would only solve it for me and not for other Pelican users. Ergo, I don’t consider that to be a viable solution. Providing a semi-broken feature to end users and then telling them that they must set BROWSER to fix it… is not a reasonable position to take.

This PR is bringing more benefits than problems in my opinion

As I said before, I like the intention and am open to an implementation that works well. In its current form, this implementation does not deliver a good user experience for a significant proportion of users.

you happen to be unlucky in your setup. It works for me, and I guess it will work for a lot of other people too.

Unlucky? That is an… interesting perspective. If the same thing happens to all macOS users, then it’s not really a question of luck, is it? (That is a rhetorical question.) And what about folks on Windows? Are we casting them aside as well?

I understand the precarious temptation to hand-wave and dismiss with, “It works on my machine, so there must be a problem with your set-up.“ However, I suggest resisting that temptation. For starters, such assertions are usually wrong. More importantly, it’s easy to say that “It works for me” when you don’t have to respond to a swarm of new issues from folks when it doesn’t work well for them. I, on the other hand, do have to deal with those support requests, so I don’t have the luxury of getting by with “works for me”.

“It works on my machine ¯\_(ツ)_/¯” and open source are inherently incompatible.

Now that we’ve adjusted our perspective, let’s try to find a solution that also works well on other folks’ workstations, shall we?

Since Python’s webbrowser module does not seem capable of opening default browsers in a reliable manner, one approach for environments in which BROWSER is not set would be to determine the user’s operating system and invoke the appropriate OS-specific command. For example, on macOS, the following addition to the livereload task actually opens the default browser correctly:

c.run("open http://{host}:{port}".format(**CONFIG))

For Windows, perhaps the following will work:

c.run('explorer "http://{host}:{port}"'.format(**CONFIG))

For Linux, maybe xdg-open is appropriate:

c.run('xdg-open "http://{host}:{port}"'.format(**CONFIG))

While that would require a few extra lines of code in tasks.py, at least then it would provide users with a better out-of-the-box experience. Would you be willing to modify your PR to use that flow? That is:

1. Check if $BROWSER is set. If so, open that via webbrowser.
2. Else, use sys.platform to get the current OS, and…
3. … use the appropriate OS-specific invocation to open the default browser.

Hello Justin, thanks for your answer and your time.

I agree that the it works on my machine is an incorrect approach as it can quickly become a maintenance hell as more users report issues. This is why we should spend more time to make sure that the implementation of this proposal works for everyone.

However, I also think that one should not reinvent the wheel but use the tools at disposal when possible, and enhance them if they do not behave correctly. The webbrowser module being part of the stdlib, I think we should really make the effort of using it instead of creating a poor clone that will have gone through less more eyes and testing. As the module is 713 lines long, I doubt we could be any smarter with a few lines inside of the tasks.py file.

I downloaded the Python 3.8.3 source code and had a look at the Lib/webbrowser.py file.

Inside the OSX specific part, the "open" command you suggested seems to be used if the browser is set to "default". In fact, the command is "open location" as seen in this example:

script = 'open location "%s"' % url.replace('"', '%22') # opens in default browser

The register order for OSX directly specifies that the 'default' web browser should be the first one to be tested before Chrome, so the above call should occur on your computer before the Chrome call:

def register_standard_browsers():
    global _tryorder
    _tryorder = []

    if sys.platform == 'darwin':
        register("MacOSX", None, MacOSXOSAScript('default'))
        register("chrome", None, MacOSXOSAScript('chrome'))
        register("firefox", None, MacOSXOSAScript('firefox'))
        register("safari", None, MacOSXOSAScript('safari'))
        # OS X can use below Unix support (but we prefer using the OS X
        # specific stuff)

Maybe the version of Python you are using is not containing the same implementation as this 3.8.3 release. From my understanding of the webbrowser source code, the "open location" call should first occur on your machine before starting Chrome, failing, and then going to Firefox as you described.

Any chance "open location" does not work while "open" does work on your OSX system?

Please find attached webbrowser.py.txt for your convenience, renamed to bypass Github .py exclusion rule.

For what it's worth, webbrowser.open("http://localhost:8081") works as expected for me on Windows. I have Firefox selected as my default browser (and Chrome installed as well), and it opens it in Firefox. (I've assumed, but never confirmed, that it taps into what has been selected as the default system browser/opener for URL's).

It does however need the protocol (http://) at the front, otherwise it doesn't know what to do with the link.

So at least on this one, Windows looks like it's the simple solution!

@MinchinWeb: Many thanks for testing on Windows. Much appreciated.

@MicroJoe: My apologies for not coming back to this endeavor sooner. I tested this again just now and found that, for whatever reason, it now functions as intended and opens my default browser. Why that was not the case before… remains a mystery.

I think there are just a few changes I'd like to see before merging this PR:

1. As mentioned above, perhaps it would be best to add the http:// protocol scheme prefix?
2. Some folks may not prefer this behavior. Would it make sense to add a OPEN_BROWSER_ON_SERVE setting at the top of the file (defaulting to True), so folks have an easy way to flip this behavior on/off?
3. Let's change the comment to: # Open site in default web browser

What do you think?

@justinmayer thanks for the feedback, commit updated with taking into account 1. 2. and 3. remarks.

@MicroJoe: Thank you for responding so quickly. In the interest of behavior consistency, do you think it makes sense to add this same stanza to the serve task?

if OPEN_BROWSER_ON_SERVE:
    # Open site in default browser
    import webbrowser
    webbrowser.open("http://{host}:{port}".format(**CONFIG))

@justinmayer great idea, added a new commit on top to do that as well.

Nice work, Romain. Many thanks for your contribution, patience, and willingness to work together to come up with a robust solution. I like your philosophy about fixing things upstream when the opportunity presents itself. It's been a pleasure. ✨