[wx-dev] scrolling code is wrong :-)

[Paul Cornett]

Robert Roebling wrote:
> Paul Cornett wrote:
> 
> > Robert Roebling wrote:
> > > Another thing which came to my mind while being bored is
> > > that we probably should not handle mouse wheel action
> > > in wxScrollHelper at all. There is a native handler which
> > > might do something special (either nice or just special)
> > > and we should use it (under GTK+). So I intend:
> > > a) to block events from wxWindow::SetScrollPosition()
> > 
> > Again, SetScrollPos does _not_ fire any wx events. No changes
> > are, or were, needed for this.
> 
> I know, but SetScrollPosition() does fire value_changed signals
> and they are caught by wx. I know that happens elsewhere
> in wxWidgets, but as we just learned from the wxTextCtrl
> case of filtering out events, this can go wrong if GTK+
> decides to change the event emission sequence.

Thats pretty vague.

> It strikes
> me as a very clean approach to disable events, then set
> the scrollbar (or value or whatever) and then reenable
> event emission/notification again.

I don't agree that disconnecting handlers is cleaner. Looking at the changes you made, I can see that if SetScrollPos is called recursively (which does happen) the "value_changed" handler will end up being connected multiple times. Ick.

> > > b) apply your fix to wxListCtrl uber-special scroll
> > >    handling with centralized visible line range testing
> > >    and cashing
> > > c) return true in the event processing for mouse wheel
> > >    action to prevent double action
> > > d) disable non-native mouse-wheel action in wxScrollHelper
> > >    for GTK+
> > 
> > If you want native scroll wheel behavior, which seems reasonable,
> > I think the best way to do it might be to simply use connect_after
> > for our wheel signal handler.
> 
> I don't think that GtkScrolledWindow really consumes the event
> so this won't have any effect.

It does. I checked. Before suggesting it.

> Putting the mouse wheel event
> handling code in wxScrollHelperEvtHandler in #ifndef __WXGTK20__
> will have the desired effect.

I'd say platform-specific ifdefs in common code is definitely not cleaner.