From 8e40ca5c4c51bb75237e73f70af419799037609a Mon Sep 17 00:00:00 2001 From: Kyuwon Kim Date: Mon, 19 Oct 2026 22:09:14 +0900 Subject: [PATCH 1/3] Show Browse paths and preserve contents on failed navigation (#121) --- CHANGELOG.md | 8 + Tests/GalleryIntegrationTests.cpp | 216 +++++++++++- Viewer/BrowserDirectoryWin.h | 91 +++++ Viewer/BrowserPathBar.h | 134 ++++++++ Viewer/GalleryGridCanvas.cpp | 3 + Viewer/MainFrm.cpp | 11 + Viewer/ThumbnailPane.cpp | 311 ++++++++++++++---- Viewer/ThumbnailPane.h | 25 +- .../issue-121-browse-path-safety.md | 51 +++ 9 files changed, 785 insertions(+), 65 deletions(-) create mode 100644 Viewer/BrowserDirectoryWin.h create mode 100644 Viewer/BrowserPathBar.h create mode 100644 docs/verification/issue-121-browse-path-safety.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 33fe434..dc70520 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,14 @@ All notable changes to Q1View are documented here. Releases follow [semantic ver ## [Unreleased] +### Changed + +- Windows Viewer: show the current Browse folder above both list and grid views, with compact path text and keyboard-accessible full-path copying; keep parent and drive navigation unchanged. (#121) + +### Fixed + +- Windows Viewer: prepare directory listings before replacing Browse contents, preserving the previous folder, selection, scroll and image on access or device failures; explain explicit failures without repeated background-refresh dialogs and discard superseded navigation results. (#121) + --- ## [2.8.2] — 2026-10-19 diff --git a/Tests/GalleryIntegrationTests.cpp b/Tests/GalleryIntegrationTests.cpp index 357da20..e7e1033 100644 --- a/Tests/GalleryIntegrationTests.cpp +++ b/Tests/GalleryIntegrationTests.cpp @@ -53,6 +53,174 @@ struct GalleryIntegrationTests { while (!predicate() && CGalleryGridCanvas::Now() < end) Pump(.05); Require(predicate(), text); } + void BrowserNavigationTests(CMainFrame* frame, const CString& fixture) { + auto& pane = *frame->mpDrawer; + auto& grid = *pane.mGrid; + auto* doc = static_cast(frame->GetActiveDocument()); + auto* view = static_cast(frame->GetActiveView()); + const CString oldFolder = pane.mFolder; + const auto reader = pane.mReadDirectory; + const auto reporter = pane.mReportNavigation; + const int oldStep = pane.mViewStep; + wchar_t temp[MAX_PATH]; GetTempPathW(MAX_PATH, temp); + CString root; root.Format(L"%sQ1View-navigation-%lu-%llu\\",temp,GetCurrentProcessId(),GetTickCount64()); + CString child = root + L"\xC5EC\xD589 \xC0AC\xC9C4 long folder name\\"; + Require(CreateDirectoryW(root,nullptr) && CreateDirectoryW(child,nullptr), "navigation fixtures use isolated Unicode directories"); + const CString image = root + L"sample.png"; + Require(CopyFileW(fixture,image,TRUE)!=FALSE,"navigation fixture image copied"); + pane.NavigateTo(root); + Await([&] { return !pane.mNavigationBusy && pane.mFolder==root; },"real directory metadata commits asynchronously"); + const CString document = doc->mPathName; + const auto openGeneration = doc->mOpenGeneration; + const float zoom = view->mN; + const int x = view->mXDst, y = view->mYDst; + const auto pixels = view->mStableRgbBufferInfo.addr; + for (int step : {0,1,5}) { + pane.ApplyViewStep(step,false); Pump(.05); + CString path; pane.mPathBar.GetWindowText(path); + Require(path==root && pane.mPathBar.IsWindowVisible() && (pane.mPathBar.GetStyle()&ES_READONLY), + "list and grid retain the exact read-only current Browse path"); + CRect header; pane.mPathHeader.GetWindowRect(header); pane.ScreenToClient(header); + if(step) { CRect canvas; grid.GetWindowRect(canvas); pane.ScreenToClient(canvas); + Require(canvas.top>=header.bottom && canvas.Height()>0,"grid is laid out below the native path header without overlap"); } + else { CRect row; pane.GetItemRect(0,row,LVIR_BOUNDS); + Require(row.top>=header.bottom,"compact rows start below the path header"); } + pane.FocusPath(); + Require(::GetFocus()==pane.mPathBar.m_hWnd,"path is keyboard focusable in both presentations"); + pane.mPathBar.SendMessage(WM_CHAR,'z'); pane.mPathBar.GetWindowText(path); + Require(path==root,"typing cannot edit the path or navigate"); + MSG key{};key.hwnd=pane.mPathBar.m_hWnd;key.message=WM_KEYDOWN;key.wParam=VK_ESCAPE; + Require(pane.mPathBar.PreTranslateMessage(&key) && ::GetFocus()!=pane.mPathBar.m_hWnd,"Escape returns to the list/grid without closing the app"); + } + IAccessible* accessible=nullptr; + Require(SUCCEEDED(AccessibleObjectFromWindow(pane.mPathBar.m_hWnd,OBJID_CLIENT,IID_PPV_ARGS(&accessible))),"native path edit exposes MSAA"); + VARIANT self{};self.vt=VT_I4;self.lVal=CHILDID_SELF; + BSTR name=nullptr,value=nullptr; + accessible->get_accName(self,&name);accessible->get_accValue(self,&value); + Require(name && !wcscmp(name,L"Current folder") && value && root==value,"accessible path has a meaningful name and full unelided value"); + SysFreeString(name);SysFreeString(value);accessible->Release(); + { + CClientDC dc(&pane.mPathBar);CFont* old=dc.SelectObject(&pane.mLabelFont); + const CString longPath=L"D:\\very long ancestor directory\\another ancestor\\\xC5EC\xD589 \xC0AC\xC9C4\\"; + const CString expected=L"D:\\\x2026\\\xC5EC\xD589 \xC0AC\xC9C4"; + SIZE size{};GetTextExtentPoint32W(dc,expected,expected.GetLength(),&size); + Require(CBrowserPathBar::Compact(dc,longPath,size.cx+1)==expected,"compact path preserves the drive and current Unicode folder, eliding ancestors rather than the tail"); + const CString unc=L"\\\\server\\share\\long ancestor\\another ancestor\\current"; + const CString compactUnc=L"\\\\server\\share\\\x2026\\current"; + GetTextExtentPoint32W(dc,compactUnc,compactUnc.GetLength(),&size); + Require(CBrowserPathBar::Compact(dc,unc,size.cx+1)==compactUnc,"UNC compaction preserves share identity and the current leaf"); + for(int width : {10,40,80,120,240}) { + const CString compact=CBrowserPathBar::Compact(dc,longPath,width); + GetTextExtentPoint32W(dc,compact,compact.GetLength(),&size); + Require(size.cx<=width,"even extremely narrow path text remains inside its measured bounds"); + } + dc.SelectObject(old); + for(UINT dpi : {96u,144u,192u}) { + pane.RebuildFonts(dpi); + CClientDC measured(&pane.mPathBar);CFont* previous=measured.SelectObject(&pane.mLabelFont); + TEXTMETRIC metrics{};measured.GetTextMetrics(&metrics);measured.SelectObject(previous); + CRect bounds;pane.mPathBar.GetClientRect(bounds); + Require(bounds.Height()>=metrics.tmHeight,"native path row expands to contain enlarged 100/150/200-percent caption metrics"); + } + pane.RebuildFonts(GetDpiForWindow(pane.m_hWnd)); + } + pane.SetFocus(); + const HWND browseFocus=::GetFocus(); + Require(PostMessageW(browseFocus,WM_KEYDOWN,VK_TAB,1) && PostMessageW(browseFocus,WM_KEYUP,VK_TAB,1|(1L<<30)|(1L<<31)),"physical Tab messages queued from the browser"); + Pump(.05); + Require(pane.IsPathFocused(),"the real MFC message-pump Tab path reaches the path text before splitter traversal"); + { + Microsoft::WRL::ComPtr clipboard; OleGetClipboard(&clipboard); + struct Restore { IDataObject* data; ~Restore(){OleSetClipboard(data);OleFlushClipboard();} } restore{clipboard.Get()}; + pane.FocusPath(); BYTE keyboard[256], saved[256]; GetKeyboardState(saved); memcpy(keyboard,saved,sizeof(keyboard)); keyboard[VK_CONTROL]=0x80; + struct RestoreKeys { BYTE* keys; ~RestoreKeys(){SetKeyboardState(keys);} } keys{saved}; + SetKeyboardState(keyboard); + MSG key{};key.hwnd=pane.mPathBar.m_hWnd;key.message=WM_KEYDOWN;key.wParam='A'; + Require(AfxGetApp()->PreTranslateMessage(&key),"Ctrl+A is handled by path text, not file selection"); + key.wParam='C';Require(AfxGetApp()->PreTranslateMessage(&key),"Ctrl+C uses the focused read-only path"); + Require(OpenClipboard(frame->m_hWnd)!=FALSE,"path copy clipboard opened for verification"); + const HANDLE data=GetClipboardData(CF_UNICODETEXT);const wchar_t* copied=data?static_cast(GlobalLock(data)):nullptr; + const bool exact=copied && root==copied;if(copied) GlobalUnlock(data);CloseClipboard(); + Require(exact,"path clipboard contains the exact full value, not the visually elided string"); + } + pane.SetFocus(); + { + ScopedWindowsUiPaintTestTheme theme; + for(auto appearance : {q1view::WindowsUiAppearance::Light,q1view::WindowsUiAppearance::Dark}) { + q1view::WindowsUiAppearanceState().SetAppearance(appearance); + pane.SendMessage(q1view::WM_UI_APPEARANCE_CHANGED); + auto check=[&](bool okay,const char* text){Require(okay,text);}; + CheckWindowsUiErase(pane.mPathBar.m_hWnd,q1view::WindowsUiColor::SurfaceAlt,true,check); + CheckWindowsUiErase(pane.mPathHeader.m_hWnd,q1view::WindowsUiColor::SurfaceAlt,true,check); + WindowsUiPaintTestSurface surface(pane.mPathBar.m_hWnd); + const auto brush=HBRUSH(pane.mPathHeader.SendMessage(WM_CTLCOLORSTATIC,WPARAM(surface.Dc()),LPARAM(pane.mPathBar.m_hWnd))); + LOGBRUSH logical{}; + Require(brush && GetObjectW(brush,sizeof(logical),&logical) && logical.lbColor==Q1UI_COLOR_SURFACE_ALT, + "focused native path edit uses the same Light/Dark background as its header"); + } + } + pane.SendMessage(q1view::WM_UI_APPEARANCE_CHANGED); + pane.NavigateTo(child); Await([&] { return !pane.mNavigationBusy && pane.mFolder==child; },"accessible empty Unicode folder opens normally"); + Require(pane.mEntries.size()==1 && pane.mEntries.front().kind==CThumbnailPane::ENTRY_PARENT,"empty folder retains its parent rather than being treated as an error"); + pane.GoToParent();Await([&] { return !pane.mNavigationBusy && pane.mFolder==root; },"parent navigation commits with the same worker"); + Require(pane.mSelection.focus>=0 && pane.mEntries[pane.mSelection.focus].path==child,"going up reveals the child directory"); + const CString late=child+L"late.png"; + auto once=std::make_shared>(false); + pane.mReadDirectory=[reader,fixture,late,once](const std::wstring& folder){auto snapshot=reader(folder);if(!once->exchange(true)) CopyFileW(fixture,late,TRUE);return snapshot;}; + pane.NavigateTo(child); + Await([&]{return !pane.mNavigationBusy && pane.mFolder==child && pane.mEntries.size()==2;},"post-registration validation catches a file created after enumeration but before its watch exists"); + pane.mReadDirectory=reader; + pane.GoToParent();Await([&]{return !pane.mNavigationBusy && pane.mFolder==root;},"parent restored after watch-registration race check"); + Require(DeleteFileW(late)!=FALSE,"late-arrival fixture cleaned up"); + int errors=0;CString lastError; + pane.mReportNavigation=[&](const CString& message){++errors;lastError=message;}; + for(DWORD code : {DWORD(ERROR_PATH_NOT_FOUND),DWORD(ERROR_ACCESS_DENIED),DWORD(ERROR_NOT_READY),DWORD(ERROR_DEVICE_NOT_CONNECTED),DWORD(ERROR_READ_FAULT)}) { + pane.mReadDirectory=[code](const std::wstring& folder){q1view::BrowserDirectory result;result.folder=folder;result.files.push_back(L"partial.png");result.error=code;return result;}; + const auto generation=pane.mGen.load();const auto size=pane.mEntries.size();const auto selected=pane.mSelection.focus;const float scroll=grid.mLayout.scroll; + const int before=errors; + pane.NavigateTo(root+L"unavailable\\"); + Await([&] { return !pane.mNavigationBusy; },"injected directory error is delivered through the production request path"); + CString path;pane.mPathBar.GetWindowText(path); + Require(pane.mFolder==root && path==root && pane.mGen==generation && pane.mEntries.size()==size && pane.mSelection.focus==selected && grid.mLayout.scroll==scroll, + "all directory failure types retain committed path, entries, selection, scroll and thumbnail generation"); + Require(errors==before+1 && lastError.Find(L"unavailable")>=0,"explicit failure supplies one actionable message including its target"); + pane.mDirectoryRefreshPending=true;pane.RefreshDirectoryContents(); + Await([&] { return !pane.mNavigationBusy; },"background refresh error completes"); + Require(errors==before+1 && pane.mGen==generation,"background failure neither opens a modal nor invalidates thumbnails"); + } + // Hold an old request while accepting a newer one: no sleeps or device + // disconnects are needed to reproduce a late network completion. + auto entered=std::make_shared>(false); + auto release=std::make_shared>(false); + pane.mReadDirectory=[entered,release,reader](const std::wstring& folder){entered->store(true);while(!release->load()) Sleep(1);auto result=reader(folder);result.error=ERROR_ACCESS_DENIED;return result;}; + pane.NavigateTo(child);Await([&]{return entered->load();},"slow old navigation starts off the UI thread"); + const int previousErrors=errors; + pane.mReadDirectory=reader;pane.NavigateTo(root);release->store(true); + Await([&]{return !pane.mNavigationBusy;},"newest queued request wins after a slow old failure"); + Require(pane.mFolder==root && errors==previousErrors,"stale failures do not replace a newer folder or display an error"); + pane.mReadDirectory=reader; + pane.NavigateTo(root.Left(3));Await([&]{return !pane.mNavigationBusy;},"drive root metadata loads"); + Require(pane.CanGoToParent() && pane.mEntries.front().kind==CThumbnailPane::ENTRY_PARENT,"drive root keeps a visible, applicable parent action"); + pane.GoToParent();Await([&]{return !pane.mNavigationBusy;},"root parent opens drive chooser"); + CString path;pane.mPathBar.GetWindowText(path); + Require(pane.mHasDirectory && pane.mFolder.IsEmpty() && path==L"Drives","drive chooser is explicit rather than an uninitialized empty path"); + Require(!pane.mEntries.empty() && grid.Label(0)==L"["+pane.mEntries[0].path+L"]","drive labels retain their root backslash in bracket notation"); + Require(doc->mPathName==document && doc->mOpenGeneration==openGeneration && view->mStableRgbBufferInfo.addr==pixels && view->mN==zoom && view->mXDst==x && view->mYDst==y, + "browsing success and failure never reopen or transform the displayed image"); + pane.mReportNavigation=reporter;pane.NavigateTo(oldFolder);Await([&]{return !pane.mNavigationBusy;},"original Browse location restored"); + pane.ApplyViewStep(oldStep,false); + { + CThumbnailPane closing; + Require(closing.CreatePane(frame,0x7c11)!=FALSE,"isolated drawer created for blocked-worker shutdown"); + auto started=std::make_shared>(false), unblock=std::make_shared>(false), finished=std::make_shared>(false); + closing.mReadDirectory=[started,unblock,finished](const std::wstring& folder){started->store(true);while(!unblock->load()) Sleep(1);q1view::BrowserDirectory result;result.folder=folder;finished->store(true);return result;}; + closing.NavigateTo(root);Await([&]{return started->load();},"isolated metadata worker is blocked without blocking the UI"); + const auto before=GetTickCount64();closing.DestroyWindow(); + Require(GetTickCount64()-before<1500,"closing a drawer does not join an indefinitely blocked metadata read"); + unblock->store(true);Await([&]{return finished->load();},"detached stopped worker releases its owned request safely"); + } + Require(DeleteFileW(image) && RemoveDirectoryW(child) && RemoveDirectoryW(root),"temporary navigation fixtures removed"); + } void RecycleTests(CMainFrame* frame, const CString& fixture) { auto& pane = *frame->mpDrawer; auto& grid = *pane.mGrid; @@ -172,6 +340,7 @@ struct GalleryIntegrationTests { "directory refresh waits until an in-progress recycle operation finishes"); pane.mRecycleBusy = false; pane.RefreshDirectoryContents(); + Await([&] { return !pane.mNavigationBusy; }, "deferred directory enumeration completes before selection checks"); Require(!pane.mDirectoryRefreshPending && pane.SelectedMediaPaths() == std::vector{files[0], files[1]}, "deferred directory refresh retains selection after recycling finishes"); @@ -218,9 +387,11 @@ struct GalleryIntegrationTests { } pane.mConfirmRecycle = [&](size_t) { pane.NavigateTo(child); return true; }; pane.RecycleSelected(); + Await([&] { return !pane.mNavigationBusy && pane.mFolder == child; }, "confirmation navigation completes without recycling old items"); Require(pane.mFolder == child && GetFileAttributes(files[0]) != INVALID_FILE_ATTRIBUTES, "MFC folder change during confirmation invalidates batch"); - pane.NavigateTo(folder); pane.SelectByPath(files[0]); pane.SelectIndex(indexOf(files[3]), true); + pane.NavigateTo(folder); Await([&] { return !pane.mNavigationBusy && pane.mFolder == folder; }, "recycle fixture folder reopens before choosing files"); + pane.SelectByPath(files[0]); pane.SelectIndex(indexOf(files[3]), true); const unsigned oldGeneration = pane.mGen.load(); pane.ActivateIndex(indexOf(files[0]), true); pane.mConfirmRecycle = [](size_t) { return true; }; @@ -948,6 +1119,22 @@ struct GalleryIntegrationTests { frame->MoveWindow(20, 20, 1200, 850); if (!frame->mDrawerVisible) frame->OnToggleDrawer(); Pump(.4); + wchar_t browserOnly[8] = {}; + if (GetEnvironmentVariableW(L"Q1VIEW_BROWSER_NAVIGATION_ONLY",browserOnly,_countof(browserOnly))) { + wchar_t fixture[MAX_PATH]; GetFullPathNameW(L"Tests\\fixtures\\sample_16x16.png",MAX_PATH,fixture,nullptr); + Require(AfxGetApp()->OpenDocumentFile(fixture)!=nullptr,"repository sample opened for focused navigation checks"); + Pump(.2); + BrowserNavigationTests(frame,fixture); + RecycleTests(frame,fixture); + fprintf(report,"Focused Browse navigation and interaction checks completed\n"); return; + } + // The shell/zoom checks below require a displayed image. A no-argument + // startup run has deliberately just verified the empty -- status. + if (doc->mPathName.IsEmpty()) { + wchar_t fixture[MAX_PATH]; GetFullPathNameW(L"Tests\\fixtures\\sample_16x16.png",MAX_PATH,fixture,nullptr); + Require(AfxGetApp()->OpenDocumentFile(fixture)!=nullptr,"loaded-source shell checks establish their own repository fixture"); + Pump(.2); + } ChromeTests(frame, view); ViewerShellTests(frame, view, doc); CommandRefreshTests(frame); @@ -1149,6 +1336,7 @@ struct GalleryIntegrationTests { // to the image (or back) while the key's remaining messages are still queued. const CString documentBeforePhysicalE = doc->GetPathName(); auto PostPhysicalE = [&]() { + const bool wasVisible = frame->mDrawerVisible; HWND target = ::GetFocus(); Require(target != NULL, "physical E regression has a focused target window"); Require(::PostMessage(target, WM_KEYDOWN, 'E', 1) != FALSE, @@ -1156,7 +1344,11 @@ struct GalleryIntegrationTests { Require(::PostMessage(target, WM_KEYUP, 'E', 1 | (1L << 30) | (1L << 31)) != FALSE, "physical E keyup posted through the Windows message queue"); - Pump(.2); + // A pending metadata commit/thumbnail presentation can consume a + // fixed 200 ms pump before the posted keyboard message is dequeued. + // Wait for that message's actual effect, not for an arbitrary delay. + Await([&] { return frame->mDrawerVisible != wasVisible; }, "physical E reaches the frame through the real message queue"); + Pump(.05); }; PostPhysicalE(); Require(!frame->mDrawerVisible && doc->GetPathName() == documentBeforePhysicalE, @@ -1199,7 +1391,9 @@ struct GalleryIntegrationTests { pane.mEntries.front().kind == CThumbnailPane::ENTRY_DIR, "activating the drive-root parent tile opens the drive chooser"); pane.NavigateTo(driveRoot); Pump(.2); - Require(pane.GoToParent() && pane.mFolder.IsEmpty() && !pane.mEntries.empty() && + Require(pane.GoToParent(), "drive-root parent request accepted"); + Await([&] { return !pane.mNavigationBusy; }, "drive-root parent request completes"); + Require(pane.mFolder.IsEmpty() && !pane.mEntries.empty() && pane.mEntries.front().kind == CThumbnailPane::ENTRY_DIR, "drive chooser opens after navigating above a drive root"); pane.NavigateTo(folder); Pump(.5); @@ -1226,9 +1420,14 @@ struct GalleryIntegrationTests { Require(grid.mDevice != nullptr, "GPU rendering recovers after fallback"); CString missing = folder + L"missing\\"; const auto beforeMissing = pane.mGen.load(); + const auto navigationReporter = pane.mReportNavigation; + int missingErrors = 0; + pane.mReportNavigation = [&](const CString&) { ++missingErrors; }; pane.NavigateTo(missing); Pump(.1); - Require(pane.mGen == beforeMissing && pane.mEntries.size() == 3001, "missing folder leaves navigation and cache intact"); + Await([&] { return !pane.mNavigationBusy; }, "missing-folder result delivered"); + Require(pane.mGen == beforeMissing && pane.mEntries.size() == 3001 && missingErrors == 1, "missing folder leaves navigation and cache intact with one error"); pane.NavigateTo(folder); pane.NavigateTo(missing); pane.NavigateTo(folder); Pump(1); + pane.mReportNavigation = navigationReporter; Require(pane.mOutstanding <= 4, "decode plus posted-result backlog bounded to four"); CRect beforeOpen; frame->GetWindowRect(&beforeOpen); CString expected = pane.mEntries[3].path; @@ -1496,6 +1695,7 @@ struct GalleryIntegrationTests { } for (int mode = 0; mode < pane.ViewStepCount(); ++mode) { pane.NavigateTo(folder); pane.ApplyViewStep(mode, false); + Await([&] { return !pane.mNavigationBusy && pane.mFolder == folder; }, "filtering checks wait for the requested directory snapshot"); const auto containsPath = [&](const CString& path) { return std::any_of(pane.mEntries.begin(), pane.mEntries.end(), [&](const CThumbnailPane::Entry& entry) { return entry.path.CompareNoCase(path) == 0; }); @@ -1533,12 +1733,14 @@ struct GalleryIntegrationTests { }, "video with a meaningful thumbnail remains in the grid"); } pane.NavigateTo(child); pane.ApplyViewStep(mode, false); + Await([&] { return !pane.mNavigationBusy && pane.mFolder == child; }, "child folder opens before testing its parent tile"); Require(!pane.mEntries.empty() && pane.mEntries.front().kind == CThumbnailPane::ENTRY_PARENT && (pane.IsGrid() ? grid.Label(0) == L"[..]" : pane.GetItemText(0, 0) == L"[..]"), "typographic parent entry is first at every thumbnail size"); CMenu parentMenu; pane.BuildContextMenu(parentMenu, 0); Require(parentMenu.GetMenuItemCount() == 0, "parent navigation is represented by the tile, not a context command"); - pane.ActivateIndex(0, true); Pump(.05); + pane.ActivateIndex(0, true); + Await([&] { return !pane.mNavigationBusy && pane.mFolder == folder; }, "parent tile completes its asynchronous navigation"); const int selected = pane.IsGrid() ? grid.Selection() : pane.GetNextItem(-1, LVNI_SELECTED); Require(pane.mFolder == folder && pane.mViewStep == mode && selected >= 0 && pane.mEntries[selected].path == child, "parent tile reveals child folder at every thumbnail size"); @@ -1559,7 +1761,8 @@ struct GalleryIntegrationTests { // Await the actual outcome, retaining the bounded failure timeout. Await([&] { return pane.mFolder == child; }, "folder activation survives a same-folder asynchronous refresh"); MSG up = {}; up.hwnd = pane.GetSafeHwnd(); up.message = WM_KEYDOWN; up.wParam = VK_BACK; - Require(pane.PreTranslateMessage(&up) && pane.mFolder == folder, "Backspace remains a parent-navigation shortcut"); + Require(pane.PreTranslateMessage(&up), "Backspace remains a parent-navigation shortcut"); + Await([&] { return !pane.mNavigationBusy && pane.mFolder == folder; }, "Backspace navigation completes in the parent folder"); CMenu background; pane.BuildContextMenu(background, -1); Require(background.GetMenuItemCount() == 0, "empty background has no redundant parent context command"); Require(view->mIsPlaying && doc->mPathName == media && doc->mOpenGeneration == openGeneration, @@ -1576,6 +1779,7 @@ struct GalleryIntegrationTests { Require(doc->mPathName == media, "stale deferred activation cannot open an item after folder change"); view->KillPlayTimerSafe(); } + BrowserNavigationTests(frame, fixture); RecycleTests(frame, fixture); wchar_t hold[16]; if (GetEnvironmentVariableW(L"Q1VIEW_GALLERY_TEST_HOLD_SECONDS",hold,_countof(hold))) { diff --git a/Viewer/BrowserDirectoryWin.h b/Viewer/BrowserDirectoryWin.h new file mode 100644 index 0000000..cc058aa --- /dev/null +++ b/Viewer/BrowserDirectoryWin.h @@ -0,0 +1,91 @@ +#pragma once + +// Directory metadata only: never decode a source image while navigating. +#include +#include +#include +#include +#include "ViewerFileOrder.h" + +namespace q1view { +struct BrowserDirectory { + std::wstring folder; + std::vector directories, files; + DWORD error = ERROR_SUCCESS; +}; + +inline BrowserDirectory ReadBrowserDirectory(std::wstring folder) +{ + BrowserDirectory result; + if (!folder.empty() && folder.back() != L'\\') folder += L'\\'; + result.folder = folder; + // Do not let removable media generate a second, system-owned modal dialog. + DWORD oldMode = 0; + const BOOL changedMode = SetThreadErrorMode(SEM_FAILCRITICALERRORS, &oldMode); + struct RestoreMode { BOOL changed; DWORD mode; ~RestoreMode() { if (changed) SetThreadErrorMode(mode, nullptr); } } mode{changedMode, oldMode}; + if (folder.empty()) { + DWORD size = GetLogicalDriveStringsW(0, nullptr); + if (!size) { result.error = GetLastError(); return result; } + std::vector drives(size + 1); + DWORD read = GetLogicalDriveStringsW(DWORD(drives.size()), drives.data()); + if (!read || read >= drives.size()) { result.error = read ? ERROR_MORE_DATA : GetLastError(); return result; } + for (const wchar_t* drive = drives.data(); *drive; drive += wcslen(drive) + 1) + result.directories.emplace_back(drive); + return result; + } + // An accessible empty directory is success; a failed wildcard search alone + // cannot distinguish that from an absent/inaccessible directory. + HANDLE directory = CreateFileW(folder.c_str(), FILE_LIST_DIRECTORY, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, nullptr, OPEN_EXISTING, + FILE_FLAG_BACKUP_SEMANTICS, nullptr); + if (directory == INVALID_HANDLE_VALUE) { result.error = GetLastError(); return result; } + struct Close { HANDLE handle; ~Close() { CloseHandle(handle); } } close{directory}; + BY_HANDLE_FILE_INFORMATION info{}; + if (!GetFileInformationByHandle(directory, &info)) { result.error = GetLastError(); return result; } + if (!(info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)) { result.error = ERROR_DIRECTORY; return result; } + WIN32_FIND_DATAW data{}; + HANDLE find = FindFirstFileExW((folder + L"*").c_str(), FindExInfoBasic, &data, FindExSearchNameMatch, nullptr, 0); + if (find == INVALID_HANDLE_VALUE) { + result.error = GetLastError(); + if (result.error == ERROR_FILE_NOT_FOUND) { + const DWORD attributes = GetFileAttributesW(folder.c_str()); + result.error = attributes == INVALID_FILE_ATTRIBUTES ? GetLastError() : + ((attributes & FILE_ATTRIBUTE_DIRECTORY) ? ERROR_SUCCESS : ERROR_DIRECTORY); + } + return result; + } + do { + if (!wcscmp(data.cFileName, L".") || !wcscmp(data.cFileName, L"..")) continue; + if (data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) { + if (!(data.dwFileAttributes & (FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_SYSTEM))) + result.directories.push_back(folder + data.cFileName); + } else result.files.push_back(folder + data.cFileName); + } while (FindNextFileW(find, &data)); + const DWORD error = GetLastError(); + FindClose(find); + if (error != ERROR_NO_MORE_FILES) result.error = error; + FILE_STANDARD_INFO standard{}; + if (!result.error && GetFileInformationByHandleEx(directory, FileStandardInfo, &standard, sizeof(standard)) && standard.DeletePending) + result.error = ERROR_PATH_NOT_FOUND; + const auto less = [](const std::wstring& a, const std::wstring& b) { + return LessFileNameOrdinal(CString(a.c_str()), CString(b.c_str())); + }; + std::sort(result.directories.begin(), result.directories.end(), less); + std::sort(result.files.begin(), result.files.end(), less); + return result; +} + +inline std::wstring BrowserNavigationError(const std::wstring& path, DWORD error) +{ + std::wstring text = L"Can't open folder:\n" + path + L"\n\n"; + switch (error) { + case ERROR_ACCESS_DENIED: return text + L"Access was denied. Check your permissions, then try again."; + case ERROR_FILE_NOT_FOUND: case ERROR_PATH_NOT_FOUND: case ERROR_DIRECTORY: + return text + L"The folder could not be found. Check whether it was moved or deleted, then try again."; + case ERROR_NOT_READY: case ERROR_INVALID_DRIVE: case ERROR_DEVICE_NOT_CONNECTED: + case ERROR_BAD_NETPATH: case ERROR_BAD_NET_NAME: case ERROR_NETWORK_UNREACHABLE: + return text + L"The drive or network location is unavailable. Check its connection, then try again."; + default: return text + L"The folder could not be read. Check the location and try again.\nWindows error: " + std::to_wstring(error); + } +} +} diff --git a/Viewer/BrowserPathBar.h b/Viewer/BrowserPathBar.h new file mode 100644 index 0000000..5c6b8fc --- /dev/null +++ b/Viewer/BrowserPathBar.h @@ -0,0 +1,134 @@ +#pragma once +#include +#include +#include +#include +#include "Q1UiFontWin.h" +#include "QViewerCmn.h" + +#pragma comment(lib, "oleacc.lib") +#pragma comment(lib, "uuid.lib") + +// A real read-only edit retains its complete accessible value even when its +// unfocused presentation is elided. Focus restores native selection/scrolling. +class CBrowserPathBar : public CEdit { + IAccPropServices* properties = nullptr; +public: + ~CBrowserPathBar() { if (properties) properties->Release(); } + static CString Compact(HDC dc, const CString& path, int width) { + auto fits = [&](const CString& text) { + SIZE size{}; return GetTextExtentPoint32W(dc, text, text.GetLength(), &size) && size.cx <= width; + }; + if (fits(path)) return path; + const wchar_t* rootEnd = PathSkipRootW(path); + const int rootLength = rootEnd ? int(rootEnd - path.GetString()) : 0; + CString normalized = path; + while (normalized.GetLength() > rootLength && normalized.Right(1) == L"\\") normalized.Truncate(normalized.GetLength()-1); + const int slash = normalized.ReverseFind(L'\\'); + CString leaf = normalized.Mid(slash+1); + CString prefix; + if (slash >= rootLength && !leaf.IsEmpty()) prefix = path.Left(rootLength) + L"\x2026\\"; + else if (rootLength > 0 && slash == rootLength-1 && !leaf.IsEmpty()) prefix = path.Left(rootLength); + else leaf = normalized; + if (fits(prefix+leaf)) return prefix+leaf; + // A very long UNC root must not consume the space for the current leaf. + if (!prefix.IsEmpty() && !fits(prefix+L"\x2026")) prefix = rootLength>3 ? CString(L"\\\\\x2026\\") : CString(); + int low=0, high=leaf.GetLength(); + while(low0 && low keyHandler; + CToolTipCtrl tooltip; + void SetPath(const CString& value) { + if (!GetSafeHwnd()) return; + SetWindowText(value); + if (tooltip.GetSafeHwnd()) tooltip.UpdateTipText(value, this, 1); + Invalidate(FALSE); + } + BOOL PreTranslateMessage(MSG* message) override { + if (tooltip.GetSafeHwnd()) tooltip.RelayEvent(message); + if (keyHandler && keyHandler(message)) return TRUE; + return CEdit::PreTranslateMessage(message); + } +protected: + LRESULT WindowProc(UINT message, WPARAM wp, LPARAM lp) override { + if (message == WM_ERASEBKGND) { + CRect rect; GetClientRect(rect); + CDC* dc = CDC::FromHandle(HDC(wp)); dc->FillSolidRect(rect, Q1UI_COLOR_SURFACE_ALT); return 1; + } + if (message == WM_DESTROY && properties) { + const MSAAPROPID name = PROPID_ACC_NAME; + properties->ClearHwndProps(m_hWnd, OBJID_CLIENT, CHILDID_SELF, &name, 1); + properties->Release(); properties = nullptr; + } + if (message == WM_PAINT && ::GetFocus() != m_hWnd) { + CPaintDC dc(this); + CRect rect; GetClientRect(rect); + dc.FillSolidRect(rect, Q1UI_COLOR_SURFACE_ALT); + CFont* old = dc.SelectObject(GetFont()); + dc.SetBkMode(TRANSPARENT); dc.SetTextColor(Q1UI_COLOR_TEXT); + rect.DeflateRect(MulDiv(6, GetDpiForWindow(m_hWnd), 96), 0); + CString text; GetWindowText(text); + text = Compact(dc.GetSafeHdc(), text, rect.Width()); + dc.DrawText(text, rect, DT_SINGLELINE | DT_VCENTER | DT_NOPREFIX); + dc.SelectObject(old); + return 0; + } + LRESULT result = CEdit::WindowProc(message, wp, lp); + if (message == WM_SETFOCUS || message == WM_KILLFOCUS) Invalidate(FALSE); + if (message == WM_CREATE) { + tooltip.Create(this, TTS_ALWAYSTIP | TTS_NOPREFIX); + tooltip.SetMaxTipWidth(700); + tooltip.AddTool(this, L"", CRect(0, 0, 1, 1), 1); + // Dynamic annotation leaves the native edit's value/selection intact. + if (SUCCEEDED(CoCreateInstance(CLSID_AccPropServices, nullptr, CLSCTX_INPROC_SERVER, IID_PPV_ARGS(&properties)))) { + properties->SetHwndPropStr(m_hWnd, OBJID_CLIENT, CHILDID_SELF, PROPID_ACC_NAME, L"Current folder"); + } + } + if (message == WM_SIZE && tooltip.GetSafeHwnd()) { + CRect rect; GetClientRect(rect); tooltip.SetToolRect(this, 1, rect); + } + return result; + } +}; + +// Reuse the report list's native header allocation, also above the grid canvas. +// This avoids replacing the splitter, list, scroll coordinate system or focus +// routing just to add one line. The single native column is not interactive. +class CBrowserPathHeader : public CHeaderCtrl { + CBrush brush; + COLORREF color = CLR_INVALID; +protected: + LRESULT WindowProc(UINT message, WPARAM wp, LPARAM lp) override { + if (message == WM_PAINT) { + CPaintDC dc(this); CRect rect; GetClientRect(rect); + dc.FillSolidRect(rect, Q1UI_COLOR_SURFACE_ALT); return 0; + } + if (message == WM_CTLCOLORSTATIC || message == WM_CTLCOLOREDIT || message == WM_ERASEBKGND) { + if (color != Q1UI_COLOR_SURFACE_ALT) { + brush.DeleteObject(); color = Q1UI_COLOR_SURFACE_ALT; brush.CreateSolidBrush(color); + } + if (message == WM_ERASEBKGND) { + CRect rect; GetClientRect(rect); ::FillRect(HDC(wp), rect, HBRUSH(brush.GetSafeHandle())); return 1; + } + ::SetBkColor(HDC(wp), color); ::SetTextColor(HDC(wp), Q1UI_COLOR_TEXT); + return LRESULT(brush.GetSafeHandle()); + } + LRESULT result = CHeaderCtrl::WindowProc(message, wp, lp); + if (message == HDM_LAYOUT && result) { + HDLAYOUT* layout = reinterpret_cast(lp); + CClientDC dc(this); CFont* old = dc.SelectObject(GetFont()); + TEXTMETRIC metrics{}; dc.GetTextMetrics(&metrics); dc.SelectObject(old); + const int height = metrics.tmHeight + MulDiv(10, GetDpiForWindow(m_hWnd), 96); + layout->prc->top += height - layout->pwpos->cy; + layout->pwpos->cy = height; + } + return result; + } +}; diff --git a/Viewer/GalleryGridCanvas.cpp b/Viewer/GalleryGridCanvas.cpp index 71d4a22..8fd7b10 100644 --- a/Viewer/GalleryGridCanvas.cpp +++ b/Viewer/GalleryGridCanvas.cpp @@ -153,6 +153,7 @@ CString CGalleryGridCanvas::Label(int index) const { const auto& entry = mOwner.mEntries[index]; if (entry.kind == CThumbnailPane::ENTRY_PARENT) return _T("[..]"); if (entry.kind == CThumbnailPane::ENTRY_DIR) { + if (mOwner.mFolder.IsEmpty()) return L"[" + entry.path + L"]"; CString path = entry.path; path.TrimRight(_T("\\")); CString name = PathFindFileName(path); name = name.IsEmpty() ? path : name; @@ -166,6 +167,7 @@ CString CGalleryGridCanvas::Tooltip(int index) const { const auto& entry = mOwner.mEntries[index]; if (entry.kind == CThumbnailPane::ENTRY_PARENT) return _T(".."); if (entry.kind == CThumbnailPane::ENTRY_DIR) { + if (mOwner.mFolder.IsEmpty()) return entry.path; CString path = entry.path; path.TrimRight(_T("\\")); CString name = PathFindFileName(path); return name.IsEmpty() ? path : name; @@ -476,6 +478,7 @@ LRESULT CGalleryGridCanvas::OnMouseLeave(WPARAM, LPARAM) { mTracking = false; mH LRESULT CGalleryGridCanvas::OnDpiChanged(WPARAM, LPARAM) { DropDevice(); Relayout(false); return 0; } void CGalleryGridCanvas::OnDestroy() { PauseAnimation(); ClearCache(); DropDevice(); CWnd::OnDestroy(); } BOOL CGalleryGridCanvas::PreTranslateMessage(MSG* message) { + if (message->message == WM_KEYDOWN && message->wParam == VK_TAB) { mOwner.FocusPath(); return TRUE; } if (message->message == WM_KEYDOWN && mOwner.HandleSelectionKey(UINT(message->wParam), (GetKeyState(VK_CONTROL) & 0x8000) != 0, (GetKeyState(VK_SHIFT) & 0x8000) != 0)) return TRUE; CMainFrame *frame = DYNAMIC_DOWNCAST(CMainFrame, AfxGetMainWnd()); diff --git a/Viewer/MainFrm.cpp b/Viewer/MainFrm.cpp index 06a4d6b..80ee245 100644 --- a/Viewer/MainFrm.cpp +++ b/Viewer/MainFrm.cpp @@ -777,6 +777,16 @@ BOOL CMainFrame::PreTranslateMessage(MSG *pMsg) // auto-repeat prevents a held E from rapidly opening and closing the drawer. if (IsRepeatedDrawerShortcut(pMsg)) return TRUE; + if (mpDrawer && mDrawerVisible && pMsg) { + // MFC's frame/splitter Tab and accelerator routing runs before child + // controls. Give the read-only path its native text-input scope first. + if (mpDrawer->IsPathFocused()) return mpDrawer->TranslatePathMessage(pMsg); + const HWND focus = ::GetFocus(); + if (pMsg->message == WM_KEYDOWN && pMsg->wParam == VK_TAB && + (focus == mpDrawer->GetSafeHwnd() || ::IsChild(mpDrawer->GetSafeHwnd(), focus))) { + mpDrawer->FocusPath(); return TRUE; + } + } if (pMsg != NULL && pMsg->message == WM_KEYDOWN && pMsg->wParam == VK_ESCAPE) { CViewerView *pView = DYNAMIC_DOWNCAST(CViewerView, GetActiveView()); if (pView != NULL && pView->IsFullScreen()) { @@ -1203,6 +1213,7 @@ void CMainFrame::SetDrawerVisibleImmediately(bool visible) CDocument *pDoc = GetActiveDocument(); if (pDoc != NULL && !pDoc->GetPathName().IsEmpty()) mpDrawer->SetCurrentFile(pDoc->GetPathName()); + else mpDrawer->SetCurrentFile(nullptr); mpDrawer->SetFocus(); } else if (!visible && pView != NULL && ::IsWindow(pView->GetSafeHwnd())) { // A zero-width drawer can otherwise retain keyboard focus. Return it to diff --git a/Viewer/ThumbnailPane.cpp b/Viewer/ThumbnailPane.cpp index 32d14d4..ad1a44b 100644 --- a/Viewer/ThumbnailPane.cpp +++ b/Viewer/ThumbnailPane.cpp @@ -32,6 +32,12 @@ #pragma comment(lib, "shell32.lib") +// The SDK declares these annotation GUIDs but does not export them from the +// import libraries. Instantiate the two SDK definitions in this TU only. +#include +DEFINE_GUID(CLSID_AccPropServices, 0xb5f8350b, 0x0548, 0x48b1, 0xa6, 0xee, 0x88, 0xbd, 0x00, 0xb4, 0xa5, 0xe7); +DEFINE_GUID(PROPID_ACC_NAME, 0x608d3df8, 0x8128, 0x4aa7, 0xa4, 0x28, 0xf5, 0x5e, 0x49, 0x26, 0x72, 0x91); + #ifdef _DEBUG #define new DEBUG_NEW #endif @@ -52,6 +58,24 @@ static const int kDefaultStep = 0; static const UINT_PTR kScanTimerId = 0x7100; static const UINT_PTR kDirectoryRefreshTimerId = 0x7101; +// The metadata worker owns no pane pointers. A blocked network call cannot +// keep the UI alive or publish a result into a recycled HWND after shutdown. +struct BrowserNavigationState { + struct Request { + unsigned long long id = 0; + CString folder, current; + bool notify = false, restore = false; + std::vector selected; + std::function read; + }; + std::mutex mutex; + std::condition_variable wake; + bool stop = false, pending = false, ready = false; + unsigned long long serial = 0; + Request next, completed; + q1view::BrowserDirectory result; +}; + BEGIN_MESSAGE_MAP(CThumbnailPane, CListCtrl) ON_WM_CREATE() ON_WM_DESTROY() @@ -75,6 +99,7 @@ BEGIN_MESSAGE_MAP(CThumbnailPane, CListCtrl) ON_MESSAGE(WM_THUMB_READY, &CThumbnailPane::OnThumbReady) ON_MESSAGE(WM_DRAWER_ACTIVATE, &CThumbnailPane::OnActivatePosted) ON_MESSAGE(WM_DRAWER_DIRECTORY_CHANGED, &CThumbnailPane::OnDirectoryChanged) + ON_MESSAGE(WM_DRAWER_NAVIGATION_READY, &CThumbnailPane::OnNavigationReady) END_MESSAGE_MAP() CThumbnailPane::CThumbnailPane() @@ -99,6 +124,12 @@ CThumbnailPane::CThumbnailPane() mReportRecycle = [this](const CString& message) { MessageBox(message, _T("Move to Recycle Bin"), MB_OK | MB_ICONINFORMATION); }; + mReadDirectory = q1view::ReadBrowserDirectory; + mReportNavigation = [this](const CString& message) { + const HWND focus = ::GetFocus(); + MessageBox(message, L"Can't open folder", MB_OK | MB_ICONWARNING); + if (::IsWindow(focus) && (focus == m_hWnd || ::IsChild(m_hWnd, focus))) ::SetFocus(focus); + }; } CThumbnailPane::~CThumbnailPane() @@ -138,7 +169,7 @@ BOOL CThumbnailPane::CreatePane(CWnd *pParent, UINT nID) { // Keep the compact report list as the host; the grid has its own child canvas. DWORD style = WS_CHILD | WS_VISIBLE | WS_CLIPCHILDREN | LVS_OWNERDRAWFIXED | - LVS_REPORT | LVS_SHOWSELALWAYS | LVS_NOCOLUMNHEADER; + LVS_REPORT | LVS_SHOWSELALWAYS; return Create(style, CRect(0, 0, 0, 0), pParent, nID); } @@ -160,6 +191,35 @@ int CThumbnailPane::OnCreate(LPCREATESTRUCT lpCreateStruct) SetImageList(&mImages, LVSIL_NORMAL); InsertColumn(0, _T(""), LVCFMT_LEFT, 0); + if (!(mPathHeader.SubclassWindow)(GetHeaderCtrl()->GetSafeHwnd())) return -1; + mPathHeader.ModifyStyle(HDS_BUTTONS | HDS_DRAGDROP, HDS_NOSIZING | WS_CLIPCHILDREN); + if (!mPathBar.Create(WS_CHILD | WS_VISIBLE | WS_TABSTOP | ES_READONLY | ES_AUTOHSCROLL, + CRect(0, 0, 0, 0), &mPathHeader, 1)) return -1; + mPathBar.keyHandler = [this](MSG* message) { return HandlePathKey(message); }; + mPathBar.SetPath(L"No folder"); + mNavigation = std::make_shared(); + const auto navigation = mNavigation; + const HWND target = m_hWnd; + mNavigationThread = std::thread([navigation, target]() { + for (;;) { + BrowserNavigationState::Request request; + { + std::unique_lock lock(navigation->mutex); + navigation->wake.wait(lock, [&] { return navigation->stop || navigation->pending; }); + if (navigation->stop) return; + request = std::move(navigation->next); navigation->pending = false; + } + q1view::BrowserDirectory result; + try { result = request.read(request.folder.GetString()); } + catch (...) { result.folder = request.folder.GetString(); result.error = ERROR_NOT_ENOUGH_MEMORY; } + std::lock_guard lock(navigation->mutex); + if (navigation->stop) return; + if (request.id != navigation->serial) continue; + navigation->completed = std::move(request); + navigation->result = std::move(result); navigation->ready = true; + ::PostMessage(target, WM_DRAWER_NAVIGATION_READY, 0, 0); + } + }); TCHAR module[MAX_PATH] = {}; GetModuleFileName(nullptr, module, _countof(module)); @@ -194,6 +254,8 @@ int CThumbnailPane::OnCreate(LPCREATESTRUCT lpCreateStruct) void CThumbnailPane::RebuildFonts(UINT dpi) { SetFont(nullptr); + if (mPathBar.GetSafeHwnd()) mPathBar.SetFont(nullptr); + if (mPathHeader.GetSafeHwnd()) mPathHeader.SetFont(nullptr); mLabelFont.DeleteObject(); mFolderFont.DeleteObject(); mExtFont.DeleteObject(); @@ -204,6 +266,9 @@ void CThumbnailPane::RebuildFonts(UINT dpi) label.lfQuality = CLEARTYPE_NATURAL_QUALITY; mLabelFont.CreateFontIndirect(&label); SetFont(&mLabelFont); + if (mPathBar.GetSafeHwnd()) mPathBar.SetFont(&mLabelFont); + if (mPathBar.GetSafeHwnd()) mPathBar.SetMargins(MulDiv(6, dpi, 96), MulDiv(6, dpi, 96)); + if (mPathHeader.GetSafeHwnd()) mPathHeader.SetFont(&mLabelFont); label.lfWeight = FW_MEDIUM; mFolderFont.CreateFontIndirect(&label); @@ -212,6 +277,8 @@ void CThumbnailPane::RebuildFonts(UINT dpi) lf.lfWeight = FW_SEMIBOLD; lf.lfQuality = CLEARTYPE_NATURAL_QUALITY; mExtFont.CreateFontIndirect(&lf); + if (GetSafeHwnd()) SetWindowPos(nullptr, 0, 0, 0, 0, SWP_NOMOVE | SWP_NOSIZE | SWP_NOZORDER | SWP_FRAMECHANGED); + LayoutPath(); } LRESULT CThumbnailPane::OnAppearanceChanged(WPARAM, LPARAM) @@ -229,6 +296,7 @@ LRESULT CThumbnailPane::OnAppearanceChanged(WPARAM, LPARAM) } Invalidate(FALSE); if (mGrid && mGrid->GetSafeHwnd()) mGrid->Invalidate(FALSE); + if (mPathBar.GetSafeHwnd()) mPathHeader.RedrawWindow(nullptr, nullptr, RDW_INVALIDATE | RDW_ERASE | RDW_ALLCHILDREN); return 0; } @@ -242,8 +310,9 @@ LRESULT CThumbnailPane::OnDpiChanged(WPARAM, LPARAM) void CThumbnailPane::OnSize(UINT nType, int cx, int cy) { CListCtrl::OnSize(nType, cx, cy); + LayoutPath(); if (mGrid && mGrid->GetSafeHwnd() && IsGrid()) { - mGrid->MoveWindow(0, 0, cx, cy, FALSE); + mGrid->MoveWindow(GridViewport(), FALSE); if (cx <= 0 || cy <= 0) mGrid->PauseAnimation(); return; } @@ -258,6 +327,50 @@ void CThumbnailPane::OnSetFocus(CWnd* oldWnd) if (IsGrid() && mGrid && mGrid->GetSafeHwnd()) mGrid->SetFocus(); } +CRect CThumbnailPane::GridViewport() const +{ + CRect rect; GetClientRect(rect); + if (mPathHeader.GetSafeHwnd()) { + CRect header; mPathHeader.GetWindowRect(header); + rect.top = std::min(rect.bottom, header.Height()); + } + return rect; +} + +void CThumbnailPane::LayoutPath() +{ + if (!mPathBar.GetSafeHwnd() || !mPathHeader.GetSafeHwnd()) return; + CRect rect; mPathHeader.GetClientRect(rect); + rect.DeflateRect(0, MulDiv(4, GetDpiForWindow(m_hWnd), 96)); + if (rect.Height() < 0) rect.bottom = rect.top; + mPathBar.MoveWindow(rect, FALSE); + mPathBar.Invalidate(FALSE); +} + +void CThumbnailPane::FocusPath() +{ + if (mPathBar.IsWindowVisible()) { mPathBar.SetFocus(); mPathBar.SetSel(0, -1); } +} + +bool CThumbnailPane::HandlePathKey(MSG* message) +{ + if (message->message == WM_KEYDOWN) { + const bool control = (GetKeyState(VK_CONTROL) & 0x8000) != 0; + if (control && message->wParam == 'A') { mPathBar.SetSel(0, -1); return true; } + if (control && message->wParam == 'C') { mPathBar.Copy(); return true; } + if ((control && (message->wParam == 'V' || message->wParam == 'X' || message->wParam == 'Z')) || + (message->wParam >= VK_LEFT && message->wParam <= VK_DOWN) || + message->wParam == VK_HOME || message->wParam == VK_END || message->wParam == VK_DELETE || message->wParam == VK_INSERT) + return false; // Native read-only edit keys must not become image commands. + if (message->wParam == VK_ESCAPE || message->wParam == VK_TAB) { SetFocus(); return true; } + if (message->wParam == VK_BACK) { GoToParent(); return true; } + if (message->wParam == VK_RETURN) return true; + } + if (message->message == WM_SYSKEYDOWN && message->wParam == VK_UP) { GoToParent(); return true; } + CMainFrame* frame = DYNAMIC_DOWNCAST(CMainFrame, AfxGetMainWnd()); + return frame && frame->TranslateGlobalAccelerator(message); +} + // --------------------------------------------------------------------------- // View size (Ctrl+wheel): list step 0, grid steps 1..N // --------------------------------------------------------------------------- @@ -306,11 +419,11 @@ void CThumbnailPane::ApplyViewStep(int step, bool persist) // Re-list the folder because the list and grid use different presentation and // tile sizes. Folder entries and their names remain available in both modes. - Populate(mFolder, current); + if (mHasDirectory) CommitDirectory(mDirectorySnapshot, current); if (!selectedPaths.empty()) RestoreSelection(selectedPaths, current); if (IsGrid()) { ShowScrollBar(SB_BOTH, FALSE); - CRect rc; GetClientRect(&rc); mGrid->MoveWindow(rc, FALSE); + mGrid->MoveWindow(GridViewport(), FALSE); mGrid->SetFocus(); } else SetFocus(); } @@ -542,6 +655,8 @@ void CThumbnailPane::DrawItem(LPDRAWITEMSTRUCT dis) CString name; if (e.kind == ENTRY_PARENT) { name = _T("[..]"); + } else if (mFolder.IsEmpty()) { + name = L"[" + e.path + L"]"; } else { name = e.path; if (!name.IsEmpty() && name[name.GetLength() - 1] == _T('\\')) @@ -572,6 +687,22 @@ void CThumbnailPane::OnDestroy() void CThumbnailPane::Shutdown() { + if (mNavigation) { + { + std::lock_guard lock(mNavigation->mutex); + mNavigation->stop = true; mNavigation->pending = false; mNavigation->ready = false; + } + mNavigation->wake.notify_all(); + if (mNavigationThread.joinable()) { + CancelSynchronousIo(mNavigationThread.native_handle()); + // The stopped mailbox owns the worker's remaining lifetime, not a CWnd. + mNavigationThread.detach(); + } + if (GetSafeHwnd()) { + MSG message; + while (::PeekMessage(&message, m_hWnd, WM_DRAWER_NAVIGATION_READY, WM_DRAWER_NAVIGATION_READY, PM_REMOVE)) {} + } + } StopDirectoryWatch(); if (GetSafeHwnd()) KillTimer(kDirectoryRefreshTimerId); @@ -618,8 +749,11 @@ static CString ExtensionOf(const CString &path) void CThumbnailPane::SetCurrentFile(LPCTSTR lpszPath) { - if (!GetSafeHwnd() || lpszPath == NULL || lpszPath[0] == _T('\0')) + if (!GetSafeHwnd()) return; + if (lpszPath == NULL || lpszPath[0] == _T('\0')) { + if (!mHasDirectory && !mNavigationBusy) RequestDirectory(L"", L"", false); return; + } CString path(lpszPath); path.Replace(_T('/'), _T('\\')); @@ -630,13 +764,15 @@ void CThumbnailPane::SetCurrentFile(LPCTSTR lpszPath) CString folder = path.Left(slash + 1); if (folder.CompareNoCase(mFolder) == 0 && !mEntries.empty()) { + if (mNavigationBusy) mDirectoryRefreshPending = true; + CancelNavigation(); if (mDirectoryRefreshPending) RefreshDirectoryContents(); SelectByPath(path); return; } - Populate(folder, path); + RequestDirectory(folder, path, false); } void CThumbnailPane::StartDirectoryWatch(const CString& folder) @@ -695,30 +831,14 @@ LRESULT CThumbnailPane::OnDirectoryChanged(WPARAM wParam, LPARAM) void CThumbnailPane::RefreshDirectoryContents() { - if (!mDirectoryRefreshPending || mRecycleBusy || !GetSafeHwnd() || mFolder.IsEmpty()) + if (!mDirectoryRefreshPending || mRecycleBusy || mNavigationBusy || !GetSafeHwnd() || mFolder.IsEmpty()) return; const CString folder = mFolder; - const auto selectedPaths = SelectedMediaPaths(); const CString focusedPath = mSelection.focus >= 0 && mSelection.focus < static_cast(mEntries.size()) ? mEntries[mSelection.focus].path : CString(); mDirectoryRefreshPending = false; - Populate(folder, focusedPath); - CString restoredFocus = focusedPath; - const bool focusStillExists = std::any_of(mEntries.begin(), mEntries.end(), - [&](const Entry& entry) { return entry.path.CompareNoCase(restoredFocus) == 0; }); - if (!focusStillExists) { - restoredFocus.Empty(); - for (const CString& path : selectedPaths) { - if (std::any_of(mEntries.begin(), mEntries.end(), [&](const Entry& entry) { - return entry.path.CompareNoCase(path) == 0; - })) { - restoredFocus = path; - break; - } - } - } - RestoreSelection(selectedPaths, restoredFocus); + RequestDirectory(folder, focusedPath, false, true); } // Returns the parent folder (with trailing '\\') of a folder, or "" if it has @@ -728,12 +848,100 @@ static CString ParentFolderOf(const CString &folder) return CString(q1view::ParentDirectory(folder.GetString()).c_str()); } -void CThumbnailPane::Populate(const CString &folder, const CString ¤t) +bool CThumbnailPane::Populate(const CString &folder, const CString ¤t) +{ + CancelNavigation(); + const auto snapshot = mReadDirectory(folder.GetString()); + if (snapshot.error) return false; + CommitDirectory(snapshot, current); + return true; +} + +void CThumbnailPane::CancelNavigation() +{ + if (!mNavigation) return; + std::lock_guard lock(mNavigation->mutex); + if (mNavigationThread.joinable()) CancelSynchronousIo(mNavigationThread.native_handle()); + ++mNavigation->serial; mNavigation->pending = false; mNavigation->ready = false; + mNavigationBusy = false; +} + +void CThumbnailPane::RequestDirectory(const CString& folder, const CString& current, bool notify, bool restore) +{ + if (!mNavigation) return; + // A queued click belongs to the old list, even while its replacement is + // still being prepared. A later, new click on the retained list is valid. + if (!restore) { mPendingGeneration = 0; mPendingFolder.Empty(); } + BrowserNavigationState::Request request; + request.folder = folder; request.current = current; request.notify = notify; + request.restore = restore; request.read = mReadDirectory; + if (restore) request.selected = SelectedMediaPaths(); + { + std::lock_guard lock(mNavigation->mutex); + if (mNavigation->stop) return; + // Interrupt a superseded device/UNC read before the worker can take the + // next request. Drivers that support cancellation need not time out first. + if (mNavigationThread.joinable()) CancelSynchronousIo(mNavigationThread.native_handle()); + request.id = ++mNavigation->serial; + mNavigation->next = std::move(request); mNavigation->pending = true; mNavigation->ready = false; + mNavigationBusy = true; + } + mNavigation->wake.notify_one(); +} + +LRESULT CThumbnailPane::OnNavigationReady(WPARAM, LPARAM) +{ + BrowserNavigationState::Request request; + q1view::BrowserDirectory snapshot; + { + std::lock_guard lock(mNavigation->mutex); + if (mNavigation->stop || !mNavigation->ready) return 0; + request = std::move(mNavigation->completed); + snapshot = std::move(mNavigation->result); mNavigation->ready = false; + if (request.id != mNavigation->serial) return 0; + } + mNavigationBusy = false; + if (snapshot.error) { + if (request.notify) mReportNavigation(q1view::BrowserNavigationError(request.folder.GetString(), snapshot.error).c_str()); + if (mDirectoryRefreshPending) RefreshDirectoryContents(); + return 0; + } + if (request.restore && mHasDirectory && snapshot.folder == mDirectorySnapshot.folder && + snapshot.directories == mDirectorySnapshot.directories && snapshot.files == mDirectorySnapshot.files) { + // Watch registration validation and no-op notifications must not reset + // selection, scroll, or thumbnail generations for identical contents. + if (mDirectoryRefreshPending) RefreshDirectoryContents(); + return 0; + } + if (request.restore && request.folder.CompareNoCase(mFolder) == 0) { + request.selected = SelectedMediaPaths(); + request.current = mSelection.focus >= 0 && mSelection.focus < int(mEntries.size()) ? mEntries[mSelection.focus].path : CString(); + } + CommitDirectory(snapshot, request.current); + if (request.restore) { + CString focus = request.current; + if (!std::any_of(mEntries.begin(), mEntries.end(), [&](const Entry& entry) { return entry.path.CompareNoCase(focus) == 0; })) { + focus.Empty(); + for (const auto& path : request.selected) + if (std::any_of(mEntries.begin(), mEntries.end(), [&](const Entry& entry) { return entry.path.CompareNoCase(path) == 0; })) { focus = path; break; } + } + RestoreSelection(request.selected, focus); + } + if (mDirectoryRefreshPending) RefreshDirectoryContents(); + return 0; +} + +void CThumbnailPane::CommitDirectory(const q1view::BrowserDirectory& snapshot, const CString ¤t) { + const CString folder(snapshot.folder.c_str()); + mDirectorySnapshot = snapshot; if (folder.CompareNoCase(mFolder) != 0) { StopDirectoryWatch(); mDirectoryRefreshPending = false; StartDirectoryWatch(folder); + // Cover changes between worker enumeration and watch registration. + // An identical validation snapshot is discarded without rebuilding UI. + mDirectoryRefreshPending = !folder.IsEmpty(); } // Invalidate any in-flight decode tasks from the previous folder. mGen++; @@ -751,6 +959,8 @@ void CThumbnailPane::Populate(const CString &folder, const CString ¤t) // Size the tiles for the current mode/width before (re)building the image list. if (!IsGrid()) { mThumb = kListThumb; ResetImageList(); } mFolder = folder; + mHasDirectory = true; + mPathBar.SetPath(folder.IsEmpty() ? CString(L"Drives") : folder); // Grids are visual media browsers: they retain thumbnail-capable image/video // entries and folder navigation, while the compact list remains the complete @@ -761,9 +971,8 @@ void CThumbnailPane::Populate(const CString &folder, const CString ¤t) if (folder.IsEmpty()) { // The drive chooser is available in both presentation modes. This makes // switching volumes possible even when the drawer is currently a grid. - TCHAR buf[512] = {0, }; - ::GetLogicalDriveStrings(_countof(buf) - 1, buf); - for (TCHAR *d = buf; *d; d += lstrlen(d) + 1) { + for (const auto& drive : snapshot.directories) { + const wchar_t* d = drive.c_str(); if (!grid) { CString label = _T("[") + CString(d) + _T("]"); InsertItem(row, label, -1); @@ -776,28 +985,8 @@ void CThumbnailPane::Populate(const CString &folder, const CString ¤t) // Collect sub-directories and supported files separately. std::vector dirs; std::vector files; - CFileFind finder; - BOOL working = finder.FindFile(folder + _T("*")); - while (working) { - working = finder.FindNextFile(); - if (finder.IsDots()) - continue; - if (finder.IsDirectory()) { - if (!finder.IsHidden() && !finder.IsSystem()) - dirs.push_back(finder.GetFilePath()); - } else { - // List every file, not just images: the main view pages through - // the whole folder (PgUp/PgDn), so the drawer must hold each of - // those files for its selection to stay in sync (issue #84). - files.push_back(finder.GetFilePath()); - } - } - finder.Close(); - - // Match the main view's PgUp/PgDn/Home/End order exactly: both paths now use - // the same ordinal, case-insensitive filename sort helper. - std::sort(dirs.begin(), dirs.end(), q1view::LessFileNameOrdinal); - std::sort(files.begin(), files.end(), q1view::LessFileNameOrdinal); + for (const auto& path : snapshot.directories) dirs.emplace_back(path.c_str()); + for (const auto& path : snapshot.files) files.emplace_back(path.c_str()); // Keep parent navigation first in every mode. A drive/share root also // gets a visible [..] entry; activating it opens the logical-drive @@ -874,12 +1063,12 @@ void CThumbnailPane::NavigateTo(const CString &folder) { // Browse to another folder without changing the main view; selecting a file // there loads it. - if (folder.IsEmpty() || PathIsDirectory(folder)) Populate(folder, _T("")); + RequestDirectory(folder, L"", true); } bool CThumbnailPane::CanGoToParent() const { - return !ParentFolderOf(mFolder).IsEmpty(); + return !mFolder.IsEmpty(); } bool CThumbnailPane::GoToParent() @@ -890,18 +1079,18 @@ bool CThumbnailPane::GoToParent() // A drive/share root has no filesystem parent, but Backspace/Alt+Up can // still return to the logical-drive chooser to switch volumes. if (!child.IsEmpty()) { - Populate(_T(""), child); + RequestDirectory(L"", child, true); return true; } return false; } - if (!PathIsDirectory(parent)) return false; - Populate(parent, child); + RequestDirectory(parent, child, true); return true; } BOOL CThumbnailPane::PreTranslateMessage(MSG* message) { + if (message->message == WM_KEYDOWN && message->wParam == VK_TAB) { FocusPath(); return TRUE; } if (message->message == WM_KEYDOWN && HandleSelectionKey(UINT(message->wParam), (GetKeyState(VK_CONTROL) & 0x8000) != 0, (GetKeyState(VK_SHIFT) & 0x8000) != 0)) return TRUE; CMainFrame *frame = DYNAMIC_DOWNCAST(CMainFrame, AfxGetMainWnd()); @@ -1064,6 +1253,12 @@ void CThumbnailPane::RecycleSelected() if (mRecycleBusy) return; const auto paths = SelectedMediaPaths(); if (paths.empty()) return; + // An explicit action on the retained list pins that list, just like opening + // one of its files. Do not silently ignore Delete during a watcher refresh. + if (mNavigationBusy) { + mDirectoryRefreshPending = !mFolder.IsEmpty(); + CancelNavigation(); + } const CString folder = mFolder; const unsigned generation = mGen.load(); mRecycleBusy = true; @@ -1075,7 +1270,7 @@ void CThumbnailPane::RecycleSelected() pane.QueueVisibleThumbs(); } } finish{*this}; - if (!mConfirmRecycle(paths.size()) || generation != mGen.load() || folder != mFolder) return; + if (!mConfirmRecycle(paths.size()) || mNavigationBusy || generation != mGen.load() || folder != mFolder) return; std::vector before; for (int i = 0; i < int(mEntries.size()); ++i) if (IsMediaEntry(i)) before.push_back(mEntries[i].path); @@ -1272,7 +1467,7 @@ LRESULT CThumbnailPane::OnActivatePosted(WPARAM wParam, LPARAM /*lParam*/) Entry e = mPending; mPendingGeneration = 0; mPendingFolder.Empty(); - if (e.kind != ENTRY_PARENT && GetFileAttributes(e.path) == INVALID_FILE_ATTRIBUTES) return 0; + if (e.kind == ENTRY_FILE && GetFileAttributes(e.path) == INVALID_FILE_ATTRIBUTES) return 0; if (e.kind == ENTRY_FILE) { // Opening routes through CViewerDoc::OnOpenDocument (raw files included). // Thumbnail selection keeps the current window size and fits the image @@ -1443,7 +1638,7 @@ LRESULT CThumbnailPane::OnThumbReady(WPARAM wParam, LPARAM /*lParam*/) const CString selection = selected >= 0 && selected < (int)mEntries.size() ? mEntries[selected].path : CString(); if (!PreviewRejected(rejected)) mRejectedPreviews.push_back(rejected); - Populate(mFolder, selection); + CommitDirectory(mDirectorySnapshot, selection); if (!selectedPaths.empty()) RestoreSelection(selectedPaths, selection); } else { // Supported images remain discoverable even when their pixels are diff --git a/Viewer/ThumbnailPane.h b/Viewer/ThumbnailPane.h index 76e0dcf..bc5a931 100644 --- a/Viewer/ThumbnailPane.h +++ b/Viewer/ThumbnailPane.h @@ -22,8 +22,11 @@ #include #include "QBrowserSelection.h" #include "Q1UiMenuWin.h" +#include "BrowserDirectoryWin.h" +#include "BrowserPathBar.h" class CGalleryGridCanvas; +struct BrowserNavigationState; // Worker -> UI notification carrying a decoded thumbnail (see Result). #define WM_THUMB_READY (WM_APP + 200) @@ -31,6 +34,7 @@ class CGalleryGridCanvas; // (mutating the list inside its own notification handler is unsafe). #define WM_DRAWER_ACTIVATE (WM_APP + 201) #define WM_DRAWER_DIRECTORY_CHANGED (WM_APP + 202) +#define WM_DRAWER_NAVIGATION_READY (WM_APP + 203) class CThumbnailPane : public CListCtrl { @@ -44,6 +48,9 @@ class CThumbnailPane : public CListCtrl virtual LRESULT WindowProc(UINT message, WPARAM wp, LPARAM lp) override; bool GoToParent(); bool CanGoToParent() const; + void FocusPath(); + bool IsPathFocused() const { return ::GetFocus() == mPathBar.GetSafeHwnd(); } + BOOL TranslatePathMessage(MSG* message) { return mPathBar.PreTranslateMessage(message); } void ShowContextMenu(int index, CPoint screenPoint); // Show the folder containing lpszPath (repopulating only when the folder @@ -82,6 +89,7 @@ class CThumbnailPane : public CListCtrl afx_msg LRESULT OnThumbReady(WPARAM wParam, LPARAM lParam); afx_msg LRESULT OnActivatePosted(WPARAM wParam, LPARAM lParam); afx_msg LRESULT OnDirectoryChanged(WPARAM wParam, LPARAM lParam); + afx_msg LRESULT OnNavigationReady(WPARAM, LPARAM); private: q1view::WindowsUiMenus* mContextMenus = nullptr; @@ -109,10 +117,25 @@ class CThumbnailPane : public CListCtrl std::function mConfirmRecycle; std::function mReportRecycle; std::unique_ptr mGrid; + CBrowserPathHeader mPathHeader; + CBrowserPathBar mPathBar; + CRect GridViewport() const; + void LayoutPath(); + bool HandlePathKey(MSG* message); + bool mHasDirectory = false; + q1view::BrowserDirectory mDirectorySnapshot; + std::shared_ptr mNavigation; + std::thread mNavigationThread; + bool mNavigationBusy = false; + std::function mReadDirectory; + std::function mReportNavigation; + void RequestDirectory(const CString& folder, const CString& current, bool notify, bool restore = false); + void CancelNavigation(); + void CommitDirectory(const q1view::BrowserDirectory& snapshot, const CString& current); struct Task { unsigned gen; int index; int size; bool crop; CString path; }; struct Result { unsigned gen; int index; int size; HBITMAP hbmp; }; - void Populate(const CString &folder, const CString ¤t); + bool Populate(const CString &folder, const CString ¤t); void RebuildFonts(UINT dpi); bool PreviewRejected(const CString &path) const; void StepFile(bool next); // PgUp/PgDn: select prev/next file (no open) diff --git a/docs/verification/issue-121-browse-path-safety.md b/docs/verification/issue-121-browse-path-safety.md new file mode 100644 index 0000000..59c3029 --- /dev/null +++ b/docs/verification/issue-121-browse-path-safety.md @@ -0,0 +1,51 @@ +# Issue #121 — Browse path and transactional navigation + +## Scope and approval — 2026-10-19 + +The user approved implementing the reduced [issue #121](https://github.com/chammoru/Q1View/issues/121) design. Work starts from `origin/master` `155844e2e260bc59d7a943f71b191ace5b0a5b78` on `codex/issue-121-browse-path-safety`. The scope is the Windows MFC Viewer's current-folder line and safe directory navigation. Existing parent/drive/UNC navigation, image transforms, source interpretation and autoplay remain contracts. Comparator and Qt are unchanged. No merge, release or issue closure is authorized by this implementation step. + +## Implementation + +- A read-only native Edit occupies the existing report control's header allocation, above both compact rows and the grid canvas. This retains the splitter, thumbnail coordinates within the canvas and document/view routing. Header height follows the current font metrics and DPI padding. Theme-aware header/edit painting avoids native white header bands. +- The native edit retains the exact committed Browse directory as its value. Unfocused painting middle-elides ancestors while preserving the drive/UNC identity and current leaf where space permits. A long leaf is itself elided without splitting UTF-16 surrogate pairs. Focus exposes selectable, horizontally scrollable full text; tooltip and MSAA retain the full string. Dynamic annotation names the edit `Current folder` and is cleared at destruction. Initial `No folder`, actual `Drives`, filesystem roots and normal folders are distinct states. +- Tab from the list/grid focuses the path. Escape returns to the same browser presentation without navigating or leaving fullscreen. Ctrl+A/C select/copy the path, and native caret/edit keys cannot turn into image commands. E, application accelerators and parent shortcuts are retained. Frame-level handling is necessary because MFC splitter/accelerator routing can consume these keys before child PreTranslateMessage. +- Directory metadata is prepared off the UI thread, never by decoding images. A directory handle validates list access; wildcard exhaustion, an accessible empty folder, access denial, missing targets and mid-enumeration failure are distinguished. The previous entries, selection, scroll, thumbnail generations and watcher remain intact until successful publication. Explicit failures report a target, reason and next action through the existing standard message-dialog path. Background failures remain silent. +- One metadata worker has a bounded latest-request mailbox. Superseded results/errors are discarded. Supported synchronous device/UNC operations are cancelled when superseded. Shutdown stops publication under the mailbox lock, cancels synchronous IO, drains notifications and detaches the potentially blocked worker; only shared mailbox/request data survives, never a pane pointer. No UI thread waits for an indefinitely blocked metadata call to return. +- A successful new-folder commit registers its watcher and validates once more to cover changes between enumeration and watch registration. Identical validation/refresh snapshots do not rebuild the list or invalidate thumbnails. Watch changes received during preparation are drained after completion, and current selection is captured at refresh publication, not overwritten with stale selection from request time. View-size changes reuse committed metadata. +- Explicit navigation invalidates deferred clicks on the old list; background refresh preserves the existing same-folder activation contract. A file action on the retained list pins that list and supersedes pending metadata rather than silently dropping Delete. Navigation during recycle confirmation invalidates the old batch before any recycle operation. Existing drive choices keep the root backslash in both rendered presentations. + +## Regression fixes made during verification + +The first ordinary-window observation exposed native white header bands. The header now paints the entire background using its theme role. A later observation showed that combining native end/path ellipsis flags could hide the current leaf; measured middle compaction now has direct drive/Unicode/UNC and narrow-width assertions. + +Native Tab initially moved to the image pane instead of the path. Direct child-handler tests were insufficient: frame-level input routing was corrected and a real posted-message-pump assertion was added. Ctrl+A/C is also verified through the application's actual PreTranslateMessage path, including exact Unicode clipboard contents and restoration of the previous clipboard. + +The asynchronous conversion exposed tests that assumed NavigateTo/GoToParent had already replaced the list when they returned. These tests now await actual bounded completion while retaining their original folder, selection, source and playback assertions. The existing no-argument full test launch also needed to establish a repository image before its loaded-source zoom/status assertions; empty-startup assertions still run separately. + +A fixed 200 ms physical-E pump failed while a posted E key was still waiting in the queue and the folder request remained pending. Queue inspection confirmed unprocessed input, unchanged document, and no held Ctrl/Alt modifiers. The test now awaits the actual toggle with its existing bounded timeout, settles further messages, and still verifies one toggle per keypress, unchanged file, and no auto-repeat retoggle. No E production behavior was weakened to make the test pass. + +Full interaction checks additionally caught Delete being ignored during asynchronous background refresh. Pinning the visible snapshot corrected that behavior while preserving navigation-during-confirmation batch cancellation. Watch registration timing has a deterministic late-file fixture, and blocked-worker shutdown has its own isolated real-pane check. + +## Local evidence + +- Final full Viewer integration run: **1,848 PASS**, exit 0; `Tests/bin/x64/Release/issue-121-gallery.txt`. Includes the new Browse tests and the existing themes, selection, autoplay, RAW interpretation, E/refit/manual-zoom, native keyboard, video seek/EOF, thumbnail filtering/loading/cache and GPU resource recreation paths. Earlier failures were corrected and the full suite rerun, not waived. +- Final empty-startup/RAW-default run: **131 PASS**, exit 0; `Tests/bin/x64/Release/issue-121-startup.txt`. +- Final focused Browse/navigation/interaction run: **356 PASS**, exit 0; `Tests/bin/x64/Release/issue-121-focused.txt`. Includes actual local empty/Unicode directories, drive roots and chooser, full accessible path, compaction, 100/150/200-percent caption metrics in real HWNDs, native Tab routing, exact path clipboard, Light/Dark background probes, five injected directory-error categories with partial-result rejection, silent refresh failures, superseded completion, watch-registration race, blocked-worker shutdown, live filesystem notifications and real recycle/playing-video handle release with temporary fixtures. +- CoreRegressionTests: built Release/x64/v143 and passed. +- WindowsUiTypographyTests: passed, including shared theme/command/paint/menu/frame checks and four DPI by five text-scale font cases; GDI objects remained 53 before and after. +- GalleryLayoutTests: passed for 1,000 and 1,000,000 entries. +- Instrumented and ordinary Viewer Release/x64/v143 builds passed. Binaries are under `Viewer/x64/Issue121Tests/` and `Viewer/x64/Issue121Review/`. + +Reports are ignored machine-local output, not private media or tracked test artifacts. `CHANGELOG.md` contains one logical line per new bullet under Unreleased. The local version remains `0.0.0-dev`. + +## Native observation and limits + +Computer Use inspected the ordinary Windows app on this PC: a drive chooser labeled Drives, root D:\ with [..], bracketed root choices, and a repository sample image with the Browse directory independent of the document. The corrected header has no white bands. Actual Tab exposed the full accessible `Current folder` value and native text selection; Escape restored the compact presentation without changing the image or 100.0% zoom. Final-build follow-up observations are recorded below as completed. + +The final ordinary binary was relaunched after all production changes and the repository sample was opened through File's observed first MRU item. Browse visibly displays `D:\…\fixtures` without native white bands, while the accessible value remains the exact `D:\80_sources\Q1View\Tests\fixtures\`. The source remains `sample_16x16.png`, Auto / 100.0%. This review window is left open for the user. Actual OS DPI is 96 with 100% text size. + +The existing installed Store Viewer was left untouched. The standard Open dialog exposed stale structured focus information, so visible native filename-field focus was checked before literal repository-path entry; the sample opened through the ordinary user path. No private image was used as a test fixture. + +No live drive was forcibly disconnected, no user folder permissions were changed, and no Windows DPI/text-size/high-contrast settings were changed. Device/permission/mid-read errors and blocked/stale completion are deterministic integration injections through the real request/publication path, not claims of testing every physical device or network provider. The new row's enlarged-font HWND checks are not actual 125/150/200-percent OS DPI acceptance. Shared high-contrast mapping is covered by common tests, but a new native OS high-contrast transition was not performed. Existing watcher registration and provider-specific cancellation behavior are not a hard real-time guarantee for every UNC provider. + +The issue remains open for user review. No version bump, master merge, release tag or Store submission is part of this work. From b976026029c4293a4e0dce0ae32cd889ea8e1730 Mon Sep 17 00:00:00 2001 From: Kyuwon Kim Date: Mon, 19 Oct 2026 23:27:28 +0900 Subject: [PATCH 2/3] Distinguish Browse location headers and finish validation (#121) --- QVisionCore/Q1UiAppearanceWin.cpp | 4 +- QVisionCore/Q1UiAppearanceWin.h | 2 +- Tests/ComparerTypographyIntegrationTests.cpp | 14 ++++- Tests/GalleryIntegrationTests.cpp | 53 +++++++++++++++---- Tests/WindowsUiTypographyTests.cpp | 6 ++- Viewer/BrowserPathBar.h | 37 +++++++------ Viewer/ThumbnailPane.cpp | 6 ++- .../issue-121-browse-path-safety.md | 25 +++++++++ 8 files changed, 115 insertions(+), 32 deletions(-) diff --git a/QVisionCore/Q1UiAppearanceWin.cpp b/QVisionCore/Q1UiAppearanceWin.cpp index e9628da..44486a5 100644 --- a/QVisionCore/Q1UiAppearanceWin.cpp +++ b/QVisionCore/Q1UiAppearanceWin.cpp @@ -20,12 +20,12 @@ WindowsUiPalette ResolveWindowsUiPalette(bool dark) RGB(24,24,24), RGB(32,32,32), RGB(38,38,38), RGB(48,48,48), RGB(56,56,56), RGB(69,69,69), RGB(119,129,142), RGB(242,242,242), RGB(183,190,199), RGB(138,180,255), RGB(24,24,24), RGB(38,54,80), RGB(234,196,106), - RGB(255,154,164), RGB(125,217,170), RGB(24,24,24), RGB(242,242,242), 0,0,0 + RGB(255,154,164), RGB(125,217,170), RGB(24,24,24), RGB(242,242,242), 0,0,0, RGB(48,48,48) } : WindowsUiPalette{ RGB(243,244,246), RGB(255,255,255), RGB(247,248,250), RGB(238,240,243), RGB(228,231,236), RGB(215,218,223), RGB(123,132,144), RGB(32,33,36), RGB(88,97,110), RGB(37,102,217), RGB(255,255,255), RGB(234,241,255), RGB(135,90,0), - RGB(180,35,50), RGB(19,109,69), RGB(247,248,250), RGB(32,33,36), 0,0,0 + RGB(180,35,50), RGB(19,109,69), RGB(247,248,250), RGB(32,33,36), 0,0,0, RGB(233,236,240) }; // One theme controls chrome and the exposed image surround together. colors[size_t(WindowsUiColor::Canvas)] = dark ? RGB(24,24,24) : RGB(236,236,236); diff --git a/QVisionCore/Q1UiAppearanceWin.h b/QVisionCore/Q1UiAppearanceWin.h index aaa1d56..09bdab0 100644 --- a/QVisionCore/Q1UiAppearanceWin.h +++ b/QVisionCore/Q1UiAppearanceWin.h @@ -14,7 +14,7 @@ enum class WindowsUiAppearance { System, Light, Dark }; enum class WindowsUiColor { Window, Surface, SurfaceAlt, Hover, Pressed, Border, Boundary, Text, Muted, Accent, OnAccent, Selection, Warning, Danger, Success, Overlay, OverlayText, - Canvas, CanvasText, CanvasMuted, Count + Canvas, CanvasText, CanvasMuted, LocationHeader, Count }; using WindowsUiPalette = std::array(WindowsUiColor::Count)>; diff --git a/Tests/ComparerTypographyIntegrationTests.cpp b/Tests/ComparerTypographyIntegrationTests.cpp index 73f97ae..27b9847 100644 --- a/Tests/ComparerTypographyIntegrationTests.cpp +++ b/Tests/ComparerTypographyIntegrationTests.cpp @@ -399,11 +399,23 @@ int RunComparerTypographyTests() { } const float savedN = doc->mN, savedD = doc->mD, savedX = doc->mXOff, savedY = doc->mYOff; auto* view = doc->mPane[0].pView; + // This checks menu repaint/layout, not the fit/100-percent stop. + // Restoring a differently sized CI desktop can leave the opening fit + // below 1 even when the image now fits at 1; zoom-out at 1 is then a + // legitimate no-op. Start above that boundary and still require every + // real wheel action to change magnification in the correct direction. + doc->mN = 1.f; doc->mD = ZOOM_DELTA(doc->mN); + doc->ViewOnMouseWheel(-WHEEL_DELTA, doc->mW, doc->mH); + Check(doc->mN == 1.f, "zoom-out at 100 percent legitimately stops when the image already fits"); + doc->mN = 2.f; doc->mD = ZOOM_DELTA(doc->mN); doc->setDstSize(); + for (int slot = 0; slot < CComparatorDoc::IMG_VIEW_MAX; ++slot) + if (doc->mPane[slot].pView) doc->mPane[slot].pView->DeterminDestOriginCoord(doc); + frame->UpdateMagnication(doc->mN); doc->UpdateAllViews(nullptr); Pump(20); CPoint center(view->mWCanvas / 2, view->mRcControls.bottom + view->mHCanvas / 2); view->ClientToScreen(¢er); for (int repeat = 0; repeat < 4; ++repeat) for (short delta : {short(WHEEL_DELTA), short(-WHEEL_DELTA)}) { const float previousN = doc->mN; view->OnMouseWheel(0, delta, center); Pump(5); - Check(doc->mN != previousN, "real image-wheel handler still zooms in and out"); + Check(delta > 0 ? doc->mN > previousN : doc->mN < previousN, "real image-wheel handler still zooms in and out"); CRect helpAfter; ::GetWindowRect(help, helpAfter); Check(helpAfter == helpBefore, "real wheel handler does not move Help"); } diff --git a/Tests/GalleryIntegrationTests.cpp b/Tests/GalleryIntegrationTests.cpp index e7e1033..75ae83a 100644 --- a/Tests/GalleryIntegrationTests.cpp +++ b/Tests/GalleryIntegrationTests.cpp @@ -81,6 +81,10 @@ struct GalleryIntegrationTests { Require(path==root && pane.mPathBar.IsWindowVisible() && (pane.mPathBar.GetStyle()&ES_READONLY), "list and grid retain the exact read-only current Browse path"); CRect header; pane.mPathHeader.GetWindowRect(header); pane.ScreenToClient(header); + CRect pathBounds; pane.mPathBar.GetWindowRect(pathBounds); pane.ScreenToClient(pathBounds); + const int separator=CBrowserPathHeader::SeparatorThickness(GetDpiForWindow(pane.mPathHeader.m_hWnd)); + Require(pathBounds.top>=header.top && pathBounds.bottom=header.bottom && canvas.Height()>0,"grid is laid out below the native path header without overlap"); } else { CRect row; pane.GetItemRect(0,row,LVIR_BOUNDS); @@ -100,7 +104,7 @@ struct GalleryIntegrationTests { Require(name && !wcscmp(name,L"Current folder") && value && root==value,"accessible path has a meaningful name and full unelided value"); SysFreeString(name);SysFreeString(value);accessible->Release(); { - CClientDC dc(&pane.mPathBar);CFont* old=dc.SelectObject(&pane.mLabelFont); + CClientDC dc(&pane.mPathBar);CFont* old=dc.SelectObject(pane.mPathBar.GetFont()); const CString longPath=L"D:\\very long ancestor directory\\another ancestor\\\xC5EC\xD589 \xC0AC\xC9C4\\"; const CString expected=L"D:\\\x2026\\\xC5EC\xD589 \xC0AC\xC9C4"; SIZE size{};GetTextExtentPoint32W(dc,expected,expected.GetLength(),&size); @@ -117,8 +121,16 @@ struct GalleryIntegrationTests { dc.SelectObject(old); for(UINT dpi : {96u,144u,192u}) { pane.RebuildFonts(dpi); - CClientDC measured(&pane.mPathBar);CFont* previous=measured.SelectObject(&pane.mLabelFont); + CClientDC measured(&pane.mPathBar);CFont* previous=measured.SelectObject(pane.mPathBar.GetFont()); TEXTMETRIC metrics{};measured.GetTextMetrics(&metrics);measured.SelectObject(previous); + LOGFONT pathFont{}, rowFont{}; + pane.mPathBar.GetFont()->GetLogFont(&pathFont); pane.mLabelFont.GetLogFont(&rowFont); + Require(pathFont.lfWeight==FW_MEDIUM && rowFont.lfWeight==FW_NORMAL && + pathFont.lfHeight==rowFont.lfHeight && !lstrcmp(pathFont.lfFaceName,rowFont.lfFaceName) && + pane.mPathBar.GetFont()->GetSafeHandle()==pane.mFolderFont.GetSafeHandle(), + "location header reuses medium Pretendard at the same caption size without changing regular item text"); + Require(CBrowserPathHeader::SeparatorThickness(dpi)==q1view::WindowsUiPixels(1,dpi), + "location boundary remains one DIP at 100/150/200-percent DPI"); CRect bounds;pane.mPathBar.GetClientRect(bounds); Require(bounds.Height()>=metrics.tmHeight,"native path row expands to contain enlarged 100/150/200-percent caption metrics"); } @@ -149,14 +161,35 @@ struct GalleryIntegrationTests { for(auto appearance : {q1view::WindowsUiAppearance::Light,q1view::WindowsUiAppearance::Dark}) { q1view::WindowsUiAppearanceState().SetAppearance(appearance); pane.SendMessage(q1view::WM_UI_APPEARANCE_CHANGED); - auto check=[&](bool okay,const char* text){Require(okay,text);}; - CheckWindowsUiErase(pane.mPathBar.m_hWnd,q1view::WindowsUiColor::SurfaceAlt,true,check); - CheckWindowsUiErase(pane.mPathHeader.m_hWnd,q1view::WindowsUiColor::SurfaceAlt,true,check); - WindowsUiPaintTestSurface surface(pane.mPathBar.m_hWnd); - const auto brush=HBRUSH(pane.mPathHeader.SendMessage(WM_CTLCOLORSTATIC,WPARAM(surface.Dc()),LPARAM(pane.mPathBar.m_hWnd))); - LOGBRUSH logical{}; - Require(brush && GetObjectW(brush,sizeof(logical),&logical) && logical.lbColor==Q1UI_COLOR_SURFACE_ALT, - "focused native path edit uses the same Light/Dark background as its header"); + for(int step : {0,1,5}) { + pane.ApplyViewStep(step,false); Pump(.05); + auto check=[&](bool okay,const char* text){Require(okay,text);}; + CheckWindowsUiErase(pane.mPathBar.m_hWnd,q1view::WindowsUiColor::LocationHeader,true,check); + WindowsUiPaintTestSurface surface(pane.mPathBar.m_hWnd); + for(UINT message : {UINT(WM_CTLCOLORSTATIC),UINT(WM_CTLCOLOREDIT)}) { + const auto brush=HBRUSH(pane.mPathHeader.SendMessage(message,WPARAM(surface.Dc()),LPARAM(pane.mPathBar.m_hWnd))); + LOGBRUSH logical{}; + Require(brush && GetObjectW(brush,sizeof(logical),&logical) && + logical.lbColor==q1view::WindowsUiColorValue(q1view::WindowsUiColor::LocationHeader) && GetTextColor(surface.Dc())==Q1UI_COLOR_TEXT, + "focused and unfocused native path keep the neutral header surface and readable normal text color"); + } + WindowsUiPaintTestSurface header(pane.mPathHeader.m_hWnd); + const int thickness=CBrowserPathHeader::SeparatorThickness(GetDpiForWindow(pane.mPathHeader.m_hWnd)); + for(UINT message : {UINT(WM_ERASEBKGND),UINT(WM_PRINTCLIENT)}) { + header.Fill(RGB(17,34,51)); + pane.mPathHeader.SendMessage(message,WPARAM(header.Dc()),PRF_CLIENT); + GdiFlush(); + Require(GetPixel(header.Dc(),2,0)==q1view::WindowsUiColorValue(q1view::WindowsUiColor::LocationHeader) && + GetPixel(header.Dc(),2,header.rect.bottom-thickness-1)==q1view::WindowsUiColorValue(q1view::WindowsUiColor::LocationHeader) && + GetPixel(header.Dc(),2,header.rect.bottom-thickness)==Q1UI_COLOR_BORDER_SOFT && + GetPixel(header.Dc(),header.rect.right-2,header.rect.bottom-1)==Q1UI_COLOR_BORDER_SOFT, + "header paint and erase draw exactly one full-width bottom separator, without per-row or focus outlines"); + } + const DWORD gdi=GetGuiResources(GetCurrentProcess(),GR_GDIOBJECTS); + for(int i=0;i<100;++i) pane.mPathHeader.SendMessage(WM_PRINTCLIENT,WPARAM(header.Dc()),PRF_CLIENT); + Require(GetGuiResources(GetCurrentProcess(),GR_GDIOBJECTS)==gdi, + "repeated location header painting allocates no persistent GDI objects"); + } } } pane.SendMessage(q1view::WM_UI_APPEARANCE_CHANGED); diff --git a/Tests/WindowsUiTypographyTests.cpp b/Tests/WindowsUiTypographyTests.cpp index 97b454f..027511d 100644 --- a/Tests/WindowsUiTypographyTests.cpp +++ b/Tests/WindowsUiTypographyTests.cpp @@ -75,9 +75,12 @@ static void TestAppearance() for (bool dark : {false,true}) { const auto colors = ResolveWindowsUiPalette(dark); auto color = [&](WindowsUiColor role) { return colors[size_t(role)]; }; - for (auto background : {WindowsUiColor::Window,WindowsUiColor::Surface,WindowsUiColor::SurfaceAlt,WindowsUiColor::Selection}) + for (auto background : {WindowsUiColor::Window,WindowsUiColor::Surface,WindowsUiColor::SurfaceAlt,WindowsUiColor::Selection,WindowsUiColor::LocationHeader}) for (auto text : {WindowsUiColor::Text,WindowsUiColor::Muted}) Require(Contrast(color(text),color(background)) >= 4.5, "Light/Dark normal and supporting text meet 4.5:1 contrast"); + Require(color(WindowsUiColor::LocationHeader)!=color(WindowsUiColor::SurfaceAlt) && + color(WindowsUiColor::LocationHeader)!=color(WindowsUiColor::Selection), + "location header has a distinct neutral surface rather than list or selection styling"); for (auto text : {WindowsUiColor::OverlayText,WindowsUiColor::Warning,WindowsUiColor::Danger,WindowsUiColor::Success}) Require(Contrast(color(text),color(WindowsUiColor::Overlay)) >= 4.5, "overlay and semantic status text meet 4.5:1 contrast"); Require(Contrast(color(WindowsUiColor::Boundary),color(WindowsUiColor::Surface)) >= 3.0 && @@ -94,6 +97,7 @@ static void TestAppearance() "System/Light/Dark resolve the entire theme including canvas together"); } Require(WindowsUiSystemColor(WindowsUiColor::Canvas)==COLOR_WINDOW && WindowsUiSystemColor(WindowsUiColor::Text)==COLOR_WINDOWTEXT && + WindowsUiSystemColor(WindowsUiColor::LocationHeader)==COLOR_WINDOW && WindowsUiSystemColor(WindowsUiColor::Border)==COLOR_WINDOWTEXT && WindowsUiSystemColor(WindowsUiColor::Selection)==COLOR_HIGHLIGHT && WindowsUiSystemColor(WindowsUiColor::OnAccent)==COLOR_HIGHLIGHTTEXT, "high-contrast roles defer to Windows system colors"); struct Profile { diff --git a/Viewer/BrowserPathBar.h b/Viewer/BrowserPathBar.h index 5c6b8fc..09d44a5 100644 --- a/Viewer/BrowserPathBar.h +++ b/Viewer/BrowserPathBar.h @@ -60,7 +60,8 @@ class CBrowserPathBar : public CEdit { LRESULT WindowProc(UINT message, WPARAM wp, LPARAM lp) override { if (message == WM_ERASEBKGND) { CRect rect; GetClientRect(rect); - CDC* dc = CDC::FromHandle(HDC(wp)); dc->FillSolidRect(rect, Q1UI_COLOR_SURFACE_ALT); return 1; + CDC* dc = CDC::FromHandle(HDC(wp)); + dc->FillSolidRect(rect, q1view::WindowsUiColorValue(q1view::WindowsUiColor::LocationHeader)); return 1; } if (message == WM_DESTROY && properties) { const MSAAPROPID name = PROPID_ACC_NAME; @@ -70,7 +71,7 @@ class CBrowserPathBar : public CEdit { if (message == WM_PAINT && ::GetFocus() != m_hWnd) { CPaintDC dc(this); CRect rect; GetClientRect(rect); - dc.FillSolidRect(rect, Q1UI_COLOR_SURFACE_ALT); + dc.FillSolidRect(rect, q1view::WindowsUiColorValue(q1view::WindowsUiColor::LocationHeader)); CFont* old = dc.SelectObject(GetFont()); dc.SetBkMode(TRANSPARENT); dc.SetTextColor(Q1UI_COLOR_TEXT); rect.DeflateRect(MulDiv(6, GetDpiForWindow(m_hWnd), 96), 0); @@ -102,23 +103,29 @@ class CBrowserPathBar : public CEdit { // This avoids replacing the splitter, list, scroll coordinate system or focus // routing just to add one line. The single native column is not interactive. class CBrowserPathHeader : public CHeaderCtrl { - CBrush brush; - COLORREF color = CLR_INVALID; +public: + static int SeparatorThickness(UINT dpi) { return q1view::WindowsUiPixels(1, dpi); } protected: + void Paint(HDC dc) { + CRect rect; GetClientRect(rect); + CDC* target = CDC::FromHandle(dc); + target->FillSolidRect(rect, q1view::WindowsUiColorValue(q1view::WindowsUiColor::LocationHeader)); + // A structural boundary, not a selection/focus outline. Leave the + // native edit inset above it so text selection cannot cover the line. + rect.top = (std::max)(rect.top, rect.bottom - SeparatorThickness(GetDpiForWindow(m_hWnd))); + target->FillSolidRect(rect, Q1UI_COLOR_BORDER_SOFT); + } LRESULT WindowProc(UINT message, WPARAM wp, LPARAM lp) override { if (message == WM_PAINT) { - CPaintDC dc(this); CRect rect; GetClientRect(rect); - dc.FillSolidRect(rect, Q1UI_COLOR_SURFACE_ALT); return 0; + CPaintDC dc(this); Paint(dc.GetSafeHdc()); return 0; } - if (message == WM_CTLCOLORSTATIC || message == WM_CTLCOLOREDIT || message == WM_ERASEBKGND) { - if (color != Q1UI_COLOR_SURFACE_ALT) { - brush.DeleteObject(); color = Q1UI_COLOR_SURFACE_ALT; brush.CreateSolidBrush(color); - } - if (message == WM_ERASEBKGND) { - CRect rect; GetClientRect(rect); ::FillRect(HDC(wp), rect, HBRUSH(brush.GetSafeHandle())); return 1; - } - ::SetBkColor(HDC(wp), color); ::SetTextColor(HDC(wp), Q1UI_COLOR_TEXT); - return LRESULT(brush.GetSafeHandle()); + if (message == WM_PRINTCLIENT || message == WM_ERASEBKGND) { + Paint(HDC(wp)); return message == WM_ERASEBKGND ? 1 : 0; + } + if (message == WM_CTLCOLORSTATIC || message == WM_CTLCOLOREDIT) { + ::SetBkColor(HDC(wp), q1view::WindowsUiColorValue(q1view::WindowsUiColor::LocationHeader)); + ::SetTextColor(HDC(wp), Q1UI_COLOR_TEXT); + return LRESULT(q1view::WindowsUiColorBrush(q1view::WindowsUiColor::LocationHeader)); } LRESULT result = CHeaderCtrl::WindowProc(message, wp, lp); if (message == HDM_LAYOUT && result) { diff --git a/Viewer/ThumbnailPane.cpp b/Viewer/ThumbnailPane.cpp index ad1a44b..3a09046 100644 --- a/Viewer/ThumbnailPane.cpp +++ b/Viewer/ThumbnailPane.cpp @@ -266,11 +266,13 @@ void CThumbnailPane::RebuildFonts(UINT dpi) label.lfQuality = CLEARTYPE_NATURAL_QUALITY; mLabelFont.CreateFontIndirect(&label); SetFont(&mLabelFont); - if (mPathBar.GetSafeHwnd()) mPathBar.SetFont(&mLabelFont); if (mPathBar.GetSafeHwnd()) mPathBar.SetMargins(MulDiv(6, dpi, 96), MulDiv(6, dpi, 96)); - if (mPathHeader.GetSafeHwnd()) mPathHeader.SetFont(&mLabelFont); label.lfWeight = FW_MEDIUM; mFolderFont.CreateFontIndirect(&label); + // The location is a header, not another selectable entry. Reuse the + // medium-weight caption font without introducing another GDI font cache. + if (mPathBar.GetSafeHwnd()) mPathBar.SetFont(&mFolderFont); + if (mPathHeader.GetSafeHwnd()) mPathHeader.SetFont(&mFolderFont); LOGFONT lf = q1view::WindowsUiLogFont(q1view::WindowsUiFontRole::Supporting, dpi, q1view::WindowsUiSettings().Scale()); lstrcpyn(lf.lfFaceName, mFontFamily, LF_FACESIZE); diff --git a/docs/verification/issue-121-browse-path-safety.md b/docs/verification/issue-121-browse-path-safety.md index 59c3029..c80bffd 100644 --- a/docs/verification/issue-121-browse-path-safety.md +++ b/docs/verification/issue-121-browse-path-safety.md @@ -49,3 +49,28 @@ The existing installed Store Viewer was left untouched. The standard Open dialog No live drive was forcibly disconnected, no user folder permissions were changed, and no Windows DPI/text-size/high-contrast settings were changed. Device/permission/mid-read errors and blocked/stale completion are deterministic integration injections through the real request/publication path, not claims of testing every physical device or network provider. The new row's enlarged-font HWND checks are not actual 125/150/200-percent OS DPI acceptance. Shared high-contrast mapping is covered by common tests, but a new native OS high-contrast transition was not performed. Existing watcher registration and provider-specific cancellation behavior are not a hard real-time guarantee for every UNC provider. The issue remains open for user review. No version bump, master merge, release tag or Store submission is part of this work. + +## Current-location visual hierarchy follow-up — 2026-10-19 + +The user approved the compact, single-line location-header proposal after reviewing file-manager examples. The previous header used the same surface and foreground as the folder list. It now uses a dedicated neutral `LocationHeader` surface (`#E9ECF0` in Light, `#303030` in Dark), the existing medium-weight 13-DIP Pretendard caption font and one full-width, one-DIP soft bottom separator. The native edit retains its existing vertical inset, leaving space above that boundary. No Location label, breadcrumb links, icons, extra buttons, per-row rules or persistent blue location highlight were added. Folder/drive notation and item selection are unchanged. + +The header's paint, background erase and print-client paths share the same fill/boundary drawing. Focused native Edit background brushes use the same semantic surface as unfocused painting, without creating per-paint GDI objects. High contrast maps the header to Windows `COLOR_WINDOW`, readable text and the separator to `COLOR_WINDOWTEXT`; no custom neutral overrides system high-contrast colors. The shared palette role is appended so existing role values and Comparator styling remain unchanged. + +- Release/x64/v143 instrumented and ordinary Viewer builds passed; the ordinary review binary remains `Viewer/x64/Issue121Review/Viewer.exe`. +- Focused Browse/navigation/interaction suite: **397 PASS**, exit 0; `Tests/bin/x64/Release/issue-121-header-focused.txt`. Added checks cover compact/list and small/large grid geometry, header font reuse and enlarged font metrics, exact Light/Dark erase/print-client separator pixels, native edit background/text colors, stable GDI counts during repeated painting and unchanged full-path accessibility/copy/keyboard/navigation behavior. Existing failure, watcher, source/transform preservation and recycle/playback checks also pass. +- WindowsUiTypographyTests passed, including normal/supporting text contrast on the new Light/Dark surface, distinct location/list/selection surfaces and system high-contrast role mapping. GDI objects remained **55 before / 55 after**. CoreRegressionTests passed. +- Computer Use inspected the ordinary Light-theme Viewer with the repository `sample_16x16.png`: the compact `D:\…\fixtures` location band is visibly separated from `[..]`, `[raw]` and file rows. The exact accessible path remains `D:\80_sources\Q1View\Tests\fixtures\`; source sampling/zoom remains Auto / 100.0%. + +The full 1,848-check suite above belongs to the preceding implementation; this visual-only follow-up reran the focused suite and shared/core tests, not the entire gallery/GPU suite. No Windows DPI, text-size or high-contrast settings were changed. Light/Dark drawing is tested in actual app HWNDs, while a live OS high-contrast transition remains outside this follow-up. The branch is still unmerged and unreleased. + +## Release approval and final local validation — 2026-10-19 + +After reviewing the ordinary header appearance, the user explicitly approved committing, pushing and releasing this work. The release is prepared as Windows patch version `v2.8.3`, with the current Browse feature and transactional navigation fixes in the matching changelog section and the Store description updated. The local generated version stays `0.0.0-dev`; Actions generates the release version from the tag. + +The preceding PR's Gallery workflow had failed in the existing Comparer wheel/menu test, not in Browse or its build. That test assumed every wheel-down must change magnification. `ViewOnMouseWheel` intentionally stays at 100 percent if the image already fits, so a desktop-dependent initial fit is not a valid baseline for this repaint/layout check. The regression now explicitly verifies that legitimate stop, starts the actual view-wheel loop at 200 percent and requires strict increase/decrease on every wheel action, then restores all saved zoom/pan inputs. No Comparator production zoom behavior is changed. + +- Final Viewer full actual integration rerun: **1,925 PASS**, exit 0; `Tests/bin/x64/Release/issue-121-release-gallery.txt`. This supersedes the earlier follow-up's focused-only validation limitation, while the OS/device limitations remain. +- Final Comparer actual integration: **780 PASS**, exit 0; `Tests/bin/x64/Release/issue-121-release-comparer.txt`. Includes the explicit 100-percent stop, actual wheel direction/menu-geometry checks and the existing theme, source-state, A/B/C/D metrics, ROI, sequence and LPIPS/backend checks. +- Ordinary Comparator Release/x64/v143 build passed; `Tests/bin/x64/Release/issue-121-release-normal-comparer-build.log`. Ordinary/instrumented Viewer and instrumented Comparator builds passed. CoreRegressionTests and WindowsUiTypographyTests passed as recorded above; the focused Browse result remains 397 PASS. + +Tagged GitHub Actions remains authoritative for packaging, signing, GitHub Release publication and Store submission. The user requested that the assistant start the release workflow without waiting for it to finish; release initiation must not be reported as completed asset publication or Store availability. From e0bc62e910cc2e5257e54d07f124ba68faedf125 Mon Sep 17 00:00:00 2001 From: Kyuwon Kim Date: Mon, 19 Oct 2026 23:27:28 +0900 Subject: [PATCH 3/3] Prepare v2.8.3 Browse path and navigation safety release --- CHANGELOG.md | 9 +++++++-- docs/STORE_LISTING.md | 2 ++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc70520..fa6239a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,9 +6,13 @@ All notable changes to Q1View are documented here. Releases follow [semantic ver ## [Unreleased] +--- + +## [2.8.3] — 2026-10-19 + ### Changed -- Windows Viewer: show the current Browse folder above both list and grid views, with compact path text and keyboard-accessible full-path copying; keep parent and drive navigation unchanged. (#121) +- Windows Viewer: show the current Browse folder in a distinct, medium-weight header with a neutral background and thin separator above both list and grid views, with compact path text and keyboard-accessible full-path copying; keep parent and drive navigation unchanged. (#121) ### Fixed @@ -543,7 +547,8 @@ Native display checks passed at 100–175%; 200% layout/font calculations passed --- -[Unreleased]: https://github.com/chammoru/Q1View/compare/v2.8.2...HEAD +[Unreleased]: https://github.com/chammoru/Q1View/compare/v2.8.3...HEAD +[2.8.3]: https://github.com/chammoru/Q1View/compare/v2.8.2...v2.8.3 [2.8.2]: https://github.com/chammoru/Q1View/compare/v2.8.1...v2.8.2 [2.8.1]: https://github.com/chammoru/Q1View/compare/v2.8.0...v2.8.1 [2.8.0]: https://github.com/chammoru/Q1View/compare/v2.7.28...v2.8.0 diff --git a/docs/STORE_LISTING.md b/docs/STORE_LISTING.md index ae1e070..2075f60 100644 --- a/docs/STORE_LISTING.md +++ b/docs/STORE_LISTING.md @@ -55,6 +55,8 @@ Viewer also supports image-sequence navigation, video playback, frame/time progr BT.2020 HLG videos are tone-mapped for accurate color and contrast on standard SDR Windows displays instead of being shown with a washed-out legacy color conversion. +The Windows Browse pane displays its current folder in a distinct, read-only path header above both list and gallery views. Long paths keep the current folder visible where space permits, with full-path tooltips and keyboard selection/copy. Directory listings are prepared before replacing the current view; failed navigation preserves the previous folder, selection, scroll and displayed image, with guidance for explicit failures and no repeated background-refresh dialogs. + In the Windows Viewer, right-click a single thumbnail to open it, copy the file or its path, reveal it in File Explorer, view Windows properties, or move it to the Recycle Bin; right-click a selection of two to four media files and choose Compare selected files to open them in Comparator, or move a larger selection to the Recycle Bin. Delete performs the same confirmed recycle operation. Gallery grids focus on useful image and video previews, while compact mode keeps the complete folder list. Directories use clean **[Folder name]** and **[..]** notation with balanced Pretendard typography instead of decorative folder artwork; long names wrap across the card and use an ellipsis only on the final visible line. Every non-empty folder, including a drive root, starts with a visible **[..]** entry; activating it from `D:\` opens the top-level logical-drive chooser so another volume such as `C:\` can be selected. Backspace and Alt+Up provide the same navigation without interrupting the photo or video in the main view, while Viewer shortcuts remain available when the drawer has focus. Choose Auto, Smooth (Bilinear), or Pixel Exact (Nearest) image scaling from Options > Image Scaling. Auto keeps scanned text clear at moderate reductions, suppresses aliasing when large photos are reduced substantially, and preserves exact pixels during close inspection.