wxWidgets

I believe this has been committed, hasn't it?

Looks fine to me now, thanks for the update!

Hi Vadim,

Thanks for your comments. I think your approach is better. I went ahead
and implemented all of your points into an updated patch which I have just
attached.

Let me know if this works for you and then I'll apply it.

Best,
Ben

File Added: vista2.patch

It would be indeed great to fix the problem under Vista so yes, please go
ahead and commit vista.patch. However if you have time it would be nice to
change it slightly: AFAICS all items are supposed to be locked all the time
and only one of them can be unlocked at any given moment because it's
always relocked immediately after changing it. If this is true, I see 2
enhancements:

1. Instead of storing m_state_locked in every HTREEITEM just store a
global HTREEITEM of the unlocked item, this should save memory (I know it's
not a primary consideration when using Vista but still...) and also
actually make code simpler (for example you probably won't need the code in
TVN_DELETEITEM any more then).

2. Manually calling LockItemState(false/true) is error-prone as the item
may remain unlocked if there is a return in the middle of a function so I'd
much prefer to have the usual "unlocker" class which would unlock the item
in the ctor and lock it back in dtor (with the proposal (1) above it would
just a global variable to items handle in ctor and reset it back to invalid
value in dtor)


Otherwise I have a couple of minor comments:

3. It would be better to define TVN_ITEMCHANGING and TVITEMCHANGE (we can
suppose the latter is not defined if the former isn't and we can check for
it with #ifdef) in the beginning of the file as we usually do for missing
constants.

4. There is a missing (or extra?) word after "these" and a spelling error
in "equivalent" in the comment

+ // Note that, since we don't want to depend on the Vista SDK,
these
+ // we use equivalet message numbers and structures.

Thanks!

Hi Vadim,

This is kind of picking up on the work that was done 7 months ago.
Unfortunately, it's taken that long for me to have another look at this.

You wrote:
> So, assuming it still works on the other
> systems and makes the control
> under Vista too, could you please commit
> just the Vista part of the patch?

I would like to do this now. I have isolated the vista-relevant portions
of my tree work. This brings the multiple-selection tree up to the quality
that it is on other platforms. I do have other patches that I will submit
separately, but this one needs to get committed first.

A few notes:

1) This patch only modifies the multi-selection mode of the tree. The
normal single-selection mode, which is stable, hasn't been changed.

2) Regarding your question below, LockItemState() doesn't use
GetItemParam() because GetItemParam() is a member function, whereas
LockItemState() needs to be used in various other static functions.
LockItemState() itself will be static when I commit this.

Are you ok with me committing this now?

Thanks,
Ben

File Added: vista.patch

This Tracker item was closed automatically by the system. It was
previously set to a Pending status, and the original submitter
did not respond within 14 days (the time period specified by
the administrator of this Tracker).

I think Ben's solution of preventing the item change from taking place is
better so I believe this part of this patch should be committed. I've
commented separately about the others, I think the ones related to multiple
DND are not needed any more while the right button click one still has a
problem. But I don't think Vista fixes really depend on the other patches,
do they?

So, assuming it still works on the other systems and makes the control
under Vista too, could you please commit just the Vista part of the patch?
Concerning I have just 2 minor comments:

1. it would be better to define TVN_ITEMCHANGING instead of using
TVN_FIRST-16 (in the usual way, i.e. inside an #ifndef)

2. Why doesn't LockItemState() use GetItemParam()?

Thanks!

Hi Ryan,

That also seems fine. Does it work on Vista? The main problem driving
this patch is the fact that, on Vista, whenever TreeView_SelectItem is
called, Vista unhighlights previously highlighted tree items. This is in
contrast to previous versions of Windows. Indeed, on all versions prior to
Windows Vista, only the micro-patches #1702102, #1702133, and #1702201 are
necessary.

If you got your patch to work on Vista, I'd be really interested in taking
a look. Which patch number is it?

Best,
Ben

Hi Ben,

To deal with the differing selection behaviour of TreeView_SelectItem
across Windows versions, what I did was to remember the selections in the
treeview, call TreeView_SelectItem, then restore the selections. I am
curious to hear what you think of this approach vs. your own - my way is
perhaps less efficient but is perhaps easier to understand and more
futureproof.

Also concerning right-click behaviour I chose a more listview-oriented
approach summarized as:

// 1) Set focus to the treeview
// 2) If ctrl or shift is not pressed go to 3, Otherwise 4
// 3) If the clicked item is selected, focus it; otherwise unselect all
items and TreeView_SelectItem the clicked item
// 4) Bypass the treeview WndProc and call DefWindowProc