[FIX] use a local server rather than temp file to open plots in a bro…

…wser (nilearn#4180)

* use a local server rather than temp file to open plots in a browser

* add deprecation warning & whatsnew

* update tests

* add tests + use a DeprecationWarning

* Update doc/changes/latest.rst

---------

Co-authored-by: Yasmin <[email protected]>

doc/changes/latest.rst

@@ -10,6 +10,7 @@ HIGHLIGHTS
 
 NEW
 ---
+- :bdg-info:`Plotting` The ``temp_file_lifetime`` parameter of interactive plots' ``open_in_browser`` method is deprecated and has no effect (:gh:`4180` by `Jerome Dockes`_)..
 
 Fixes
 -----

nilearn/plotting/html_document.py

@@ -1,15 +1,17 @@
 """Handle HTML plotting."""
 
-import os
-import subprocess
-import sys
-import tempfile
 import warnings
 import weakref
 import webbrowser
 from html import escape
+from http import HTTPStatus
+from http.server import BaseHTTPRequestHandler
+from queue import Empty, Queue
+from socketserver import TCPServer
+from threading import Thread
 
 MAX_IMG_VIEWS_BEFORE_WARNING = 10
+BROWSER_TIMEOUT_SECONDS = 3.0
 
 
 def set_max_img_views_before_warning(new_value):
@@ -21,12 +23,45 @@ def set_max_img_views_before_warning(new_value):
     MAX_IMG_VIEWS_BEFORE_WARNING = new_value
 
 
-def _remove_after_n_seconds(file_name, n_seconds):
-    script = os.path.join(os.path.dirname(__file__), "rm_file.py")
-    proc = subprocess.Popen(
-        [sys.executable, script, file_name, str(n_seconds)]
-    )
-    return proc
+def _open_in_browser(content):
+    """Open a page in the user's web browser.
+
+    This function starts a local server in a separate thread, opens the page
+    with webbrowser, and shuts down the server once it has served one request.
+    """
+    queue = Queue()
+
+    class Handler(BaseHTTPRequestHandler):
+        def log_message(self, *args):
+            del args
+
+        def do_GET(self):
+            if not self.path.endswith("index.html"):
+                self.send_error(HTTPStatus.NOT_FOUND, "File not found")
+                return
+            self.send_response(HTTPStatus.OK)
+            self.send_header("Content-type", "text/html")
+            self.send_header("Content-Length", str(len(content)))
+            self.end_headers()
+            self.wfile.write(content)
+            queue.put("done")
+
+    server = TCPServer(("", 0), Handler)
+    _, port = server.server_address
+
+    server_thread = Thread(target=server.serve_forever, daemon=True)
+    server_thread.start()
+
+    url = f"http://localhost:{port}/index.html"
+    webbrowser.open(url)
+    try:
+        queue.get(timeout=BROWSER_TIMEOUT_SECONDS)
+    except Empty:
+        raise RuntimeError(
+            "Failed to open nilearn plot or report in a web browser."
+        )
+    server.shutdown()
+    server_thread.join()
 
 
 class HTMLDocument:
@@ -146,7 +181,7 @@ def save_as_html(self, file_name):
         with open(file_name, "wb") as f:
             f.write(self.get_standalone().encode("utf-8"))
 
-    def open_in_browser(self, file_name=None, temp_file_lifetime=30):
+    def open_in_browser(self, file_name=None, temp_file_lifetime="deprecated"):
         """Save the plot to a temporary HTML file and open it in a browser.
 
         Parameters
@@ -155,41 +190,21 @@ def open_in_browser(self, file_name=None, temp_file_lifetime=30):
             HTML file to use as a temporary file.
 
         temp_file_lifetime : :obj:`float`, default=30
-            Time, in seconds, after which the temporary file is removed.
-            If None, it is never removed.
 
+            .. deprecated:: 0.11
 
+                The parameter is kept for backward compatibility and will be
+                removed in a future version. It has no effect.
         """
+        if temp_file_lifetime != "deprecated":
+            warnings.warn(
+                "temp_file_lifetime is deprecated. It has no "
+                "effect and passing a value for this parameter "
+                "will result in an error starting with nilearn version 0.13",
+                DeprecationWarning,
+            )
         if file_name is None:
-            fd, file_name = tempfile.mkstemp(".html", "nilearn_plot_")
-            os.close(fd)
-            named_file = False
-        else:
-            named_file = True
-        self.save_as_html(file_name)
-        self._temp_file = file_name
-        file_size = os.path.getsize(file_name) / 1e6
-        if temp_file_lifetime is None:
-            if not named_file:
-                warnings.warn(
-                    f"Saved HTML in temporary file: {file_name}\n"
-                    f"file size is {file_size:.1f}M, "
-                    "delete it when you're done, "
-                    "for example by calling this.remove_temp_file"
-                )
+            _open_in_browser(self.get_standalone().encode("utf-8"))
         else:
-            self._temp_file_removing_proc = _remove_after_n_seconds(
-                self._temp_file, temp_file_lifetime
-            )
-        webbrowser.open(f"file://{file_name}")
-
-    def remove_temp_file(self):
-        """Remove the temporary file created by \
-        ``open_in_browser``, if necessary."""
-        if self._temp_file is None:
-            return
-        if not os.path.isfile(self._temp_file):
-            return
-        os.remove(self._temp_file)
-        print(f"removed {self._temp_file}")
-        self._temp_file = None
+            self.save_as_html(file_name)
+            webbrowser.open(f"file://{file_name}")

nilearn/plotting/tests/test_html_document.py

@@ -1,10 +1,9 @@
-import os
-import tempfile
 import time
-import warnings
 import webbrowser
+from unittest.mock import Mock
 
 import pytest
+import requests
 from numpy.testing import assert_no_warnings
 
 from nilearn.plotting import html_document
@@ -14,41 +13,52 @@
 # warnings
 
 
-def _open_mock(f):
-    print(f"opened {f}")
-
-
-def test_temp_file_removing():
-    html = html_document.HTMLDocument("hello")
-    wb_open = webbrowser.open
-    webbrowser.open = _open_mock
-    fd, tmpfile = tempfile.mkstemp()
-    try:
-        os.close(fd)
-        with warnings.catch_warnings(record=True) as record:
-            html.open_in_browser(file_name=tmpfile, temp_file_lifetime=None)
-        for warning in record:
-            assert "Saved HTML in temporary file" not in str(warning.message)
-        html.open_in_browser(temp_file_lifetime=0.5)
-        assert os.path.isfile(html._temp_file)
-        html._temp_file_removing_proc.wait()
-        assert not os.path.isfile(html._temp_file)
-        with pytest.warns(UserWarning, match="Saved HTML in temporary file"):
-            html.open_in_browser(temp_file_lifetime=None)
-        html.open_in_browser(temp_file_lifetime=None)
-        assert os.path.isfile(html._temp_file)
-        time.sleep(1.5)
-        assert os.path.isfile(html._temp_file)
-    finally:
-        webbrowser.open = wb_open
-        try:
-            os.remove(html._temp_file)
-        except Exception:
-            pass
-        try:
-            os.remove(tmpfile)
-        except Exception:
-            pass
+class Get:
+    def __init__(self, delay=0.0):
+        self.delay = delay
+
+    def __call__(self, url):
+        time.sleep(self.delay)
+        self.url = url
+        requests.get(url.replace("index.html", "favicon.ico"))
+        self.content = requests.get(url).content
+
+
+# disable request mocking for this test -- note we are accessing localhost only
+@pytest.mark.parametrize("request_mocker", [None])
+def test_open_in_browser(monkeypatch):
+    opener = Get()
+    monkeypatch.setattr(webbrowser, "open", opener)
+    doc = html_document.HTMLDocument("hello")
+    doc.open_in_browser()
+    assert opener.content == b"hello"
+
+
+def test_open_in_browser_timeout(monkeypatch):
+    opener = Get(delay=1.0)
+    monkeypatch.setattr(webbrowser, "open", opener)
+    monkeypatch.setattr(html_document, "BROWSER_TIMEOUT_SECONDS", 0.01)
+    doc = html_document.HTMLDocument("hello")
+    with pytest.raises(RuntimeError, match="Failed to open"):
+        doc.open_in_browser()
+
+
+@pytest.mark.parametrize("request_mocker", [None])
+def test_open_in_browser_deprecation_warning(monkeypatch):
+    monkeypatch.setattr(webbrowser, "open", Get())
+    doc = html_document.HTMLDocument("hello")
+    with pytest.warns(DeprecationWarning, match="temp_file_lifetime"):
+        doc.open_in_browser(temp_file_lifetime=30.0)
+
+
+def test_open_in_browser_file(tmp_path, monkeypatch):
+    opener = Mock()
+    monkeypatch.setattr(webbrowser, "open", opener)
+    file_path = tmp_path / "doc.html"
+    doc = html_document.HTMLDocument("hello")
+    doc.open_in_browser(file_name=str(file_path))
+    assert file_path.read_text("utf-8") == "hello"
+    opener.assert_called_once_with(f"file://{file_path}")
 
 
 def _open_views():

nilearn/plotting/tests/test_js_plotting_utils.py

@@ -2,7 +2,6 @@
 import os
 import re
 import tempfile
-import webbrowser
 
 import numpy as np
 import pytest
@@ -208,7 +207,6 @@ def check_html(
     assert html._repr_html_() == html.get_iframe()
     assert str(html) == html.get_standalone()
     assert '<meta charset="UTF-8" />' in str(html)
-    _check_open_in_browser(html)
     resized = html.resize(3, 17)
     assert resized is html
     assert (html.width, html.height) == (3, 17)
@@ -241,32 +239,6 @@ def check_html(
     assert len(view.findall("option")) == 7
 
 
-def _open_mock(f):
-    print(f"opened {f}")
-
-
-def _check_open_in_browser(html):
-    wb_open = webbrowser.open
-    webbrowser.open = _open_mock
-    try:
-        html.open_in_browser(temp_file_lifetime=None)
-        temp_file = html._temp_file
-        assert html._temp_file is not None
-        assert os.path.isfile(temp_file)
-        html.remove_temp_file()
-        assert html._temp_file is None
-        assert not os.path.isfile(temp_file)
-        html.remove_temp_file()
-        html._temp_file = "aaaaaaaaaaaaaaaaaaaaaa"
-        html.remove_temp_file()
-    finally:
-        webbrowser.open = wb_open
-        try:
-            os.remove(temp_file)
-        except Exception:
-            pass
-
-
 @pytest.mark.parametrize(
     "colors",
     [