From cbd8ed018677c80e2943a2440007e097ab7412fa Mon Sep 17 00:00:00 2001 From: claudi Date: Tue, 14 Apr 2026 14:12:51 +0200 Subject: [PATCH] feat: Enhance URL conversion and bridge script handling for improved drag-and-drop functionality --- src/webdrop_bridge/core/url_converter.py | 16 +- .../ui/bridge_script_intercept.js | 20 +- src/webdrop_bridge/ui/main_window.py | 270 ++++++++++++++++-- tests/unit/test_url_converter.py | 6 +- 4 files changed, 275 insertions(+), 37 deletions(-) diff --git a/src/webdrop_bridge/core/url_converter.py b/src/webdrop_bridge/core/url_converter.py index 05b127f..62ab51c 100644 --- a/src/webdrop_bridge/core/url_converter.py +++ b/src/webdrop_bridge/core/url_converter.py @@ -1,6 +1,7 @@ """URL to local path conversion for Azure Blob Storage URLs.""" import logging +import os from pathlib import Path from typing import Optional from urllib.parse import unquote @@ -51,13 +52,14 @@ class URLConverter: # Combine with local path local_path = Path(mapping.local_path) / relative_path - # Normalize path (resolve .. and .) but don't follow symlinks yet - try: - # On Windows, normalize separators - local_path = Path(str(local_path).replace("/", "\\")) - except (OSError, RuntimeError) as e: - logger.warning(f"Failed to normalize path {local_path}: {e}") - return None + # Keep legacy Windows separator normalization to preserve + # existing Windows drag/drop behavior. + if os.name == "nt": + try: + local_path = Path(str(local_path).replace("/", "\\")) + except (OSError, RuntimeError) as e: + logger.warning(f"Failed to normalize path {local_path}: {e}") + return None logger.debug(f"Converted URL to path: {url} -> {local_path}") return local_path diff --git a/src/webdrop_bridge/ui/bridge_script_intercept.js b/src/webdrop_bridge/ui/bridge_script_intercept.js index 53e28f0..bc8d8e7 100644 --- a/src/webdrop_bridge/ui/bridge_script_intercept.js +++ b/src/webdrop_bridge/ui/bridge_script_intercept.js @@ -79,13 +79,13 @@ function installDragHandler() { console.log('[Intercept] Installing dragstart handler, have', angularDragHandlers.length, 'Angular handlers'); - + if (angularDragHandlers.length === 0) { console.warn('[Intercept] No Angular handlers found yet, will retry in 1s...'); setTimeout(installDragHandler, 1000); return; } - + // Only install once, even if called multiple times if (dragHandlerInstalled) { console.log('[Intercept] Handler already installed, skipping'); @@ -122,7 +122,7 @@ // Intercept any drag with URLs that match our configured mappings if (currentDragUrls.length > 0) { var shouldIntercept = false; - + // Check each URL against configured URL mappings // Intercept if ANY URL matches if (window.webdropConfig && window.webdropConfig.urlMappings) { @@ -148,14 +148,14 @@ } } } - + if (shouldIntercept) { console.log('%c[Intercept] PREVENTING browser drag, using Qt for ' + currentDragUrls.length + ' file(s)', 'background: #F44336; color: white; font-weight: bold; padding: 4px 8px;'); - + e.preventDefault(); e.stopPropagation(); - + ensureChannel(function() { if (window.bridge && typeof window.bridge.start_file_drag === 'function') { console.log('%c[Intercept] → Qt: start_file_drag with ' + currentDragUrls.length + ' file(s)', 'color: #9C27B0; font-weight: bold;'); @@ -165,7 +165,7 @@ console.error('[Intercept] bridge.start_file_drag not available!'); } }); - + currentDragUrls = []; return false; } @@ -176,12 +176,12 @@ console.log('[Intercept] dragstart handler installed ✓'); } - + // Wait for Angular to register its listeners, then install our handler // Start checking after 3 seconds (give Angular time to load), then retry for up to 30 seconds var installRetries = 0; var maxRetries = 27; // 3 initial + 27 retries * 1s = 30s total - + function scheduleInstall() { if (dragHandlerInstalled) return; // Already done installRetries++; @@ -193,7 +193,7 @@ console.warn('[Intercept] Gave up waiting for Angular handlers after 30s'); } } - + setTimeout(scheduleInstall, 3000); // ============================================================================ diff --git a/src/webdrop_bridge/ui/main_window.py b/src/webdrop_bridge/ui/main_window.py index 7b7bb5a..2549857 100644 --- a/src/webdrop_bridge/ui/main_window.py +++ b/src/webdrop_bridge/ui/main_window.py @@ -426,6 +426,11 @@ class MainWindow(QMainWindow): self.config = config self._background_threads = [] # Keep references to background threads self._background_workers = {} # Keep references to background workers + self._bridge_script_source = "" # Cache combined bridge source for recovery injection + self._bridge_script_re_registered = False # Flag to prevent duplicate re-registration on same load + self._is_page_loading = False # Track if a page load is currently in progress + self._pending_reload = False # Coalesce multiple rapid reload requests into one + self._load_sequence = 0 # Monotonic counter to ignore stale async recovery callbacks self.checking_dialog = None # Track the checking dialog self.downloading_dialog = None # Track the download dialog self._is_manual_check = False # Track if this is a manual check (for UI feedback) @@ -498,6 +503,10 @@ class MainWindow(QMainWindow): # Install the drag bridge script self._install_bridge_script() + # Connect to loadStarted to re-register script before page loads + # This ensures the script is injected even on toolbar Reload button clicks + self.web_view.loadStarted.connect(self._on_page_load_started) + # Connect to loadFinished to verify script injection self.web_view.loadFinished.connect(self._on_page_loaded) @@ -598,9 +607,12 @@ class MainWindow(QMainWindow): script = QWebEngineScript() script.setName("webdrop-bridge") - # Use Deferred instead of DocumentCreation to allow DOM to be ready first - # This prevents race conditions with JavaScript event listeners - script.setInjectionPoint(QWebEngineScript.InjectionPoint.Deferred) + # Preserve existing Windows behavior and use earlier injection on macOS + # so dragstart handlers are captured before Angular registers them. + if sys.platform == "darwin": + script.setInjectionPoint(QWebEngineScript.InjectionPoint.DocumentCreation) + else: + script.setInjectionPoint(QWebEngineScript.InjectionPoint.Deferred) script.setWorldId(QWebEngineScript.ScriptWorldId.MainWorld) script.setRunsOnSubFrames(False) @@ -735,6 +747,9 @@ class MainWindow(QMainWindow): for i, mapping in enumerate(self.config.url_mappings): logger.debug(f" Mapping {i+1}: {mapping.url_prefix} → {mapping.local_path}") + # Keep script source for runtime recovery if a navigation drops injected scripts. + self._bridge_script_source = combined_code + script.setSourceCode(combined_code) self.web_view.page().scripts().insert(script) logger.debug(f"✅ Successfully installed bridge script") @@ -1338,32 +1353,243 @@ class MainWindow(QMainWindow): logger.error(f"JS Console: {message}") logger.debug(f" at {source_id}:{line_number}") + def _on_page_load_started(self) -> None: + """Called when a page starts loading (before loadFinished). + + Re-registers the bridge script to ensure it will be injected on reload, + page navigation, or any load event. + + Uses a flag to prevent duplicate re-registrations if loadStarted fires multiple times. + """ + self._is_page_loading = True + self._load_sequence += 1 + self._bridge_script_re_registered = False + + logger.debug("Page load started - ensuring bridge script is registered") + if self._bridge_script_source: + self._ensure_bridge_script_exists(verbose=False) + self._bridge_script_re_registered = True + else: + logger.debug("Bridge script source not cached; skipping re-registration") + def _on_page_loaded(self, success: bool) -> None: """Called when a page finishes loading. - Checks if the bridge script was successfully injected. + Checks if the bridge script was successfully injected, with automatic recovery + for page reloads and redirects. + + Resets the re-registration flag for the next load cycle. Args: success: True if page loaded successfully """ + # Mark load finished and reset flag for next load cycle + finished_sequence = self._load_sequence + self._is_page_loading = False + self._bridge_script_re_registered = False + + # If user pressed reload multiple times during loading, perform exactly one + # more reload after a short settle delay. + if self._pending_reload: + self._pending_reload = False + QTimer.singleShot(150, self._trigger_reload_now) + if not success: logger.warning("Page failed to load") return - # Check if bridge script is loaded - def check_script(result): - if result: - logger.debug("WebDrop Bridge script is active") - logger.debug("QWebChannel bridge is ready") - else: - logger.error("WebDrop Bridge script NOT loaded!") - logger.error("Drag-and-drop conversion will not work") + def _verify_bridge_loaded(stage: str, attempt: int = 1, sequence: int = finished_sequence) -> None: + """Check if bridge marker exists and optionally recover script injection. + + Implements multi-attempt recovery strategy: + - initial: First check after page load (50ms delay) + - recovery_N: Recovery attempts with progressive delays + - Each recovery reattempt after a delay to handle late injections + """ + if sequence != self._load_sequence: + logger.debug("Skipping stale bridge verification run for an older page load") + return - # Execute JS to check if our script is loaded - self.web_view.page().runJavaScript( - "typeof window.__webdrop_intercept_injected !== 'undefined' && window.__webdrop_intercept_injected === true", - check_script, - ) + def check_script(result): + if sequence != self._load_sequence: + logger.debug( + "Skipping stale bridge verification callback for an older page load" + ) + return + + if result: + logger.debug("WebDrop Bridge script is active") + logger.debug("QWebChannel bridge is ready") + return + + # Multi-stage recovery for page reloads and redirects + if stage == "initial" and self._bridge_script_source: + logger.warning( + "Bridge marker missing after page load; attempting recovery injection via runJavaScript" + ) + + def after_recovery(_: object) -> None: + # Schedule a recheck with delay + QTimer.singleShot( + 100, lambda: _verify_bridge_loaded("recovery", 1, sequence) + ) + + self.web_view.page().runJavaScript(self._bridge_script_source, after_recovery) + return + + # Multiple recovery attempts with increasing delays + if stage.startswith("recovery") and self._bridge_script_source: + if attempt < 3: # Allow up to 3 recovery attempts (100ms, 250ms, 500ms) + logger.warning( + f"Bridge marker still missing (recovery attempt {attempt}); " + f"retrying with progressive delay" + ) + + def after_retry(_: object) -> None: + # Exponential backoff: 100ms, 250ms, 500ms + delay = int(100 * (1.5 ** (attempt - 1))) + QTimer.singleShot( + delay, + lambda: _verify_bridge_loaded( + "recovery", attempt + 1, sequence + ), + ) + + self.web_view.page().runJavaScript(self._bridge_script_source, after_retry) + return + + # Final recovery attempt: re-register script in page().scripts() after 800ms + if stage.startswith("recovery") and attempt == 3: + logger.error( + "Bridge marker missing after 3 recovery attempts; " + "attempting QWebEngineScript re-registration" + ) + + def after_re_register(_: object) -> None: + # Final check after re-registration + QTimer.singleShot( + 100, lambda: _verify_bridge_loaded("final_check", 1, sequence) + ) + + self._re_register_bridge_script() + self.web_view.page().runJavaScript(self._bridge_script_source, after_re_register) + return + + # All recovery attempts exhausted + logger.error("❌ WebDrop Bridge script failed to inject after all recovery attempts!") + logger.error(" Drag-and-drop functionality is DISABLED") + logger.debug(f" Stage: {stage}, Attempt: {attempt}") + + self.web_view.page().runJavaScript( + "typeof window.__webdrop_intercept_injected !== 'undefined' && window.__webdrop_intercept_injected === true", + check_script, + ) + + # Run verification slightly deferred to avoid races with redirect-heavy loads. + QTimer.singleShot(50, lambda: _verify_bridge_loaded("initial", 1, finished_sequence)) + + def _request_reload(self) -> None: + """Handle toolbar reload clicks with coalescing during active page loads.""" + if self._is_page_loading: + self._pending_reload = True + logger.debug("Reload requested while loading; queued one pending reload") + return + + self._trigger_reload_now() + + def _trigger_reload_now(self) -> None: + """Execute a single reload with bridge script availability check.""" + self._pending_reload = False + # Lock immediately so rapid clicks between reload() and loadStarted don't queue + # additional concurrent reloads. + self._is_page_loading = True + self._ensure_bridge_script_exists(verbose=False) + self.web_view.reload() + + def _ensure_bridge_script_exists(self, verbose: bool = False) -> None: + """Ensure bridge script exists in QWebEngineScript collection (idempotent). + + Checks if the script already exists. If not, adds it. + Never removes/re-adds to avoid race conditions with Qt's injection mechanism. + + This is safer than removing+re-adding because: + - Avoids concurrent access conflicts with Qt's internal injection + - Prevents missing injections during rapid reloads + - Guarantees script is available without timing gaps + + Args: + verbose: If True, use debug logging; otherwise use minimal logging + """ + try: + scripts = self.web_view.page().scripts() + + # Check if script already exists + already_exists = False + for script in scripts.toList(): # type: ignore + if script.name() == "webdrop-bridge": + already_exists = True + if verbose: + logger.debug("Bridge script already exists in page().scripts()") + break + + # If script doesn't exist, add it + if not already_exists and self._bridge_script_source: + new_script = QWebEngineScript() + new_script.setName("webdrop-bridge") + + if sys.platform == "darwin": + new_script.setInjectionPoint(QWebEngineScript.InjectionPoint.DocumentCreation) + else: + new_script.setInjectionPoint(QWebEngineScript.InjectionPoint.Deferred) + + new_script.setWorldId(QWebEngineScript.ScriptWorldId.MainWorld) + new_script.setRunsOnSubFrames(False) + new_script.setSourceCode(self._bridge_script_source) + + scripts.insert(new_script) + logger.debug(f"✓ Added bridge script to collection ({len(self._bridge_script_source)} chars)") + except Exception as e: + logger.error(f"Failed to ensure bridge script exists: {e}") + + def _re_register_bridge_script(self, verbose: bool = False) -> None: + """Force re-registration of bridge script in QWebEngineScript collection. + + Removes old script and re-adds it to ensure it's injected on next page load. + This is a fallback for recovery mechanics when normal injection fails. + + Args: + verbose: If True, use debug logging; otherwise use minimal logging + """ + try: + # Remove old script with same name if it exists + scripts = self.web_view.page().scripts() + removed = False + for script in scripts.toList(): # type: ignore + if script.name() == "webdrop-bridge": + if verbose: + logger.debug("Removing old webdrop-bridge script from page().scripts()") + scripts.remove(script) + removed = True + + # Re-register the script + if self._bridge_script_source: + new_script = QWebEngineScript() + new_script.setName("webdrop-bridge") + + if sys.platform == "darwin": + new_script.setInjectionPoint(QWebEngineScript.InjectionPoint.DocumentCreation) + else: + new_script.setInjectionPoint(QWebEngineScript.InjectionPoint.Deferred) + + new_script.setWorldId(QWebEngineScript.ScriptWorldId.MainWorld) + new_script.setRunsOnSubFrames(False) + new_script.setSourceCode(self._bridge_script_source) + + scripts.insert(new_script) + if verbose or removed: + logger.debug(f"✓ Re-registered webdrop-bridge script ({len(self._bridge_script_source)} chars)") + except Exception as e: + logger.error(f"Failed to re-register bridge script: {e}") def _create_navigation_toolbar(self) -> None: """Create navigation toolbar with Home, Back, Forward, Refresh buttons. @@ -1401,13 +1627,18 @@ class MainWindow(QMainWindow): home_action.triggered.connect(self._navigate_home) # Refresh button - refresh_action = self.web_view.pageAction(self.web_view.page().WebAction.Reload) + refresh_action = toolbar.addAction("") reload_icon_path = self._resolve_toolbar_icon_path( os.getenv("TOOLBAR_ICON_RELOAD", "resources/icons/reload.ico") ) if reload_icon_path is not None: refresh_action.setIcon(QIcon(str(reload_icon_path))) - toolbar.addAction(refresh_action) + else: + refresh_action.setIcon( + self.style().standardIcon(self.style().StandardPixmap.SP_BrowserReload) + ) + refresh_action.setToolTip("Reload") + refresh_action.triggered.connect(self._request_reload) # Open-with-default-app drop zone (right of Reload) self._open_drop_zone = OpenDropZone() @@ -1849,6 +2080,7 @@ class MainWindow(QMainWindow): def _navigate_home(self) -> None: """Navigate to the home (start) URL.""" + self._pending_reload = False home_url = self.config.webapp_url if home_url.startswith("http://") or home_url.startswith("https://"): self.web_view.load(QUrl(home_url)) diff --git a/tests/unit/test_url_converter.py b/tests/unit/test_url_converter.py index 1f2a7a3..8ab5726 100644 --- a/tests/unit/test_url_converter.py +++ b/tests/unit/test_url_converter.py @@ -1,5 +1,6 @@ """Unit tests for URL converter.""" +import os from pathlib import Path import pytest @@ -44,7 +45,10 @@ def test_convert_simple_url(converter): result = converter.convert_url_to_path(url) assert result is not None - assert str(result).endswith("test\\file.png") # Windows path separator + if os.name == "nt": + assert str(result).endswith("test\\file.png") + else: + assert str(result).endswith("test/file.png") def test_convert_url_with_special_characters(converter):