Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-102871: Remove obsolete browsers from webbrowser #102872

Merged
merged 10 commits into from
Mar 31, 2023
Merged

gh-102871: Remove obsolete browsers from webbrowser #102872

merged 10 commits into from
Mar 31, 2023

Conversation

DBJim
Copy link
Contributor

@DBJim DBJim commented Mar 21, 2023

@terryjreedy
Copy link
Member

Do any of these removals need deprecation?

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is literally the first time I hear about most of them.
Even https://browsersl.ist/ and https://caniuse.com/ do not list them anywhere.

I don't think that using Python3.12 and Netscape at the same time is even possible in this reality :)

@arhadthedev arhadthedev added the stdlib Python modules in the Lib dir label Mar 22, 2023
@cpython-cla-bot
Copy link

cpython-cla-bot bot commented Mar 26, 2023

All commit authors signed the Contributor License Agreement.
CLA signed

@merwok merwok changed the title gh-102871: Remove obsolete browsers and test from webbrowser gh-102871: Remove obsolete browsers from webbrowser Mar 26, 2023
@merwok
Copy link
Member

merwok commented Mar 26, 2023

I propose to merge this for 3.12 with a note in what’s new and module docs, no deprecation warning, and no backports.

@terryjreedy
Copy link
Member

terryjreedy commented Mar 26, 2023

The test failure is a patchcheck whitespace failure in webbrowser.py. James, please run patchcheck in your local repository with the branch checked out. Either make patchcheck on non-windows or python tools/scripts/patchcheck.py on windows. If something is changed, which there should be, commit and push as usual.

Copy link
Member

@merwok merwok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name the removed browsers in the news note, and in a versionchanged note in the module docs.

@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@DBJim
Copy link
Contributor Author

DBJim commented Mar 27, 2023

I have made the requested changes; please review again.

@bedevere-bot
Copy link

Thanks for making the requested changes!

@merwok: please review the changes made to this pull request.

@bedevere-bot bedevere-bot requested a review from merwok March 27, 2023 00:20
@merwok
Copy link
Member

merwok commented Mar 27, 2023

I don’t get the note about Firefox older than version 35, can you explain?

@DBJim
Copy link
Contributor Author

DBJim commented Mar 27, 2023

I don’t get the note about Firefox older than version 35, can you explain?

Sure! 'Netscape' class was used as an alias for Firefox until 2015. The current Firefox handling was added to webbrowser in 2015 because support for the -raise, -remote and other arguments used by the 'Netscape' class were dropped from Firefox v36+. The discussion is here: https://bugs.python.org/issue23262

There was discussion to drop the old 'Netscape' class in that issue, but it was decided to keep for backwards compatibility for old browsers which may have still had a user base then. Since it's been another 7 years, I suggest that's no longer true and the class is not required in future versions of Python.

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

It may be worth to add also a What's New entry (in the "Removed" section).

Lib/webbrowser.py Outdated Show resolved Hide resolved
DBJim and others added 2 commits March 28, 2023 08:05
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@merwok merwok merged commit b0422e1 into python:main Mar 31, 2023
warsaw pushed a commit to warsaw/cpython that referenced this pull request Apr 11, 2023
@vstinner
Copy link
Member

This issue broke the Konqueror class which still uses the _remote() method, removed by this PR:

$ ./python -m webbrowser https://www.google.com/
Traceback (most recent call last):
  File "/home/vstinner/python/main/Lib/runpy.py", line 198, in _run_module_as_main
    return _run_code(code, main_globals, None,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vstinner/python/main/Lib/runpy.py", line 88, in _run_code
    exec(code, run_globals)
  File "/home/vstinner/python/main/Lib/webbrowser.py", line 631, in <module>
    main()
  File "/home/vstinner/python/main/Lib/webbrowser.py", line 626, in main
    open(url, new_win)
  File "/home/vstinner/python/main/Lib/webbrowser.py", line 86, in open
    if browser.open(url, new, autoraise):
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vstinner/python/main/Lib/webbrowser.py", line 398, in open
    ok = self._remote("LOAD " + url)
         ^^^^^^^^^^^^
AttributeError: 'Konqueror' object has no attribute '_remote'

By the way, webbrowser.Konqueror defines twice its open() method!

@sobolevn
Copy link
Member

@vstinner seems like a bad merge conflict resolution, PR is here: #105745

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Python modules in the Lib dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants