[Patches] [ python-Patches-728278 ] Multiple webbrowser.py bug
fixes / improvements
SourceForge.net
noreply at sourceforge.net
Mon Mar 21 00:46:09 CET 2005
Patches item #728278, was opened at 2003-04-27 00:02
Message generated for change (Comment added) made by sdeibel
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=728278&group_id=5470
Category: Library (Lib)
Group: Python 2.4
Status: Open
Resolution: None
Priority: 5
Submitted By: Stephan R.A. Deibel (sdeibel)
Assigned to: Fred L. Drake, Jr. (fdrake)
Summary: Multiple webbrowser.py bug fixes / improvements
Initial Comment:
In using webbrowser.py we uncovered and fixed a number
of problems and made some improvements in usability and
consistency of behavior.
Appended below is a summary of the changes made. This
list is also found at the top of the uploaded file,
which was based on the version of webbrowser.py found
in Python 2.3a2. Sorry to submit so many changes at
once but they were all made in a period of a few days
when we went through this module for use in Wing IDE.
I'm also submitting some unit tests for the module.
I've tried to review everything carefully, further
review is definately needed. Feel free to contact me
if you have questions or comments or want me to make
changes and resubmit.
Hope this is helpful.
- Stephan
----------------------------------------------------------------------
>Comment By: Stephan R.A. Deibel (sdeibel)
Date: 2005-03-20 18:46
Message:
Logged In: YES
user_id=12608
Sorry, guilty as charged re: uploading whole files. I did
this for lack of time since there is some value here. Diffs
are also not that useful given large number of changes.
Agree this needs to be split up.
Some additional comments:
Splitting off the bug fixes for specific OSes may also be a
good idea so they can be reviewed separately.
Ediff is a good way to review the changes against current
sources and make smaller patches.
I deleted the initial (bad) upload. Not sure why I didn't
do that originally.
----------------------------------------------------------------------
Comment By: Rodrigo Dias Arruda Senra (rodsenra)
Date: 2005-03-20 09:04
Message:
Logged In: YES
user_id=9057
Against the [http://python.org/patches/ Patch Submission
Guidelines] this entry has uploaded both the whole file and
.tgz, but no patch file.
The large numbers of changes make it difficult to review.
All the changes were documented to the top of the file, I
don't believe they
belong there. At least that is not complaint to the first
rule in
http://www.python.org/patches/style.html
There are several OS X specific corrections that I'm unable
to reproduce/validate, since I have no access to that
platform.
The modules was renamed to wingwebbrowser.py and the test
case was built using this name. I believe this is
inadequate, the original module name should be kept.
There are three categories of changes: bug fixes, internal
changes and interface changes. IMO it would be better to
break this patch in at least three, matching these types of
changes. Bug fixes would have a higher priority, less chance
to break backward compatibility and more chance to be
incorporated earlier.
It should be applied before patch 1077979, otherwise that
fix would be reversed.
In spite of the formatting comments above, there are
*valuable* changes in the submitted file.
This is a nice candidate to be tackled in a Python Bug Day,
since it functionality involves a broad range of platforms
and user browsers preferences. Moreover, thourough test
cases are hard to be cooked.
I recommend keeping it open until somebody (possibly its
author) reshape it properly and then applying it.
----------------------------------------------------------------------
Comment By: Rodrigo Dias Arruda Senra (rodsenra)
Date: 2005-03-19 23:38
Message:
Logged In: YES
user_id=9057
According to the Patch Submission Guidelines, we are
supposed to submit patch files nor modified versions of
modules nor zip files. I advise you
to resubmit according to the guidelines documented in
http://python.org/patches/
----------------------------------------------------------------------
Comment By: Stephan R.A. Deibel (sdeibel)
Date: 2004-11-23 19:08
Message:
Logged In: YES
user_id=12608
I'm uploading a newer version of the webbrowser.py file,
which contains a
small fix to avoid failing when there is an empty ("")
BROWSER environment variable set. The diff is just this:
--- wingwebbrowser.py 20 Feb 2004 23:45:26 -0000 1.3
+++ wingwebbrowser.py 10 Nov 2004 16:51:36 -0000 1.4
@@ -210,6 +210,8 @@
def _splitcommand(cmd):
"""Extract command name and args from command line"""
+ if cmd == '':
+ return '', ''
if cmd[0] in ('"', "'"):
name = cmd[1:cmd[1:].find(cmd[0])+1]
args = cmd[len(name)+2:].strip()
@@ -603,7 +605,8 @@
# Treat choices in same way as if passed into get() but
do register
# and prepend to _tryorder
for cmdline in _userchoices:
- _synthesize(cmdline, -1, True)
+ if cmdline != '':
+ _synthesize(cmdline, -1, True)
cmdline = None # to make del work if _userchoices was empty
del cmdline
del _userchoices
----------------------------------------------------------------------
Comment By: Michael Hudson (mwh)
Date: 2003-04-29 10:50
Message:
Logged In: YES
user_id=6656
Is now.
----------------------------------------------------------------------
Comment By: Stephan R.A. Deibel (sdeibel)
Date: 2003-04-29 10:46
Message:
Logged In: YES
user_id=12608
Yes, I intended it to be post-2.3. The Group popup choices
doesn't include 2.4, however.
----------------------------------------------------------------------
Comment By: Martin v. Löwis (loewis)
Date: 2003-04-29 07:51
Message:
Logged In: YES
user_id=21627
Because this is quite a large change, I recommend to
postpone it after 2.3.
----------------------------------------------------------------------
Comment By: Stephan R.A. Deibel (sdeibel)
Date: 2003-04-27 00:32
Message:
Logged In: YES
user_id=12608
Jeez, here is the list of changes in this patch which I
meant to append to the original report.
Bugs fixed:
* Don't apply lower() to command lines or commands that are
going to be executed
* Don't confuse browser name/id with command line used to
launch the browser
* Require that browser commands be executable
* Identify user-provided strings as a command line by
looking for any args and
not just for %s
* Handle spaces in user-provided command names, either if
quoted on the
user-provided command line or escaped with back slashes
* Use '%s' instead of "%s", which avoids character
interpretation on
Unix command lines
* Escape and quote urls more safely to prevent crafting urls
that result
in command lines that execute arbitrary commands
* Fixed Galeon so it doesn't hang up the app until the
browser is quit
* Added the Mac OS X support that doesn't make bad
environment assumptions
on OS 10.1
* Fixed win32 use of Netscape, which before would never work
because
the _tryorder entry was being pruned out
* Leave found items in _browsers and _tryorder on OS X (but
prepend
the OS X specific support in _tryorder). OS X is Unix so
these are useful
there.
* Now add %s to end of command lines if they don't contain
%s already, for
compatibility e.g. with KDE's default value for BROWSER
environment
* Now add '&' to end of Unix command lines that are going to
be launched
from GenericBrowser to avoid hanging up on user-provided
command lines
* Added additional quoting of command names, so some of the
browser classes
can work with a renamed browser with a space in the
executable name
* Added a number of return values / checks where before
there were bad
assumptions in the code that might have led to errors
Other internal changes made:
* Added unit tests
* Removed the loop in get() which always returned in the
first iteration
* _tryorder more tightly coupled with registering in
_browsers, and removed
potentially problematic last-ditch GenericBrowser
registering clause and
the prune-_tryorder clauses from the end of the module
* _iscommand also returns true if cmd is an existing file
name, and it
returns the full path or None instead of just True and False
* _synthesize registers and returns GenericBrowser if
synthesis fails but
the user-provided command line looks valid
* Some additional uses of True and False instead of 1 and 0
Changes to the public interface:
* Added optional update_tryorder arg to register() (default
value is same
as previous action of this function)
* Open/open_new return False if command definately failed
and True if
it may have succeeded (both previously returned None in
all cases)
* Get() returns a GenericCommand if 'using' is given and
doesn't name or
synthesize a browser (previously it did this correctly
only when %s
was in the given command line)
* Values passed via BROWSER environment will be registered
and synthesized
in the same way as 'using' arg values are treated in get()
----------------------------------------------------------------------
Comment By: Stephan R.A. Deibel (sdeibel)
Date: 2003-04-27 00:14
Message:
Logged In: YES
user_id=12608
Oops, correcting the upload
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=728278&group_id=5470
More information about the Patches
mailing list