webbrowser.open incomplete on Windows

[dfa80d8a-6ad1-4a54-93ff-4123862b80a1]

BPO	8232
Nosy	@pfmoore, @tjguk, @bitdancer, @zware, @eryksun, @zooba, @PedanticHacker, @Mariatta
Dependencies	bpo-43538: [Windows] support args and cwd in os.startfile()
Files	webbrowserdebug.py webbrowserfix.patch webbrowserfix2.patch webbrowserfix3.patch webbrowserfix3.patch webbrowserfix4.patch webbrowserfix5.patch webbrowserfix6.patch 25005_1.patch 8232_1.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields: ```python assignee = None closed_at = None created_at = labels = ['type-bug', 'library', '3.10', 'OS-windows'] title = 'webbrowser.open incomplete on Windows' updated_at = user = 'https://bugs.python.org/joncwchao' ``` bugs.python.org fields: ```python activity = actor = 'eryksun' assignee = 'none' closed = False closed_date = None closer = None components = ['Library (Lib)', 'Windows'] creation = creator = 'joncwchao' dependencies = ['43538'] files = ['36110', '36265', '39525', '39533', '39534', '39590', '39626', '39650', '40384', '40393'] hgrepos = [] issue_num = 8232 keywords = ['patch'] message_count = 55.0 messages = ['101725', '101728', '101732', '126265', '224057', '224060', '224796', '241047', '241057', '244219', '244298', '244299', '244606', '244769', '244784', '244843', '244851', '244959', '244968', '244986', '245364', '245372', '245374', '245377', '245382', '245385', '245386', '245387', '245388', '245389', '245390', '245391', '245392', '245393', '245394', '245396', '245398', '245399', '245400', '245401', '245404', '245405', '250067', '250072', '250074', '250112', '250114', '250115', '250117', '250121', '250135', '261704', '290930', '290935', '388993'] nosy_count = 13.0 nosy_names = ['paul.moore', 'tim.golden', 'Arfrever', 'r.david.murray', 'joncwchao', 'devplayer', 'python-dev', 'zach.ware', 'eryksun', 'steve.dower', 'jbmilam', 'PedanticHacker', 'Mariatta'] pr_nums = [] priority = 'normal' resolution = None stage = 'patch review' status = 'open' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue8232' versions = ['Python 3.10'] ```

[dfa80d8a-6ad1-4a54-93ff-4123862b80a1]

webbrowser.open(), webbrowser.open_new(), and webbrowser.open_new_tab() all do the exact same thing, regardless of the flags that I set. In Firefox, open('www.google.com', new=0), open_new('www.google.com'), and open_new_tab('www.google.com') all open either three new www.google.com tabs (if "Open new windows in a new tab instead" is selected in FF options) or three new www.google.com windows (if "Open new windows in a new tab instead" is not selected in FF options). In Internet Explorer, three new www.google.com tabs are created.

The issue exhibits itself the same way whether or not I have the browser open before running the script.

Environment was a Windows Vista 32-bit machine, running Python 3.1.2.

Example script reads: import webbrowser import time ff = webbrowser.get('firefox') ff.open('www.google.com', new=0) time.sleep(3) ff.open_new('www.google.com') time.sleep(3) ff.open_new_tab('www.google.com')

[briancurtin]

On Windows, the WindowsDefault class gets used and it doesn't make use of anything other than the URL (no 'new' or 'autoraise'). All it does is pass the URL onto os.startfile.

It should make a better attempt at running the URL. Patches welcome, or I'll try to come up with one shortly.

[briancurtin]

Minor correction: BackgroundBrowser gets used in this case, but it still lacks in the same area and causes what you are seeing.

[589fca95-9318-4f30-bbe7-10dec0e0a7bc]

Don't forget to check if the MS Internet Explorer's advanced option to open new URLS in a seperate windows effects this. Users can have this advanced setting set differently on different computers(or even accounts). Also different browser versions call that option by different names. And I think there may even be a way to turn off tabs altogether, so check that too.

[0e94f9d9-1b85-433e-8b98-9cb43fef9f36]

In order to fix the issue I added on to the WindowsDefault class so that it is the main browser class for windows platforms as opposed to being a default when no other browser is given. I gave the class an init where it specifies specific flags for firefox, chrome, and internet explorer (from what I could find there aren't really new window or new tab flags for internet explorer). If the flags for other browsers are known they should be easy to add to this section.

        def __init__(self,browser = "windows-default"):
            # Grab the different flags for the different browser types
            browser.lower()
            self.browsername = browser
            # If get() is used without arguments browser will be passed None
            if browser == "windows_default" or browser == None:
                self.cmd = "start"
            elif browser == 'iexplore' or browser == 'internet explorer':
                self.cmd = "start iexplore"
                self.newwindow = ""
                self.newtab = ""
            elif browser == "chrome":
                self.cmd = "start chrome.exe"
                self.newwindow = "-new-window"
                self.newtab = "-new-tab"
            elif browser == "firefox":
                self.cmd = "start firefox.exe"
                self.newwindow = "-new-window"
                self.newtab = "-new-tab"
            else:
                raise Error('The browser you entered (%s) is not currently supported on windows' % browser)

In the open method of the WindowsDefault class I changed how the browser is opened by building a command from the flags and the cmd for the specific browser and used subprocess,call.

            # Format the command for optional arguments and add the url
            if new == 1:
                self.cmd += " " + self.newwindow
            elif new == 2:
                self.cmd += " " + self.newtab
            self.cmd += " " + url

            subprocess.call(self.cmd,shell = True)

This allows the user to input different new arguments to open a new window or new tab like the documentation says they should be able to do. I added a little bit to the beginning of the get function so that it passes its argument to the WindowsDefault class and returns that object on Windows systems.

    # Let the windows default class handle different browsers on windows
    if sys.platform[:3] == "win":
        return WindowsDefault(using)

This adds some of the desired compatibility but does not completely address the module's issues. I did not see a way to open a web page in a currently open page on any of the browsers, just new windows and new tabs (when no flags are passed the browsers default to one of these two options). Also the _isexecutable function's attempt at windows compatibility is still not working because I was unsure of how to use just a string of a browser name like 'chrome' to determine if a file is on a system. This leaves _tryorder not properly containing the browsers on the system. This leaves the module's open, open_new and open_new_tab not properly working either just the WindowsDefault open method. Any feed back and direction from here is most welcome.

[0e94f9d9-1b85-433e-8b98-9cb43fef9f36]

How the _isexecutable function is set up now it would require a full path name in order to be able to tell if a specific browser is on the system. The area under platform support for windows checks for multiple browsers using this function but only passes it browser names and so it always returns false and does not add any browsers to _tryorder. I found a way to fix this using os.walk so that the simple strings of the browser names like "firefox.exe" is able to actually able to be found on the system. This method is rather slow though and the module wants to check for 8 browsers when imported.

[0e94f9d9-1b85-433e-8b98-9cb43fef9f36]

I got rid of the __init__ for the WindowsDefault class that I asked about earlier and changed it to match the sub-classing model that the Unix browsers use. This caused some changes in the get function too. Due to the _isexecutable still not completely working, the get function is hard coded for chrome, internet explorer and firefox for windows systems. This is my first attempt at making a patch file so if it is incorrect please bear with me.

[zooba]

Some hints about finding browsers on Windows.

When browsers are installed, they should register themselves in HKEY_LOCAL_MACHINE\SOFTWARE\Clients\StartMenuInternet so that users can change their default browser through the OS.

On 64-bit systems, this is always in the 64-bit registry, so to open it you need OpenKeyEx and the KEY_WOW64_64KEY flag.

Each subkey of the key represents one browser, and the key name is a moniker while the default value of each subkey is a user-friendly name.

Under each subkey is a shell\open\command key that has the path for the browser in the default value. As far as I can tell this must be the path and cannot contain command-line arguments, and it may optionally have quotes (to handle spaces in the path).

I'd expect browsers to provide command-line arguments for opening in an existing window or a new one, but they will differ between browsers. and will require individual research (though it looks like the attached patch has some of them).

[eryksun]

register themselves in HKEY_LOCAL_MACHINE\SOFTWARE\ Clients\StartMenuInternet so that users can change their default browser through the OS. On 64-bit systems, this is always in the 64-bit registry

According to "Registry Keys Affected by WOW64", the "Clients" key is redirected in Vista and shared in Windows 7.

https://msdn.microsoft.com/en-us/library/aa384253

    import winreg

    HKLM = winreg.HKEY_LOCAL_MACHINE
    subkey = r'Software\Clients\StartMenuInternet'
    read32 = winreg.KEY_READ | winreg.KEY_WOW64_32KEY
    read64 = winreg.KEY_READ | winreg.KEY_WOW64_64KEY

    key32 = winreg.OpenKey(HKLM, subkey, access=read32)
    key64 = winreg.OpenKey(HKLM, subkey, access=read64)
    # This should be true in Windows 7.
    assert winreg.QueryInfoKey(key32) == winreg.QueryInfoKey(key64)

I don't have a Vista installation to confirm that it's really redirected. If so, it's important to enumerate the WOW64 redirected key as well, since 32-bit browsers are still common on Windows.

[0e94f9d9-1b85-433e-8b98-9cb43fef9f36]

I kept the changes to the WindowsDefault.open() method and used and extended eryksun's code to build the browser list using the registry. Also I added support for a few more browsers. Some of the browsers I could not find ways to differentiate between opening a new window or new tab using command line flags. This also removed the hardcoding I had put in the get function.

[0e94f9d9-1b85-433e-8b98-9cb43fef9f36]

I went ahead and took the assert statement out and added support for vista using a union of sets for both the 32 bit and 64 bit locations.

[0e94f9d9-1b85-433e-8b98-9cb43fef9f36]

On second thought no type testing is required if sets are used because the union will take out duplicates anyways and so I removed the type testing and left in the set union code.

[0e94f9d9-1b85-433e-8b98-9cb43fef9f36]

Forgive me the excessive number of patch submissions as I am still getting my feet wet in contributing to Python. I'm posting another patch that is not functionally different from the last patch but should better adhere to the PEP-8 style guide.

Please let me know of any additional changes that need to be made or if a different functionality is preferred.

[zooba]

I do like this fix, and I'm sorry I didn't get to reviewing it before beta 1 was released - can we consider this something to fix for 3.5 or do we need to slip it until 3.6?

[bitdancer]

I haven't reviewed the patch, but if it only makes the existing API actually work for Windows, I think it would be fair game for 3.5. Larry would need to make the call, though.

[0e94f9d9-1b85-433e-8b98-9cb43fef9f36]

Here's a patch addressing all of the comments in the review. Changing the browsers from a set to a list though resulted in duplicates in the _tryorder list that were not present before because the set had filtered the duplicates before the partial string comparisons. The _browsers dictionary did not contain the duplicates so I don't think this will have any functional changes.

[larryhastings]

Go ahead for beta 3.

[0e94f9d9-1b85-433e-8b98-9cb43fef9f36]

Moved the 64 bit browser list to its own loop and switched to browsers.append rather than +=.

[zooba]

That looks good to me, I'll get it merged in when I'm at my desk.

[1762cc99-3127-4a62-9baf-30c3d0f51ef7]

New changeset b75c600e1614 by Steve Dower in branch '3.5': Issue bpo-8232: webbrowser support incomplete on Windows. Patch by Brandon Milam https://hg.python.org/cpython/rev/b75c600e1614

New changeset 63b6e150b635 by Steve Dower in branch 'default': Issue bpo-8232: webbrowser support incomplete on Windows. Patch by Brandon Milam https://hg.python.org/cpython/rev/63b6e150b635

[80036ac5-bb84-4d39-8416-02cd8e51707d]

Official spelling of name of one of these browsers is "Firefox", not "FireFox". I suggest to rename WinFireFox class accordingly.

[064b147d-1909-4665-ac07-705095c837ce]

I agree with Arfrever Frehtes Taifersar Arahesis (oh my god, you have the longest name, dude!) to rename the class WinFireFox() to WinFirefox().

Okay, so since the branch '3.5' already got its patch webbrowserfix6 applied -- are you, Brandon Milam, willing to make a new webbrowserfix7 patch with the added WinFireFox() -> WinFirefox() classname change and then apply this new patch to the '3.4' branch as well as to the '3.5' branch?

[zooba]

If it existed in 3.4 then we can only alias it now and not fix it. 3.5 and 3.6 can have the fix.

[80036ac5-bb84-4d39-8416-02cd8e51707d]

Steve Dower: Maybe thou hast already forgotten, but WinFireFox class was added by thee (only in 3.5 and 3.6) just 7 days ago :) .

[zooba]

That's what I thought, but I wasn't 100% sure it wasn't moved/rewritten in the patch and was on my phone so I didn't check :)

[1762cc99-3127-4a62-9baf-30c3d0f51ef7]

New changeset 0d54a78861cf by Steve Dower in branch '3.5': Issue bpo-8232: Renamed WinFireFox to WinFirefox https://hg.python.org/cpython/rev/0d54a78861cf

New changeset 8667c26e2bec by Steve Dower in branch 'default': Issue bpo-8232: Renamed WinFireFox to WinFirefox https://hg.python.org/cpython/rev/8667c26e2bec

[zooba]

I'll close this as fixed, but feel free to speak up if you spot anything else that needs fixing.

[064b147d-1909-4665-ac07-705095c837ce]

Now that this bug is completely fixed, can you backport this to the '3.4' branch, so that we'll be able to use webbrowser. get() in Python 3.4.4 when it becomes available?

[zooba]

The 3.4 RM is already nosied - what say you, sir?

[larryhastings]

Sorry, but I think this is more accurately described as a "new feature" than a "bugfix". Please don't backport this to 3.4.

[064b147d-1909-4665-ac07-705095c837ce]

No, Larry, this is not a new feature. The feature, as it stands, is broken in Python 3.4, so we need to fix it.

[larryhastings]

This is not a bugfix to existing code. This is new code to implement a missing feature.

[064b147d-1909-4665-ac07-705095c837ce]

Sure, let's have a broken feature in Python 3.4, who cares.

[larryhastings]

Rules like this are there for a reason. People rely on Python being consistent. We've added harmless new features to point releases in the past and broken people's code. So, we don't do it anymore.

It's not because we don't care, it's because stability is more important than new features.

[064b147d-1909-4665-ac07-705095c837ce]

I understand. I know that Python 3.4 is way past feature freeze.

But if we document the new stuff in the documentation, saying "Added to Python 3.4.4", people would know about and be able to use the new stuff. And we won't break people's code. In fact, people might benefit from this particular new feature.

People's Python 3.4 code like webbrowser.get("chrome") would then start to work, plus they'd benefit from this new stuff added by Brandon Milam.

Don't you find that a great thing to be?

[larryhastings]

Yes, which is why I permitted a feature freeze exception for it for 3.5. But it's simply far, far too late to add a feature like this to 3.4.

[064b147d-1909-4665-ac07-705095c837ce]

I understand. But then webbrowser.get("chrome") won't ever work in Python 3.4. Is there no other way to fix this bug in Python 3.4 without adding the new feature?

[zooba]

Fairly sure webbrowser.py is stand-alone enough that you could redist it with your package easily enough. None of the rest of the stdlib should depend on the internals, AFAIK.

[064b147d-1909-4665-ac07-705095c837ce]

Steve, I know. But it's a hassle for a newcomer to fix Python first before he/she uses it. I'm not a newcomer, but even I don't know how to fix webbrowser.py, more specifically the webbrowser.get() method, to be able to use it.

Maybe I should copy webbrowser.py from Python 3.5 and paste it to my Python 3.4.3. Sounds good, right?

[zooba]

Sure, you can do that. You can also copy-paste it into your project to get the same effect.

[064b147d-1909-4665-ac07-705095c837ce]

Ah, interesting! But which webbrowser module would Python import if I have one webbrowser.py in my interpreter's directory and one webbrowser.py in the directory of my application?

[064b147d-1909-4665-ac07-705095c837ce]

No need to answer. Python used the webbrowser module that was located in the directory of my application and not the one from the interpreter's directory. That's great!

[larryhastings]

This was backed out of 3.5, as we discovered it introduced a security hole just before 3.5.0 shipped. (See bpo-25005 for more.)

Since it's been backed out, I've reopened the issue. However I've moved it forward to 3.6, as it's no longer viable to accept for 3.5.

[1762cc99-3127-4a62-9baf-30c3d0f51ef7]

New changeset aa60b34d5200 by Steve Dower in branch '3.5': Issue bpo-25005: Backout fix for bpo-8232 because of use of unsafe subprocess.call(shell=True) https://hg.python.org/cpython/rev/aa60b34d5200

New changeset 7d320c3bf9c6 by Larry Hastings in branch '3.5': Merged in stevedower/cpython350 (pull request #20) https://hg.python.org/cpython/rev/7d320c3bf9c6

[zooba]

Here's an alternate patch I proposed on bpo-25005 before we decided to back out the change.

The problem is that subprocess.call() with shell=True is unsafe because we don't escape shell operators (such as &, \<, >, |).

The fix in this patch is to allow passing arguments to os.startfile so we can use that instead. Arguments do not need to be escaped in this case.

[0e94f9d9-1b85-433e-8b98-9cb43fef9f36]

applying 25005_1.patch patching file Lib/webbrowser.py Hunk #1 FAILED at 498 Hunk #2 FAILED at 524 Hunk #3 FAILED at 532 Hunk #4 FAILED at 540 Hunk #5 FAILED at 548 I'm trying to apply your patch after applying webbrowserfix6.patch but I am encountering problems. I first tried "hg import --no-commit file.patch" for both patches but it wouldn't let me use the command two times in a row without committing the changes for the first one so I tried committing the webbrowserfix6.patch changes and then using the import command and I get this error message. I would like to try to make sure the code still does what it is supposed to but I can't check until I can get both patches in.

Hunk #6 FAILED at 556 6 out of 6 hunks FAILED -- saving rejects to file Lib/webbrowser.py.rej patching file Modules/posixmodule.c Hunk #1 FAILED at 10522 Hunk #2 FAILED at 10578 Hunk #3 FAILED at 10590 Hunk #4 FAILED at 10606 Hunk #5 FAILED at 10616 5 out of 5 hunks FAILED -- saving rejects to file Modules/posixmodule.c.rej abort: patch failed to apply

[zooba]

Try doing:

hg up -r 4e329892817c1eed81aafd14e82b8ef23b45a6e6 hg import --no-commit http://bugs.python.org/file40384/25005_1.patch

That *should* apply it where I originally made it from. I'll do the same and rebase the patch against tip.

[zooba]

New patch against 3.6.

[0e94f9d9-1b85-433e-8b98-9cb43fef9f36]

Ok I've been able to test the new patch now and I'm not sure that os.startfile is going to work. I've been able to get os.startfile() to open a specified browser (>>> os.startfile("chrome.exe", "open")), however, the function does not allow additional arguments(>>> os.startfile("chrome.exe", "open", "www.yahoo.com")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: startfile() takes at most 2 arguments (3 given))) so not even a url will be allowed to be specified as the code is written in the patch let alone specifying new window or new tab. Is this an error on os.startfile's part? The documentation for it seems to indicate that it should take multiple inputs. I don't have much experience with C to be able to find out figure out what the rest of your patch accomplished.

[zooba]

The C part of the patch adds an extra argument to startfile to accept the arguments. You'll need to rebuild Python to test the change completely - it's no longer just a pure Python change.