From d3150c5d3ccbe2868aacfda69de4462302c1c032 Mon Sep 17 00:00:00 2001 From: Lukas Geiger Date: Thu, 15 Oct 2026 09:19:57 +0200 Subject: [PATCH] fix(browser): classify provider clicks off the GUI thread Delay file selection and activation through AsyncFs; discard results after newer clicks or navigation. Refs #27; batching and physical CrossDevice acceptance remain open. Generated-with: gpt-6.1-sol (Codex) --- src/gui/browser/file_browser.py | 45 +++++++++----- tests/test_file_browser.py | 75 ++++++++++++++++++++++++ tests/test_store_regressions_20260928.py | 3 + 3 files changed, 108 insertions(+), 15 deletions(-) diff --git a/src/gui/browser/file_browser.py b/src/gui/browser/file_browser.py index 569493f..ea516d5 100644 --- a/src/gui/browser/file_browser.py +++ b/src/gui/browser/file_browser.py @@ -160,6 +160,7 @@ def __init__(self, parent=None): self._known_dirs = set() self._tokens = {} # token -> (kind, path) self._nav_token = 0 + self._item_seq = 0 self._preload_queue = [] self._preload_busy = False self._preload_seq = 0 @@ -323,6 +324,7 @@ def navigate_to(self, path: str): self._switch(normalize_user_path(path), True) def _switch(self, path: str, record: bool): + self._item_seq += 1 # navigation invalidates pending preview/activation # Every navigation (also back/forward/up, which all end here) invalidates older # checks with exactly ONE new token; the recheck/nav branches below reuse it. # Without this a late failure of an earlier target (e.g. a dead E:) stayed @@ -413,6 +415,20 @@ def _on_fs_result(self, token, result, error): if entry is None: # timed out: a late answer is dropped return kind, path = entry + if kind in ("item-select", "item-activate"): + if token != ("item", self._item_seq) or error is not None: + return + if kind == "item-select": + if result == "file": + self.file_selected.emit(path) + elif result == "dir": + self.navigate_to(path) + elif result == "file": + if Path(path).suffix.lower() in EDITOR_EXTENSIONS: + self._edit_file(path) + else: + self._open_file(path) + return if kind == "preload": self._preload_busy = False if error is None and result: @@ -552,25 +568,24 @@ def set_show_hidden_files(self, show: bool): self.refresh() def _on_item_clicked(self, index: QModelIndex): - source_index = self.proxy.mapToSource(index) - file_path = self.model.filePath(source_index) - - if os.path.isfile(file_path): - self.file_selected.emit(file_path) + self._submit_item_action(index, "item-select") def _on_item_double_clicked(self, index: QModelIndex): + self._submit_item_action(index, "item-activate") + + def _submit_item_action(self, index: QModelIndex, kind: str): + # Even stat() can wait for a cloud/phone provider. Do not classify an + # entry in the GUI slot, before asynchronous navigation has a chance. + self._item_seq += 1 + if not index.isValid(): + return source_index = self.proxy.mapToSource(index) + if not source_index.isValid(): + return file_path = self.model.filePath(source_index) - - if os.path.isdir(file_path): - self.navigate_to(file_path) - else: - # Prüfe ob Editor-Datei - ext = Path(file_path).suffix.lower() - if ext in EDITOR_EXTENSIONS: - self._edit_file(file_path) - else: - self._open_file(file_path) + if file_path: + self._submit(("item", self._item_seq), kind, file_path, + _classify, self.NAV_TIMEOUT_MS) def _show_context_menu(self, pos): """Zeigt das Kontextmenü""" diff --git a/tests/test_file_browser.py b/tests/test_file_browser.py index 3da0658..e9deb5f 100644 --- a/tests/test_file_browser.py +++ b/tests/test_file_browser.py @@ -5,6 +5,8 @@ from pathlib import Path from unittest.mock import Mock +import pytest + os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") SRC_DIR = Path(__file__).resolve().parents[1] / "src" @@ -23,6 +25,79 @@ def _ensure_app() -> QApplication: return app +@pytest.mark.parametrize("double_click", [False, True]) +def test_provider_item_click_does_not_block_gui(tmp_path, monkeypatch, qtbot, double_click): + import threading + import time + + _ensure_app() + browser = FileBrowser() + qtbot.addWidget(browser) + target = tmp_path / "provider-folder" + target.mkdir() + browser.navigate_to(str(tmp_path)) + qtbot.waitUntil(lambda: browser.model.index(str(target)).isValid()) + qtbot.waitUntil(lambda: browser.proxy.mapFromSource(browser.model.index(str(target))).isValid()) + index = browser.proxy.mapFromSource(browser.model.index(str(target))) + entered, release = threading.Event(), threading.Event() + caller_threads = [] + real_isdir, real_isfile = os.path.isdir, os.path.isfile + + def provider_metadata(real, answer): + def call(path): + if os.path.normcase(os.path.normpath(os.fspath(path))) == os.path.normcase(str(target)): + caller_threads.append(threading.get_ident()) + entered.set() + release.wait(0.4) + return answer + return real(path) + return call + + monkeypatch.setattr(os.path, "isdir", provider_metadata(real_isdir, True)) + monkeypatch.setattr(os.path, "isfile", provider_metadata(real_isfile, False)) + try: + start = time.monotonic() + handler = browser._on_item_double_clicked if double_click else browser._on_item_clicked + handler(index) + assert time.monotonic() - start < 0.2, "provider metadata blocked the GUI handler" + qtbot.waitUntil(entered.is_set) + assert all(thread != threading.get_ident() for thread in caller_threads) + # Moving away must remain possible while the provider is still waiting. + browser.go_back() + previous = browser.current_path + finally: + release.set() + qtbot.waitUntil(lambda: not browser._tokens) + assert browser.current_path == previous, "late click result replaced the newer page" + + +@pytest.mark.parametrize("name,action", [("code.py", "edit"), ("document.pdf", "open"), ("folder", "navigate")]) +def test_async_item_activation_keeps_file_and_folder_actions(tmp_path, monkeypatch, qtbot, name, action): + _ensure_app() + browser = FileBrowser() + qtbot.addWidget(browser) + target = tmp_path / name + if action == "navigate": + target.mkdir() + else: + target.write_text("content", encoding="utf-8") + browser.navigate_to(str(tmp_path)) + qtbot.waitUntil(lambda: browser.model.index(str(target)).isValid()) + index = browser.proxy.mapFromSource(browser.model.index(str(target))) + edit, opened, navigate = Mock(), Mock(), Mock() + monkeypatch.setattr(browser, "_edit_file", edit) + monkeypatch.setattr(browser, "_open_file", opened) + monkeypatch.setattr(browser, "navigate_to", navigate) + browser._on_item_double_clicked(index) + expected = {"edit": edit, "open": opened, "navigate": navigate}[action] + qtbot.waitUntil(lambda: expected.called) + assert expected.call_count == 1 + assert Path(expected.call_args.args[0]) == target + for other in (edit, opened, navigate): + if other is not expected: + other.assert_not_called() + + def test_dnd_flags_enabled(): """DnD-Flags: acceptDrops auf Tabelle UND FileBrowser muss True sein.""" _ensure_app() diff --git a/tests/test_store_regressions_20260928.py b/tests/test_store_regressions_20260928.py index 538ab09..4dc8af3 100644 --- a/tests/test_store_regressions_20260928.py +++ b/tests/test_store_regressions_20260928.py @@ -114,6 +114,9 @@ def test_navigation_loads_files_and_selection_reaches_preview(tmp_path): item = browser.proxy.mapFromSource(browser.model.index(str(target))) assert item.isValid() browser._on_item_clicked(item) + deadline = time.monotonic() + 5 + while "Äpfel" not in preview.text_preview.toPlainText() and time.monotonic() < deadline: + QTest.qWait(10) assert "Äpfel" in preview.text_preview.toPlainText() browser.close() preview.close()