Skip to content

Commit 764fb72

Browse files
Sushmeyneiljp
authored andcommitted
bugfix: core: Resolve macOS crash on opening message in web browser.
This occurs due to an attribute error in the existing code, since a class in the standard library webbrowser module didn't have a 'name' attribute until Python 3.11 under macOS. While this attribute was not officially documented until Python 3.11, many if not all other browser drivers provided a name attribute before this point, and the existing use of the attribute did not cause issues when our original implementation was tested. This bugfix substitutes a default value in the absence of the name, rather than testing against the specific platform/python combination, which will protect against similar issues arising in future. Test updated. Fixes #1471.
1 parent b71aec2 commit 764fb72

File tree

2 files changed

+27
-10
lines changed

2 files changed

+27
-10
lines changed

tests/core/test_core.py

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -406,27 +406,41 @@ def test_copy_to_clipboard_exception(
406406
assert popup.call_args_list[0][0][1] == "area:error"
407407

408408
@pytest.mark.parametrize(
409-
"url",
409+
"url, webbrowser_name, expected_webbrowser_name",
410410
[
411-
"https://chat.zulip.org/#narrow/stream/test",
412-
"https://chat.zulip.org/user_uploads/sent/abcd/efg.png",
413-
"https://github.com/",
411+
("https://chat.zulip.org/#narrow/stream/test", "chrome", "chrome"),
412+
(
413+
"https://chat.zulip.org/user_uploads/sent/abcd/efg.png",
414+
"mozilla",
415+
"mozilla",
416+
),
417+
("https://github.com/", None, "default browser"),
414418
],
415419
)
416420
def test_open_in_browser_success(
417-
self, mocker: MockerFixture, controller: Controller, url: str
421+
self,
422+
mocker: MockerFixture,
423+
controller: Controller,
424+
url: str,
425+
webbrowser_name: Optional[str],
426+
expected_webbrowser_name: str,
418427
) -> None:
419428
# Set DISPLAY environ to be able to run test in CI
420429
os.environ["DISPLAY"] = ":0"
421430
mocked_report_success = mocker.patch(MODULE + ".Controller.report_success")
422431
mock_get = mocker.patch(MODULE + ".webbrowser.get")
423432
mock_open = mock_get.return_value.open
424433

434+
if webbrowser_name is None:
435+
del mock_get.return_value.name
436+
else:
437+
mock_get.return_value.name = webbrowser_name
438+
425439
controller.open_in_browser(url)
426440

427441
mock_open.assert_called_once_with(url)
428442
mocked_report_success.assert_called_once_with(
429-
[f"The link was successfully opened using {mock_get.return_value.name}"]
443+
[f"The link was successfully opened using {expected_webbrowser_name}"]
430444
)
431445

432446
def test_open_in_browser_fail__no_browser_controller(

zulipterminal/core.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -414,11 +414,14 @@ def open_in_browser(self, url: str) -> None:
414414
# Suppress stdout and stderr when opening browser
415415
with suppress_output():
416416
browser_controller.open(url)
417+
418+
# MacOS using Python version < 3.11 has no "name" attribute
419+
# - https://github.com/python/cpython/issues/82828
420+
# - https://github.com/python/cpython/issues/87590
421+
# Use a default value if missing, for macOS, and potential later issues
422+
browser_name = getattr(browser_controller, "name", "default browser")
417423
self.report_success(
418-
[
419-
"The link was successfully opened using "
420-
f"{browser_controller.name}"
421-
]
424+
[f"The link was successfully opened using {browser_name}"]
422425
)
423426
except webbrowser.Error as e:
424427
# Set a footer text if no runnable browser is located

0 commit comments

Comments
 (0)