Closed Bug 927259 Opened 11 years ago Closed 11 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: 11 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: