Closed Bug 927259 Opened 12 years ago Closed 12 years ago

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

Categories

(Core :: DOM: UI Events & Focus Handling, defect)

Product:
Core
Component:
DOM: UI Events & Focus Handling
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
major

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla27
Tracking Flags:
Tracking Status
firefox24 --- unaffected
firefox25 --- unaffected
firefox26 --- unaffected
firefox27 + verified
firefox-esr17 --- unaffected
firefox-esr24 --- unaffected

People

(Reporter: masayuki, Assigned: areinald.bug)

References

Details

(Keywords: regression, verifyme)

Attachments

(1 file)

bugzilla-927259-1.patch
1.58 KB, patch
masayuki
: review+
bajaj
: approval-mozilla-aurora+
Details | Diff | Splinter Review
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)
Attachment #817760 - Flags: review?(masayuki) → review+
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: 12 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
status-firefox27: fixed → 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.

Attachment

General

Creator:
Created:
Updated:
Size: