Closed Bug 1754674 Opened 2 years ago Closed 2 years ago

DManipEventHandler doesn't produce pan momentum events as expected

Categories

(Core :: Panning and Zooming, defect)

defect

Tracking

()

RESOLVED FIXED
99 Branch
Tracking Status
firefox99 --- fixed

People

(Reporter: hiro, Assigned: hiro)

References

Details

Attachments

(1 file)

This is the cause of the lag a user reported to us I believe.

With my external touchpad, I get a bunch of PANGESTURE_PAN events during I suppose it should be PANGESTURE_MOMENTUMPAN. Moreover I don't reliably get PANGESURE_MOMENTUSTART event.

With dropping this mState == State::eInertia condition, I do get momentum events as expected, but there's no PANGESTURE_END. :/

(In reply to Hiroyuki Ikezoe (:hiro) from comment #1)

With dropping this mState == State::eInertia condition, I do get momentum events as expected, but there's no PANGESTURE_END. :/

The same line in the Chrome code I mostly copied

https://source.chromium.org/chromium/chromium/src/+/main:content/browser/renderer_host/direct_manipulation_event_handler_win.cc;l=245;bpv=0

does not have the Inertia condition. So I must have thought I was improving the code, but instead I introduced a bug.

(In reply to Timothy Nikkel (:tnikkel) from comment #2)

Looks like we won't send Pan End because of the code here

https://searchfox.org/mozilla-central/rev/c12a59323ee46b29b90c9917a3a7a70ea714ffec/widget/windows/DirectManipulationOwner.cpp#222

Yeah, calling SendPan(Phase::eEnd, 0.f, 0.f, false) unconditionally gives me pan end events as expected.

(In reply to Timothy Nikkel (:tnikkel) from comment #3)

The same line in the Chrome code I mostly copied

https://source.chromium.org/chromium/chromium/src/+/main:content/browser/renderer_host/direct_manipulation_event_handler_win.cc;l=245;bpv=0

does not have the Inertia condition. So I must have thought I was improving the code, but instead I introduced a bug.

I was looking at the code actually, it makes me think Chrome doesn't want to send pan end events until all momentum events finished? Am I correctly remember on Mac the order of pan events and pan momentum events something like; pan-start, pan-pan,...., pan-end, pan-momentum-start, pan-momentum-pan,..., pan-momentum-end?

Anyway, Timothy can you handle this bug?

(In reply to Hiroyuki Ikezoe (:hiro) from comment #4)

I was looking at the code actually, it makes me think Chrome doesn't want to send pan end events until all momentum events finished? Am I correctly remember on Mac the order of pan events and pan momentum events something like; pan-start, pan-pan,...., pan-end, pan-momentum-start, pan-momentum-pan,..., pan-momentum-end?

(I was writing this comment as you were writing yours)

From this commit

https://source.chromium.org/chromium/chromium/src/+/19e338fb2fd4111f84770dd39dc1790d872fb5aa

it sounds like they don't want to send a pan end (scroll end) event if they get a momentum event after it. So that explains that part.

Assignee: nobody → hikezoe.birchill
Status: NEW → ASSIGNED
Pushed by hikezoe.birchill@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4bea3a445c03
Send a pan end event before momentum events on Windows. r=tnikkel
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 99 Branch
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: