STR: 1) open testcase.html 2) click down on the link and drag it around the page a bit 3) release link drag 4) scroll page with mouse wheel result: jerky scroll movement

clicking on the page (which clears the 'hot' state of the link) fixes the problem.

I can reproduce the problem on Windows. Interestingly, it doesn't happen on Linux, scrolling remains smooth regardless of what I do with the link.

The slowdown doesn't happen with APZ disabled.

I'm not able to repro this. I might need a better mouse...

I hit on this all the time and it's pretty bad. I'd suggest blocking on figuring this out. I can generate logs if you need them kats.

Botond will look into it after he is done with the snap points stuff he's currently working on. Do you know if 46 and/or 47 are affected as well?

I can't reproduce 47.

This seems to be caused by an interaction between APZ scrollbar dragging and wheel scrolling. It appears that we are creating drag input blocks even if apz.drag.enabled is false, and as a result the bug is present independent of that pref.

This patch limits the bug to the case where apz.drag.enabled is set to true, by disabling the creation of drag input blocks when the pref is set to false. It might be useful as a temporary workaround if you're really annoyed by this bug.

cc'ing BenWa as this appears to be a regression from the APZ scrollbar dragging work.

Attached is logging from the INPQ, TBS, and DRAG components while reproducing the bug. I note the conspicuous absence of a "DRAG: Ending drag" message, even though I've definitely ended my drag.

I don't think I'm going to have a chance to get to the bottom of this before TRIBE this week. BenWa, if you'd like to look into this while I'm at TRIBE, please feel free. Otherwise I can continue investigating when I'm back. We could also land the workaround from comment 9 in the meantime.

Note: if you're trying to reproduce this, and you're on a touchscreen laptop, be sure to set "browser.tabs.remote.force-enable" to true, otherwise e10s (and thus APZ) will be disabled.

(In reply to Botond Ballo [:botond] from comment #9) > Created attachment 8737994 [details] [diff] [review] > Do not create drag input blocks if apz.drag.enabled is false > > This patch limits the bug to the case where apz.drag.enabled is set to true, > by disabling the creation of drag input blocks when the pref is set to false. > > It might be useful as a temporary workaround if you're really annoyed by > this bug. I say we take the patch. We have no immediate plan on enabling this feature in the short term. We can then make this issue block enabling the feature. There's several regression from the feature so no point in giving this one special treatment.

Comment on attachment 8737994 [details] [diff] [review] Do not create drag input blocks if apz.drag.enabled is false Review of attachment 8737994 [details] [diff] [review]: ----------------------------------------------------------------- Works for me.

Comment on attachment 8737994 [details] [diff] [review] Do not create drag input blocks if apz.drag.enabled is false Review of attachment 8737994 [details] [diff] [review]: ----------------------------------------------------------------- Creation of drag blocks was done intentionally in bug 1242690, this is probably a regression from that. I'll look into it in more detail but landing this as-is will probably break something else related to that bug.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #16) > Creation of drag blocks was done intentionally in bug 1242690, this is > probably a regression from that. Ah, I missed that!

(In reply to Botond Ballo [:botond] from comment #11) > Attached is logging from the INPQ, TBS, and DRAG components while > reproducing the bug. I note the conspicuous absence of a "DRAG: Ending drag" > message, even though I've definitely ended my drag. It appears that APZCTreeManager::ReceiveInputEvent() simply doesn't receive a "mouse up" event at the end of the link drag. It does seem to receive it for other (non-link) drags.

(In reply to Botond Ballo [:botond] from comment #18) > It appears that APZCTreeManager::ReceiveInputEvent() simply doesn't receive > a "mouse up" event at the end of the link drag. It does seem to receive it > for other (non-link) drags. The issue is not at the APZC level: nsWindow::ProcessMessage() is not receiving a WM_LBUTTONUP message for the button-up of the link drag. It does receive WM_LBUTTONUP for the button-up of other drags.

I don't know much about Windows event handling, but I have a theory. The MSDN docs on WM_LBUTTONUP [1] say the following: > Posted when the user releases the left mouse button while the cursor is in > the client area of a window. If the mouse is not captured, the message is > posted to the window beneath the cursor. Otherwise, the message is posted > to the window that has captured the mouse. When I drag the link, a faded-out "copy" of the link appears and moves with the mouse while dragging. Perhaps this is a new window, and it "captures" the mouse? [1] https://msdn.microsoft.com/en-us/library/windows/desktop/ms645608(v=vs.85).aspx

I discussed this with :jimm on IRC. The widget's behaviour (not sending a mouse-end event) is unexpected, but the theory in comment 20 seems to be wrong, as we don't create a window for the drag image.

I spent some more time looking at the Windows widget code, but I'm at a loss as to why we're not receiving the WM_LBUTTONUP event. We do capture the mouse with ::SetCapture() at the beginning of a drag operation, but the window that captures the mouse is the widget's own window, so it should still be the one receiving the WM_LBUTTONUP; moreover, modifying the code so it doesn't capture the mouse doesn't help, so it doesn't look like the capture is the problem.

So based on the test page at http://people.mozilla.org/~kgupta/bug/1260018.html it seems like once we enter an actual drag session, there is no mouseup at the end. Instead we can detect the end by listening for a dragend event. Updating DragTracker.cpp to listen for either a MOUSE_DRAG_END or a MOUSE_UP should fix this.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #23) > So based on the test page at > http://people.mozilla.org/~kgupta/bug/1260018.html it seems like once we > enter an actual drag session, there is no mouseup at the end. Instead we can > detect the end by listening for a dragend event. Updating DragTracker.cpp to > listen for either a MOUSE_DRAG_END or a MOUSE_UP should fix this. I thought of this, but it looks like the drag-end event is dispatched directly to content [1] [2], rather than going through the widget. [1] https://dxr.mozilla.org/mozilla-central/rev/21bf1af375c1fa8565ae3bb2e89bd1a0809363d4/widget/nsBaseDragService.cpp#387 [2] https://dxr.mozilla.org/mozilla-central/rev/21bf1af375c1fa8565ae3bb2e89bd1a0809363d4/widget/nsBaseDragService.cpp#441

Hm, we could add a bit of code there to send an event to APZCTreeManager::ReceiveInputEvent, and then just throw it away. I don't know if that's the best way forward though.

I split out the "no mouse-up event received at the end of the drag" issue into bug 1265105. I'm a bit out of my depth in terms of fixing that bug - I think it needs help from someone more familiar with the Windows widget code. For now, I will pursue the workaround described in comment 25 for this bug. If bug 1265105 ends up being fixed, we can the workaround.

This patch implements the workaround approach described in comment 25. I tested it locally and it's working well - we detect the end of the drag, and subsequent scrolling is nice and smooth. I didn't handle the case where the controller thread is not the main thread, as that doesn't arise on desktop; the drag event is just silently dropped on its way to APZ. Let me know if you feel this is not appropriate.

Comment on attachment 8742479 [details] [diff] [review] Route drag events to APZ so it can detect the end of a drag Review of attachment 8742479 [details] [diff] [review]: ----------------------------------------------------------------- Logic looks fine, formatting is pretty messed up. r+ with that cleaned up. ::: gfx/layers/apz/src/APZCTreeManager.cpp @@ +621,5 @@ > { > return aEvent.mMessage == eMouseMove || > aEvent.mMessage == eMouseDown || > + aEvent.mMessage == eMouseUp || > + aEvent.mMessage == eDragEnd; s/tabs/spaces/, here and throughout the patch @@ +1137,5 @@ > *aOutInputBlockId = InputBlockState::NO_BLOCK_ID; > } > > switch (aEvent.mClass) { > + case eMouseEventClass: trailing ws, here and throughout the patch ::: widget/nsBaseWidget.cpp @@ +1205,5 @@ > +nsBaseWidget::DispatchEventToAPZOnly(mozilla::WidgetInputEvent* aEvent) > +{ > + MOZ_ASSERT(NS_IsMainThread()); > + if (mAPZC) { > + if (APZThreadUtils::IsControllerThread()) { This is fine, but I'd prefer just making this a MOZ_ASSERT(APZThreadUtils::IsControllerThread()) and then running the body unconditionally. I agree that it's not worth properly handling the alternative, but we should probably catch that other case somehow.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #28) > formatting is pretty messed up. r+ with that cleaned up. Sorry; I don't have Eclipse set up on my Windows machine, I typed this patch up using Notepad++, which I haven't configured much. Will fix.

Fixed formatting. Carrying r+.

(In reply to Botond Ballo [:botond] from comment #30) > Created attachment 8742490 [details] [diff] [review] > Route drag events to APZ so it can detect the end of a drag > > Fixed formatting. Carrying r+. (The updated patch also addresses the comment about the controller thread.) https://treeherder.mozilla.org/#/jobs?repo=try&revision=d7e3a76d4148

Updating title to reflect the bug more accurately.

I couldn't reproduce under Win 7 or Windows 10 64-bit. Reproduced and verified fixed using Aurora 48.0a2 under Mac OS X 10.9.5.