PICARD-3041: Fix QUrl drag-n-drop issue; added path canonicalisation

picard/ui/itemviews/basetreeview.py

@@ -48,6 +48,9 @@
     heappop,
     heappush,
 )
+import re
+from typing import TYPE_CHECKING, cast
+from urllib.parse import unquote
 
 from PyQt6 import (
     QtCore,
@@ -66,6 +69,7 @@
     UnclusteredFiles,
 )
 from picard.config import get_config
+from picard.const.sys import IS_WIN
 from picard.const.tags import ALL_TAGS
 from picard.extension_points.item_actions import (
     ext_point_album_actions,
@@ -82,12 +86,16 @@
     Track,
 )
 from picard.util import (
+    canonicalize_path,
     icontheme,
     iter_files_from_objects,
-    normpath,
     restore_method,
 )
 
+
+if TYPE_CHECKING:  # Avoid runtime circular import
+    from picard.tagger import Tagger
+
 from picard.ui.collectionmenu import CollectionMenu
 from picard.ui.enums import MainAction
 from picard.ui.filter import Filter
@@ -475,16 +483,38 @@ def scrollTo(self, index, scrolltype=QtWidgets.QAbstractItemView.ScrollHint.Ensu
     def drop_urls(urls, target, move_to_multi_tracks=True):
         files = []
         new_paths = []
-        tagger = QtCore.QCoreApplication.instance()
+        # We need to cast to Tagger here to avoid type checking errors below.
+        tagger = cast("Tagger", QtCore.QCoreApplication.instance())
         for url in urls:
             log.debug("Dropped the URL: %r", url.toString(QtCore.QUrl.UrlFormattingOption.RemoveUserInfo))
-            if url.scheme() == 'file' or not url.scheme():
-                filename = normpath(url.toLocalFile().rstrip('\0'))
+            # Accept local files via explicit file:// scheme or scheme-less URLs.
+            # Qt on Windows can misparse "C:\path" as scheme "c" with an empty path.
+            # Detect a leading Windows drive (single letter + ":") in the original (sans user info)
+            # string and treat it as a local path rather than a custom scheme.
+            if (
+                url.scheme() == 'file'
+                or not url.scheme()
+                or (
+                    IS_WIN
+                    and len(url.scheme()) == 1
+                    and re.match(r'^[a-zA-Z]:', url.toString(QtCore.QUrl.UrlFormattingOption.RemoveUserInfo))
+                )
+            ):
+                # Prefer a local file if provided; fall back to treating the URL path as a local filesystem path.
+                # For some Windows inputs (e.g. raw "C:\path"), QUrl.path() can drop the drive ("/Users/..."),
+                # which would cause os.path.abspath to adopt the current drive. Prefer the original string
+                # (without user info) before url.path() to preserve the drive letter.
+                raw_str = url.toString(QtCore.QUrl.UrlFormattingOption.RemoveUserInfo)
+                # Unquote percent-encoding (e.g. "\\" → "%5C") to recover a plain local path
+                local: str = url.toLocalFile() or unquote(raw_str) or url.path()
+                # canonicalize handles trimming NULs, normalization and macOS NFC/NFD resolution
+                filename: str = canonicalize_path(local) if local else ''
                 file = tagger.files.get(filename)
                 if file:
                     files.append(file)
                 else:
-                    new_paths.append(filename)
+                    if filename:
+                        new_paths.append(filename)
             elif url.scheme() in {'http', 'https'}:
                 file_lookup = tagger.get_file_lookup()
                 file_lookup.mbid_lookup(url.path(), browser_fallback=False)

picard/ui/mainwindow/__init__.py

@@ -89,6 +89,7 @@
 from picard.track import Track
 from picard.util import (
     IgnoreUpdatesContext,
+    canonicalize_path,
     icontheme,
     iter_files_from_objects,
     iter_unique,
@@ -927,6 +928,8 @@ def add_files(self):
             filter=";;".join(formats),
         )
         if files:
+            # Canonicalize paths before use
+            files = [canonicalize_path(p) for p in files if p]
             config = get_config()
             config.persist['current_directory'] = os.path.dirname(files[0])
             self.tagger.add_files(files)
@@ -943,12 +946,14 @@ def add_directory(self):
                 dir=current_directory,
             )
             if directory:
-                dir_list.append(directory)
+                dir_list.append(canonicalize_path(directory))
         else:
             dir_list = FileDialog.getMultipleDirectories(
                 parent=self,
                 directory=current_directory,
             )
+            if dir_list:
+                dir_list = [canonicalize_path(d) for d in dir_list if d]
 
         dir_count = len(dir_list)
         if dir_count:

picard/util/__init__.py

@@ -61,7 +61,7 @@
 import ntpath
 from operator import attrgetter
 import os
-from pathlib import PurePath
+from pathlib import Path, PurePath
 import re
 import subprocess  # nosec: B404
 import sys
@@ -254,6 +254,148 @@ def normpath(path, realpath=True):
     return path
 
 
+def _resolve_path_components_macos(path: str) -> str:
+    """Best-effort macOS path component resolution.
+
+    Resolve path components against the filesystem on macOS to account for
+    Unicode normalization differences (NFC vs. NFD). Returns a best‑effort
+    on-disk representation, resolving as many components as possible.
+
+    Parameters
+    ----------
+    path : str
+        The input path to resolve. Can be absolute or relative. The final
+        file/directory does not need to exist.
+
+    Returns
+    -------
+    str
+        A path with components matched to existing directory entries where
+        possible. If resolution fails for any component, the original
+        component is kept. Absolute vs. relative form is preserved.
+
+    Notes
+    -----
+    - This function does not change the drive/volume.
+    - It tolerates missing final components (resolves what exists).
+    - Matching is performed using exact, NFC, and NFD comparisons.
+    """
+
+    def _listdir_safe(directory: Path) -> list[str]:
+        try:
+            return [entry.name for entry in directory.iterdir()]
+        except OSError:
+            return []
+
+    def _match_component(candidate: str, entries: list[str]) -> str:
+        if not entries:
+            return candidate
+        if candidate in entries:
+            return candidate
+        cand_nfc = unicodedata.normalize('NFC', candidate)
+        cand_nfd = unicodedata.normalize('NFD', candidate)
+        for entry in entries:
+            if (
+                entry == cand_nfc
+                or entry == cand_nfd
+                or unicodedata.normalize('NFC', entry) == cand_nfc
+                or unicodedata.normalize('NFD', entry) == cand_nfd
+            ):
+                return entry
+        return candidate
+
+    def _join_resolved(parts: list[str], absolute: bool) -> str:
+        if not parts:
+            return os.sep if absolute else ''
+        if absolute:
+            return str(Path('/').joinpath(*parts))
+        return str(Path().joinpath(*parts))
+
+    try:
+        if not path:
+            return path
+        is_abs = Path(path).is_absolute()
+        parts = [p for p in PurePath(path).parts if p not in ('', '.')]
+        resolved_parts: list[str] = []
+        current_dir = Path('/') if is_abs else Path.cwd()
+
+        for part in parts:
+            entries = _listdir_safe(current_dir)
+            chosen = _match_component(part, entries)
+            resolved_parts.append(chosen)
+            current_dir = current_dir / chosen
+
+        return _join_resolved(resolved_parts, is_abs)
+    except Exception as err:  # noqa: BLE001
+        log.debug("canonicalize components failed for %r: %s", path, err)
+        return str(Path(path))
+
+
+def canonicalize_path(path: str) -> str:
+    """Canonicalize a path for robust I/O.
+
+    Produces a sanitized path suitable for file operations by trimming
+    trailing NULs, normalizing separators, and resolving platform-specific
+    quirks (e.g., Unicode normalization on macOS).
+
+    Parameters
+    ----------
+    path : str
+        The input path as a Unicode string.
+
+    Returns
+    -------
+    str
+        A canonicalized path. On macOS, components are matched against the
+        filesystem using normalization-insensitive rules. On all platforms,
+        the result is normalized and, where possible, resolved via
+        ``os.path.realpath``.
+
+    Notes
+    -----
+    - Trailing ``"\0"`` characters are stripped.
+    - If resolution via ``realpath`` fails, the normalized input is returned.
+    - Non-string inputs are returned unchanged.
+    """
+    if not isinstance(path, str):
+        return path  # type: ignore[return-value]
+
+    # Trim any accidental NUL terminators and normalize separators
+    trimmed = path.rstrip('\0')
+    if not trimmed:
+        return ''
+
+    # Avoid realpath here; resolve components manually on macOS first
+    normalized = str(Path(trimmed))
+
+    # Windows: Qt QUrl without scheme can yield paths like "/C:/foo".
+    # Strip the leading slash so we keep the correct drive when absolutizing.
+    if IS_WIN and re.match(r'^/[A-Za-z]:', normalized):
+        normalized = normalized[1:]
+
+    if IS_MACOS:
+        resolved = _resolve_path_components_macos(normalized)
+        # realpath only if possible, otherwise keep best-effort resolved path
+        try:
+            candidate = str(Path(resolved).resolve(strict=False))
+        except OSError:
+            candidate = resolved
+    else:
+        # On non-macOS platforms avoid resolving symlinks/junctions here, as this can
+        # unexpectedly change drive letters on Windows runners (e.g. C: → D:).
+        # Prefer an absolute, normalized path without crossing filesystem links.
+        candidate = os.path.abspath(os.path.normpath(normalized))
+
+    # Windows: normalize drive letter to uppercase for stable comparisons
+    if IS_WIN and len(candidate) >= 2 and candidate[1] == ':':
+        candidate = candidate[0].upper() + candidate[1:]
+    # Carry over Windows long-path handling from normpath
+    if IS_WIN and not system_supports_long_paths():
+        candidate = win_prefix_longpath(candidate)
+
+    return candidate
+
+
 def win_prefix_longpath(path):
     """
     For paths longer then WIN_MAX_FILEPATH_LEN enable long path support by prefixing with WIN_LONGPATH_PREFIX.