regression: focus does not hold in Vision list view

[pulkomandy]

Trying to select a channel in Vision, it will not stay focused in the list view, as a result it is not possible to see which channel is active.

Possibly a regression from hrev52069 ?

[closequarters]

I was concerned that I broke this in my fix for #9190, so I did some debugging here. The problem is that BListView::MouseDown sets the instance variable fTrack->item_index, and then MouseUp uses this instance variable to determine which item in the list to select/deselect. However, Vision completely overrides MouseDown in WindowList::MouseDown, and never calls the super class BListView::MouseDown method. The result is that the MouseDown of WindowList fires, followed by the MouseUp of BListView.

I'm not sure what the right solution is here because I don't have enough experience with Haiku applications, but I think it's possible we could classify this as a Vision bug, not a Haiku bug. If Vision were to override MouseUp and do nothing there, then behavior of focus on select would be restored. Thoughts?

[jscipione]

Nice debugging closequarters: it does indeed sound like an app bug in Vision not calling the inherited classes MouseDown method which was exposed by your fix to #9190. It is definitely not a regression from hrev52069. :) I think the proper fix needs to be done in Vision.

The BeBook is a bit vague when it says under BListView::MouseDown: "you shouldn't override (or call) this [method]." Ok but if you do override it anyway, you should call the base method because it does a bunch of stuff (paraphrasing) "Responds to B_MOUSE_DOWN messages by selecting items, invoking items on double-click, and autoscrolling the list when the user drags with a mouse button down. This function also calls InitiateDrag() to give derived classes the opportunity to drag items."

Vision overriding MouseDown without calling its inherited BListView::MouseDown method certainly seems to be an app bug.

[pulkomandy]

Fair enough, but I think the Vision code worked in BeOS so it is still a behavior incompatibility. I'm fine with patching Vision if it's the only app affected...

[jscipione]

The question is not why doesn't Vision work now, the question is how did the BListView subclass work in Vision before? BeOS and Haiku until very recently were doing all this stuff in BListView::MouseDown which apparently Vision never called. We've recently changed Haiku to do most of this work in BListView::MouseUp instead which caused this bug to show in Vision but how did selecting items work on BeOS if Vision never called BListView::MouseDown and at least according to the BeBook that's where R5 did the selecting of items? A mystery.

[closequarters]

jscipione, Vison selection works because Select() is called explicitly in Vision WindowList::MouseDown. Essentially the authors of Vision completely replaced MouseDown with their own version. What is odd is that apparently BListView::MouseUp didn't do any selecting in BeOS, otherwise Vision would have been experiencing this issue prior most likely.

I like PulkoMandy's idea of testing some other apps to see if this selection problem is more widespread. So far in the built-in apps/prefs, I haven't been able to reproduce a selection failure as seen in Vision, so I'm feeling better about patching Vision instead of Haiku

[closequarters]

This Vision bug is solved by patch posted to #9190 : ​https://review.haiku-os.org/#/c/haiku/+/343