Thinking aloud: Can you factor this out into a small helper class called WheelEventFilter or similar? I can imagine this would be cleaner.
The code possibly would shrink down to something like:

if (m_wheelEventFilter.processEvent(event)) {
    // ... zoom in / out
    return;
}

Ok, we discard first wheel event cause elapsed timer isn't started, right? I think we can "catch" all events we just make it a bit different, set modifiers in contructor and restart elpased timer on global event override function if desired modifiers presents, so if wheelEvent enters before 200 ms elapsed just ignore modifiers so now we don't discard a wheel event just drops earlers modifiers.

In D5037#94944, @anthonyfieroni wrote:

Ok, we discard first wheel event cause elapsed timer isn't started, right?

No, it's not discarded; no wheel events are being discarded unless they already were being discarded (= horizontal scroll with dynamic word wrap). What my addition does is turn certain Ctrl+Wheel events into regular wheel events so that the view continues to scroll when you press the modified during a scroll (instead of switching to zooming).

There is no previous event so there's no need to do any checks to see if the modifier should be allowed to trigger zooming. So the first wheelevent just passes through the filter unmodified and if the modifier is pressed it will trigger zooming. Any other approach will probably only make things more difficult. And FWIW it's already semantically doubtful to set m_lastWheelEventUnmodified to false in the WheelEventFilter ctor (there is only a last event after the 1st wheel event has been processed) so putting it in the KateViewInternal ctor makes even less sense.

Similarly, the timer must be specific to wheel events, you wouldn't want mouse movements to interfere for instance. Or at least I think so, I only have trackpads nowadays.

Setting the modifier key in KateViewInternal does make sense but more so when it actually becomes possible to change it. Until that's not the case the compiler can optimise the current implementation a bit better (as far as that's of any importance here).

I think we can "catch" all events we just make it a bit different

I don't think I understand what you mean here. What we don't catch is situations where the events tell the view to continue scrolling in a direction where that's no longer possible. My PoC implementation on github catches those situations but would be overkill (and probably interfere with zooming when the document is at the top or bottom).

As to dropping wheel events: that doesn't happen but even if it did that shouldn't be a big deal if only 1 or a few events are dropped. This kind of event arrive in bursts from an effect-driven user input that isn't transient. IOW, contrary to clicking something, s/he will continue to turn the wheelbutton (or swipe up/down) until satisfied with whatever the desired result. I strongly doubt anyone one would notice if we dropped the 1st wheel event for instance.

Ok, note this situation, hold modifier before first ever wheel event => you call m_lastWheelEvent.elapsed on unstarted timer and function returns true (m_accidentalModifier == false, m_lastWheelEventUnmodified == false)

In D5037#95031, @anthonyfieroni wrote:

Ok, note this situation, hold modifier before first ever wheel event => you call m_lastWheelEvent.elapsed on unstarted timer

This is true, and things can indeed be improved a bit there.

and function returns true (m_accidentalModifier == false, m_lastWheelEventUnmodified == false)

Am I missing something? For me that is exactly what the function should return, and the internal state is correct too. This works because the initial values for the 2 boolean state variables are set appropriately. In this case deltaT may be assigned a random value (because ElapsedSince::elapsed() is undefined) but that value won't be used.

I can rewrite the code slightly so that the timer isn't used when not started but it won't make a difference for the end result, it will just add a few lines and an additional runtime check. Will do that ASAP.

I'd say try the patch and see if you can break its (now of after I upload the rewritten version). I just tried generating wheel events with the control modifier set from event 1. I even "primed" the text zoom via the keyboard shortcuts to prevent initial zooming hickups. I don't notice anything weird: the zoom works just as you'd expect.

Let me repeat if this wasn't clear: the purpose of this patch is only to prevent accidental zooming because you push the Control key while a scroll is still in progress. There's no way we can decide if a Control press is accidental when nothing else is going on.

In fact, we don't even know if what I now call an accidental Control press is really accidental. It's a reasonable assumption (I think) and it's still easy enough to trigger wheel-zooming with this protection in place. But it is possible that in a certain number of cases that we now treat as regular scroll events the user actually wanted zooming, not scrolling. I just have to hope that it is easier for those users to adapt to the behaviour with protection than it is for users like me to adapt to not having it :)

Yes, this is what I meant. I think the code is now better by using an isolated class.

I just read the code again, and I think I would like to suggest another change: The class name currently is too generic, this makes reasoning quite hard.
That said, I suggest:

• to rename WheelEventFilter to ZoomEventFilter
• to rename m_accidentalModifier to m_ignoreZoom
• remove the "bool skip = false" variable, and instead just check m_ignoreZoom instead

Maybe the naming can be improved even more. I will test this patch, but will not have time until Thursday.

A couple of minor issues I spotted while looking briefly over the patch.

What are the next steps here?

Given people seem to have tested this, I would be ok to let it go in now.
I guess we need some "user-test" for it anyways.

Thanks.

I've been using this patch myself and never noticed any side-effects. Except when an application shows to be susceptible to accidental zooming ... because it uses another widget class that doesn't have this protection :)