feat: Enhance URL conversion and bridge script handling for improved drag-and-drop functionality
Some checks are pending
Tests & Quality Checks / Test on Python 3.11 (push) Waiting to run
Tests & Quality Checks / Test on Python 3.12 (push) Waiting to run
Tests & Quality Checks / Test on Python 3.11-1 (push) Waiting to run
Tests & Quality Checks / Test on Python 3.12-1 (push) Waiting to run
Tests & Quality Checks / Test on Python 3.10 (push) Waiting to run
Tests & Quality Checks / Test on Python 3.11-2 (push) Waiting to run
Tests & Quality Checks / Test on Python 3.12-2 (push) Waiting to run
Tests & Quality Checks / Build Artifacts (push) Blocked by required conditions
Tests & Quality Checks / Build Artifacts-1 (push) Blocked by required conditions
Some checks are pending
Tests & Quality Checks / Test on Python 3.11 (push) Waiting to run
Tests & Quality Checks / Test on Python 3.12 (push) Waiting to run
Tests & Quality Checks / Test on Python 3.11-1 (push) Waiting to run
Tests & Quality Checks / Test on Python 3.12-1 (push) Waiting to run
Tests & Quality Checks / Test on Python 3.10 (push) Waiting to run
Tests & Quality Checks / Test on Python 3.11-2 (push) Waiting to run
Tests & Quality Checks / Test on Python 3.12-2 (push) Waiting to run
Tests & Quality Checks / Build Artifacts (push) Blocked by required conditions
Tests & Quality Checks / Build Artifacts-1 (push) Blocked by required conditions
This commit is contained in:
parent
9edadc2c16
commit
cbd8ed0186
4 changed files with 275 additions and 37 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
||||
// ============================================================================
|
||||
|
|
|
|||
|
|
@ -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))
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue