merge with master 201909 - #11
Open
livius2 wants to merge 891 commits into
Open
Conversation
…later deal with the fact that StartNode is nil
…ped(() from TVTHeader.HandleMessage(()
…_VTHEADERREFERENCE. Issue #806
…es CF_VTHEADERREFERENCE. Issue #806
…drop. This fixes issue #1254
…nstead of using the image center. Issue #1254
…ragImage(). Issue #1254,
…rag(). Removed some unused members fromTVTDragImage. Issue #1254
… of Resource file
… correct results even if some ancestor of the last visible node is effectively not visible itself Changes from commit 8df8d6 for issue #725 performance reintroduced a regression, that prevents method 'TBaseVirtualTree.GetLastVisibleNoInit' from returning a visible node if some of its ancestors are hidden. That is because methods 'TBaseVirtualTree.GetLastVisibleChildNoInit' and 'GetPreviousVisibleSiblingNoInit' both only consider direct children of given 'Node' but ignore other descendant nodes' visibility. - completely revised method 'TBaseVirtualTree.GetLastVisibleNoInit' - kept the top-down-traversal algorithm of commit 8df8d6 in particular for optimal performance - iteratively checked expansion state (but not visibility) of every ancestor of given 'Node' in the first place to prevent a false-positive result in an effectively collapsed subtree - added conditional recursion for expanded nodes having some visible children (i.e. using fast 'vsAllChildrenHidden' state) as a local function 'IterateChildren' - apply 'ChildrenAbove' option on the conditions' precedence: - for top-down trees prefer visible children over their visible parent - for bottom-up trees prefer a visible node over its visible children - therefore extracted main condition as a local function 'GetNodeIsVisible' and recursion condition as a local function 'GetNodeHasVisibleChildren' ! thus a visible subnode under some effectively invisible but expanded ancestor will be found again
ToggleSelection() - the Shift+Arrow path - removes nodes via InternalRemoveFromSelection(). That routine only *marks* the entry in FSelection (it sets the low bit of the pointer, which is what PackArray later looks for) but fires DoRemoveFromSelection() and Change() straight away. FSelectionCount is not corrected until PackArray runs after the loop, so every handler invoked in between sees a count that is too high by the number of nodes already dropped. Iterating SelectedNodes gives the right answer at the same moment because vsSelected has been cleared, which is exactly the discrepancy reported. FSelectionCount cannot simply be decremented when marking: it is also the length PackArray scans, so lowering it early would leave marked entries in the array. Instead the pending marks are counted and subtracted in GetSelectedCount, and SelectedCount now reads that getter instead of the raw field. Internal callers keep using FSelectionCount directly, so the physical bookkeeping is unchanged. Event order is deliberately left alone - Change() is called from InternalRemoveFromSelection() on purpose, see the comment referring to #1047. The five line pack-and-resize block that appeared at seven call sites is now PackSelection(), which also resets the pending counter. Centralising it is what keeps that counter from drifting, since resetting it at seven places is easy to forget. It returns whether the array was shortened, which is what InvertSelection used its local flag for. Adds Tests/VTSelectedCountIssue1197Tests.pas: one test asserts the count seen during OnRemoveFromSelection, a second asserts the count after the operation so a future change cannot over-correct. Verified both ways - with the fix the suite is 136 passed / 2 failed, reverting only the getter change puts it back to 135 / 3. The two remaining failures are the pre-existing TestCopyHTML1 and TestCopyHTML2. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…_PRINT
The header lives in the non-client area, and two separate mistakes made rendering
the tree into a caller supplied device context unusable.
WMPaint always fetched a window DC via GetDCEx to draw the header, ignoring the DC
that comes with the message. TWinControl.PaintTo performs WM_PAINT with the target
DC, so the client area ended up in the copy while the header was painted onto the
real window instead - the copy had no header at all. It now paints into Message.DC
when one is supplied.
That alone put the header two pixels off: PaintTo draws the border itself and then
moves the origin inside it, while FHeaderRect is relative to the outer window
corner, so the header was shifted by the border width and clipped on the opposite
edge. csPaintCopy marks exactly that case, and GetBorderDimensions returns negative
values, so adding them shifts back.
WMPrint drew the header regardless of the PRF_ flags, so a PRF_CLIENT only request
got the non-client header painted over the client area, which is the corrupted
border the reporter saw. It now checks PRF_NONCLIENT.
Measured on Delphi 13.1 / Win32 by rendering into an off-screen bitmap and counting
the pixels of a distinctively coloured header, so the result does not depend on
window visibility or theming:
before after
PaintTo 0 px 304 px, (362,2)-(377,20)
WM_PRINT, PRF_CLIENT only 38 px 0 px
WM_PRINT, PRF_CLIENT or PRF_NONCLIENT 304 px 304 px, unchanged
WM_PRINT, PRF_NONCLIENT 304 px 304 px, unchanged
PaintTo is now pixel identical to WM_PRINT with PRF_NONCLIENT.
Tests/VTPaintToIssue632Tests.pas covers all four cases. Three of them fail without
this change and one passes either way, guarding against over-correcting the case
that already worked. Suite goes from 135 passed / 5 failed to 138 passed / 2
failed; the two remaining failures are the pre-existing TestCopyHTML1 and
TestCopyHTML2, which also fail on an unmodified master here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
With the explorer theme active, DetermineLineImageAndSelectLevel() suppresses the tree line under the expand button by overwriting the last line image entry with ltNone. The band conversion in PaintTreeLines() however relies on the documented invariant that ltNone never appears as the last entry: on ltNone it takes over the style of the entry to the right, which for the last entry is an out-of-bounds read of the dynamic array (raises ERangeError with range checking on). The resulting garbage styles made the bands of every collapsed node with children disappear, while expanded nodes stayed intact - matching the screenshot in the issue. Skip the suppression in band mode: bands are box edges, not lines pointing at a button, so there is nothing to suppress. Measured offscreen via pixel counting (see new regression test): identical band rendering with and without the explorer theme state after the fix; before, 53 of 1954 band pixels were missing in the test scenario. Test suite: 2 pre-existing failures (TestCopyHTML1/2, also failing on unmodified master), no new failures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…a mapping mode SetCanvasOrigin() transformed its shift through LPtoDP before passing it to SetWindowOrgEx. SetWindowOrgEx however expects logical units - the same units the tree calculates with - so on a canvas with a mapping mode (the issue uses MM_ANISOTROPIC with a 2x viewport) the shift was scaled twice and every node was drawn at twice its offset: node contents, tree lines, buttons and grid lines all drifted apart, exactly as shown in the issue. With the default MM_TEXT mapping the transformation was a no-op, which is why the ordinary paint paths never showed the problem. Pass the shift to SetWindowOrgEx untransformed. Measured offscreen (2x MM_ANISOTROPIC): after the fix the unbuffered rendering places all grid lines and tree lines pixel-identical to the buffered rendering, and exactly at twice the unmapped positions; the MM_TEXT rendering is byte-identical to the one before the fix. New regression test Tests/VTPaintTreeIssue1074Tests.pas asserts both properties; without the fix both assertions fail. Test suite: 2 pre-existing failures (TestCopyHTML1/2, also failing on unmodified master), no new failures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…t, with or without explorer theme Without the explorer theme the focus rect was drawn around InnerRect (or CellRect with toGridExtensions) only, although toFullRowSelect selects and highlights the entire row. Both the draw condition and the rectangle choice in PrepareCell were gated on tsUseExplorerTheme; in addition RowRect was only computed when the explorer theme was active. Compute RowRect unconditionally and drop the two tsUseExplorerTheme conditions. Each cell draws the row-wide rect clipped to its own rectangle, so the XOR-based DrawFocusRect touches every pixel exactly once - measured: the dotted perimeter pixel count matches a single 420x18 rectangle (436 vs. expected 438) with no cancelled segments. Measured offscreen: without columns the focus rect now spans the whole client width, with three columns it spans all columns instead of just the focused one; the explorer-themed rendering is pixel-identical to before. New regression test Tests/VTFocusRectIssue765Tests.pas asserts both spans; without the fix both assertions fail. Test suite: 2 pre-existing failures (TestCopyHTML1/2, also failing on unmodified master), no new failures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ng during keyboard navigation Keyboard navigation sets the focused node twice: WMKeyDown moves the focus (which fires OnFocusChanged), then AddToSelection() assigns the - by now unchanged - focused node again. SetFocusedNode() always ran DoFocusNode(), which begins by ending a node edit. An edit started by the application inside OnFocusChanged was therefore ended again right away by that redundant assignment. With the mouse the event order differs, which is why the same handler worked there. Fix as suggested in the issue discussion: SetFocusedNode() exits early when the node is already focused, so the property setter has no side effects for a no-op assignment. DoFocusNode() itself is unchanged, its deliberate same-node handling (scroll into view) remains available to its direct callers. New regression test Tests/VTFocusChangedIssue1379Tests.pas covers the minimal contract (re-assigning the focused node keeps tsEditing) and the reported scenario (edit started in OnFocusChanged survives a VK_DOWN); both fail without the fix. Test suite: 2 pre-existing failures (TestCopyHTML1/2, also failing on unmodified master), no new failures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rea and were trapped there Dropping a normal column in front of a fixed column made it fixed (deliberate behavior of TVirtualTreeColumn.SetPosition when a column enters the fixed area) - and since issue #1314 fixed columns lose coDraggable, so the column could never be dragged out again. Redirect the drop target in TVTHeader.DragTo(): when a non-fixed column is dragged over a fixed one, the target becomes the first non-fixed visible column instead. Drop mark and drop are consistent and the column lands right after the fixed area, staying normal and draggable. The programmatic path is unchanged: assigning Position directly still moves a column into the fixed area and makes it fixed. New regression test Tests/VTFixedColumnDragIssue1377Tests.pas drives DragIndex/DragTo/ColumnDropped directly and asserts the redirected target, the resulting position and the preserved options; it fails without the fix. A second test pins the unchanged programmatic behavior. Test suite: 2 pre-existing failures (TestCopyHTML1/2, also failing on unmodified master), no new failures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fix #632: header is missing from PaintTo and drawn for client-only WM_PRINT
…-zip Remove accidentally committed repro project from the repository root
…drag Fixed issue #1377: Normal columns could be dropped inside the fixed area and were trapped there
Fixed issue #1091: Bands disappear with explorer theme
Fixed issue #1074: PaintTree with poUnbuffered misplaces nodes under a mapping mode
Fix solid black tree on Windows 7 without DWM composition (Delphi 12), closes #1389
…ount Fix #1197: SelectedCount is stale while selection change events run
Fixed issue #765: Focus rect covers the whole row with toFullRowSelect, independent of the explorer theme
Fixed issue #1379: Redundant focused-node assignment ended node editing during keyboard navigation
In the classic (non-themed) paint path DrawBackground fills the area right of the last column with Header.Background, but PaintColumnHeader painted the column cells via DrawEdge with BF_MIDDLE, which always fills the interior with clBtnFace - a custom Header.Background only ever showed up in the filler area. The cells now get filled explicitly with Header.Background before the edges are drawn. For the default clBtnFace the result is pixel-identical, and the themed and VCL-styles paths are untouched. Tests/VTHeaderBackgroundTests.pas renders the header offscreen and checks both directions: cells follow a custom Header.Background (fails without the fix) and the default rendering keeps the clBtnFace look (guards the no-change promise). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…assic Fixed classic header cells ignoring Header.Background
Screen-reader support was effectively off unless the application added VirtualTrees.Accessibility to a uses clause itself: nothing linked that unit in Delphi (only a C++Builder $HPPEMIT pragma in VirtualTrees.BaseTree referenced it), so its initialization - which registers the default IAccessible providers - never ran and WM_GETOBJECT returned no tree accessible. Reference VirtualTrees.Accessibility from the VirtualTrees umbrella unit's implementation uses so the providers register automatically. The IAccessible objects are still created lazily on WM_GETOBJECT, so there is no cost until an accessibility client attaches. With this, keyboard navigation is announced by screen readers out of the box (verified with NVDA); previously the EVENT_OBJECT_FOCUS notifications from DoFocusChange had no registered accessible to act on. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ity-autoregister Register the accessibility providers by default (issue #873)
The header is non-client area, so hovering it produces WM_NCMOUSEMOVE.
Measured against the VCL hint pipeline, header tooltips failed for three
independent reasons:
1. While the application's hint window is the stock THintWindow, its
IsHintMsg cancels the pending hint on every WM_NCMOUSEMOVE pulled
from the message queue. The header only re-armed the hint pipeline
(Application.HintMouseMessage) when the hover COLUMN changed, so any
further movement inside the same column killed the pending hint for
good. Header tooltips only worked reliably after a virtual tree hint
had been shown, because then TVirtualTreeHintWindow.IsHintMsg
filtered the cancellation.
2. Re-arming must not happen while the cursor is inside LastHintRect
(a hint was already accepted for the header): showing the hint
window generates a synthesized WM_NCMOUSEMOVE at the unchanged
cursor position, the re-entered pipeline calls ActivateHint
immediately (CMHintShowPause reports zero pause for hmToolTip),
the LastHintRect short-circuit in CMHintShow rejects the re-show,
and TApplication.ActivateHint reacts to the rejection with
CancelHint, hiding the hint that just appeared.
3. LastHintRect is only cleared by CM_MOUSELEAVE, which the tree
receives after the mouse visited its CLIENT area. Entering and
leaving the header alone never cleared it, so no header hint would
show on the next visit ("works again after selecting and scrolling
a row", as reported in the issue).
Fix, entirely local to TVTHeader (no application-global changes):
re-arm the hint pipeline on every WM_NCMOUSEMOVE inside the header
unless the cursor is inside LastHintRect, and clear a header-band
LastHintRect (Top < 0 in client coordinates) from the header's existing
mouse-leave detection timer.
The regression tests drive the header's message handler directly and
observe the pipeline via CM_HINTSHOWPAUSE; without the fix three of the
four tests fail. The end-to-end behaviour (real cursor, real message
queue, counting visible hint windows) was verified with a deterministic
measurement harness: unpatched, a header hover with in-column movement
shows no hint and hovering after a foreign control's hint shows no hint
either; with this change both show, while foreign control hints remain
untouched - stable over repeated runs on Delphi 13.1, compiled clean on
10 Seattle Win32 and 13.1 Win32/Win64.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fixed unreliable header tooltips (issue #728)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
14 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.