webbrowser.open incomplete on Windows #52479
Comments
|
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: |
joncwchao
mannequin
added
stdlib
|
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
changed the title
|
Minor correction: BackgroundBrowser gets used in this case, but it still lacks in the same area and causes what you are seeing. |
|
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. |
|
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. |
|
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. |
|
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. |
|
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). |
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. |
|
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. |
|
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. |
|
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. |
|
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. |
|
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? |
|
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. |
|
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. |
|
Go ahead for beta 3. |
|
Moved the 64 bit browser list to its own loop and switched to browsers.append rather than +=. |
|
That looks good to me, I'll get it merged in when I'm at my desk. |
|
New changeset b75c600e1614 by Steve Dower in branch '3.5': New changeset 63b6e150b635 by Steve Dower in branch 'default': |
|
Official spelling of name of one of these browsers is "Firefox", not "FireFox". I suggest to rename WinFireFox class accordingly. |
|
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? |
|
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. |
|
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 :) . |
|
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 :) |
|
New changeset 0d54a78861cf by Steve Dower in branch '3.5': New changeset 8667c26e2bec by Steve Dower in branch 'default': |
|
I'll close this as fixed, but feel free to speak up if you spot anything else that needs fixing. |
|
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. |
|
This is not a bugfix to existing code. This is new code to implement a missing feature. |
|
Sure, let's have a broken feature in Python 3.4, who cares. |
|
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. |
|
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? |
|
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. |
|
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? |
|
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. |
|
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? |
|
Sure, you can do that. You can also copy-paste it into your project to get the same effect. |
|
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? |
|
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! |
|
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. |
|
New changeset aa60b34d5200 by Steve Dower in branch '3.5': New changeset 7d320c3bf9c6 by Larry Hastings in branch '3.5': |
|
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. |
|
applying 25005_1.patch Hunk #6 FAILED at 556 |
|
Try doing: hg up -r 4e329892817c1eed81aafd14e82b8ef23b45a6e6 That *should* apply it where I originally made it from. I'll do the same and rebase the patch against tip. |
|
New patch against 3.6. |
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. |
|
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. |
|
Finally got it rebuilt after having trouble with visual studio for awhile. I've tested the new patch and it is still able to properly find both chrome and firefox and is able to differentiate between new window and new tab for those two browsers so it appears to still be working. |
|
I've tested the new patch and it is still able to properly find both chrome and firefox and is able to differentiate between new window and new tab for those two browsers so it is still working. Would someone review the patch? |
|
Steve, would you like to create a PR based on your patch here? Also, what versions should this be targeted to now? Thanks. |
|
I have Windows 10, 64-bit, and Python 3.6.1, 64-bit, and the code still does not work! >>> import webbrowser
>>> webbrowser.get("chrome")
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "C:\Program Files\Python 3.6\lib\webbrowser.py", line 51, in get
raise Error("could not locate runnable browser")
webbrowser.Error: could not locate runnable browserNote: Yes, my Google Chrome browser was running when this command was executed. |
|
Windows Vista is no longer a concern, so find_windows_browsers() doesn't have to worry about the KEY_WOW64_* flags. IMO, it should get the browser's real name (the default value of the key) and the fully-qualified path of the executable, instead of depending solely on an "App Paths" entry being configured for the base executable name. For example: def find_windows_browsers():
""" Read the installed browsers from the Windows registry."""
import winreg
browsers = []
with winreg.OpenKey(winreg.HKEY_LOCAL_MACHINE,
r"Software\Clients\StartMenuInternet") as hkey:
i = 0
while True:
try:
subkey = winreg.EnumKey(hkey, i)
i += 1
except OSError as e:
if e.winerror != 259: # ERROR_NO_MORE_ITEMS
raise
break
try:
name = winreg.QueryValue(hkey, subkey)
if not name or not isinstance(name, str):
name = subkey
except OSError:
name = subkey
try:
cmd = winreg.QueryValue(hkey, rf"{subkey}\shell\open\command")
cmd = cmd.strip('"')
os.stat(cmd)
except (OSError, AttributeError, TypeError, ValueError):
cmd = ""
browsers.append((name, cmd))
return browsersThe loop over the result would change to The browser would get instantiated with the cmd value, which ideally is the fully-qualified path of the executable. The fallback behavior wouldn't change for the case where self.cmd is an empty string. For example: class WindowsDefault(BaseBrowser):
cmd = newwindow = newtab = ""
def __init__(self, name="", cmd=""):
super().__init__(name)
if cmd:
self.cmd = cmd
... |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: