Cannot switch scroll target by moving mouse cursor with trackpad of MacBook

VERIFIED FIXED in Firefox 27

Status

()

defect
--
major
VERIFIED FIXED
6 years ago
4 months ago

People

(Reporter: masayuki, Assigned: areinald.bug)

Tracking

({regression, verifyme})

Trunk
mozilla27
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox24 unaffected, firefox25 unaffected, firefox26 unaffected, firefox27+ verified, firefox-esr17 unaffected, firefox-esr24 unaffected)

Details

Attachments

(1 attachment)

Mouse wheel transaction should be finished if mouse cursor is moved since it means user may want to switch the scroll target.

For example,

1. Click [reply] of this comment. Then, all text in this comment is copied into the <textarea>.
2. First, try to scroll all over the page of this bug by two finger swipe.
3. Next, move mouse cursor over the <textarea>
4. Finally, do two finger swipe on the <textarea>

then, the <textarea> should be scrolled. However, actually, the <body>'s scrollbar is still shown. This means that mouse wheel transaction is still kept by nsScrollbarsForWheel.

In nsMouseWheelTransaction::OnEvent(WidgetEvent* aEvent),

> 492     case NS_WHEEL_WHEEL:
> 493       if (sMouseMoved != 0 &&
> 494           OutOfTime(sMouseMoved, GetIgnoreMoveDelayTime())) {
> 495         // Terminate the current mousewheel transaction if the mouse moved more
> 496         // than ignoremovedelay milliseconds ago
> 497         MayEndTransaction();
> 498       }
> 499       return;
> 500     case NS_MOUSE_MOVE:
> 501     case NS_DRAGDROP_OVER:
> 502       if (IsMouseEventReal(aEvent)) {
> 503         // If the cursor is moving to be outside the frame,
> 504         // terminate the scrollwheel transaction.
> 505         nsIntPoint pt = GetScreenPoint(static_cast<WidgetGUIEvent*>(aEvent));
> 506         nsIntRect r = sTargetFrame->GetScreenRectExternal();
> 507         if (!r.Contains(pt)) {
> 508           MayEndTransaction();
> 509           return;
> 510         }

I believe that both line 497 and line 508 should call EndTransaction() rather than MayEndTransaction().
You are probably right about this. I will post a patch shortly. Thanks for pointing that out.
Attachment #817760 - Flags: review?(masayuki)
I should've realized this bug at reviewing the patch. Sorry.
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #3)
> I should've realized this bug at reviewing the patch. Sorry.

Don't worry, I think both of us wanted bug 868648 to finally land. We may discover other glitches in the future.

We already know one of them is the adding timers of nsMouseWheelTransaction and ScrollbarActivity. But I'll wait for feedback before I post a followup bug.
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/d0139a74ac03

A friendly reminder that your commit message in general should be summarizing what the patch is doing, not restating the bug title :)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/d0139a74ac03
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
I guess "tracking-firefox27: +" means this patch has to be uplifted to Aurora, and that's probably why I get weekly reminders about it. But as I can't do it myself, I don't know what I'm expected to do at this point.
Flags: needinfo?(bbajaj)
(In reply to André Reinald from comment #7)
> I guess "tracking-firefox27: +" means this patch has to be uplifted to
> Aurora, and that's probably why I get weekly reminders about it. But as I
> can't do it myself, I don't know what I'm expected to do at this point.

Sorry for the delayed response here.

Regarding uplifting to aurora, please click on the attachment on this bug and set approval-mozilla-aurora to "?" and fill out the form that pop's up on setting this request.
Flags: needinfo?(bbajaj)
Comment on attachment 817760 [details] [diff] [review]
bugzilla-927259-1.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 927259
User impact if declined: Scroll target is not switched when cursor is moved.
Testing completed (on m-c, etc.): Has landed in m-c for over 5 weeks, no issue since then.
Risk to taking this patch (and alternatives if risky): Low
String or IDL/UUID changes made by this patch: None

Note: this patch is a follow-up to attachment 815905 [details] [diff] [review] from bug 868648, both targeted for Mozilla 27 (which is now aurora), hence my request for aurora uplift for both patches.
Attachment #817760 - Flags: approval-mozilla-aurora?
Keywords: verifyme
Attachment #817760 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
MXR shows this patch also had landed in aurora. I don't understand why I was getting Monday reminders. Both bug 868648 and this one are fixed in Aurora i.e. Fx27. I'm not sure how to get rid of the reminders about this bug.
Can someone with a MacBook please verify this issue is fixed using Firefox 27 beta 4? 
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/27.0b4-candidates/build1/mac/en-US/
works with 27beta6 on mac 10.9
Status: RESOLVED → VERIFIED
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.