Closed Bug 1327185 Opened 8 years ago Closed 7 years ago

[e10s] dragleave events don't fire on the page, so Imgur.com doesn't remove huge image obscuring content

Categories

(Core :: DOM: Events, defect, P2)

Product:
Core
Component:
DOM: Events
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla54
Tracking Flags:
Tracking Status
firefox54 --- verified

People

(Reporter: arni2033, Assigned: stone)

References

Details

Attachments

(2 files, 4 obsolete files)

Part1: Refine EventStateManager::FireDragEnterOrExit. r=smaug
(deleted), patch
stone
: review+
Details | Diff | Splinter Review
Part2: Forward eDragExit to remote target. r=smaug
(deleted), patch
stone
: review+
Details | Diff | Splinter Review 
>>>   My Info:   Win7_64, Nightly 49, 32bit, ID 20160526082509
STR_1:
1. Open any image on imgur, for example http://imgur.com/0Cw7aKJ or http://imgur.com/a/fTif0
2. Drag identity block [useless (i) button implemented in bug 1206244] from urlbar to the center of
   the page without dropping
3. Drop identity block to the "Back" button in urlbar

AR:  A huge image saying "Drop images here" appears in Step 2 and doesn't disappear FOREVER
ER:  In Step 3 image (obscuring the rest of content) should disappear


STR_2:
1. Open url   data:text/html,<html ondragleave="console.log(123)">
2. Drag identity block [useless (i) button] from urlbar to the center of the page without dropping
3. Drop identity block to the "Back" button in urlbar

AR:  No messages in console
ER:  Should be a message, just like in non-e10s
No longer blocks: 1277113
Component: Untriaged → DOM: Events
Product: Firefox → Core
Hi Stone, this looks a regression from e10s. Could you please take a look?
Flags: needinfo?(sshih)
Priority: -- → P2
We generated dragleave event [1] at handling dragover in EventStateManager::PreHandleEvent. However, we only dispatch it to DOM via EventDispatcher::Dispatch, which doesn't forward to content process.

[1] http://searchfox.org/mozilla-central/rev/82ebc0c5ea45477ef4a4bfb3bbff35069bd18cee/dom/events/EventStateManager.cpp#4466
Assignee: nobody → sshih
Flags: needinfo?(sshih)

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Part2: Forward eDragExit to remote target (obsolete) (deleted)
        — 
          — Splinter Review
      

    


  

    
    

    
  


  

        Attachment #8827317 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 8827318 [details] [diff] [review]
Part2: Forward eDragExit to remote target

Forward dragexit to content process when dragging something from web content to chrome.

        Attachment #8827318 -
        Flags: review?(bugs)

    
  

        Attachment #8827319 -
        Flags: review?(bugs)

    
    

    
  
Comment on attachment 8827318 [details] [diff] [review]
Part2: Forward eDragExit to remote target

oops, wrong file

        Attachment #8827318 -
        Flags: review?(bugs)

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Part2: Forward eDragExit to remote target (obsolete) (deleted)
        — 
          — Splinter Review
      

    


  

        Attachment #8827318 -
        Attachment is obsolete: true

    
  

        Attachment #8827336 -
        Flags: review?(bugs)

    
    

    
  
Comment on attachment 8827319 [details] [diff] [review]
Part1: Refine EventStateManager::FireDragEnterOrExit


> void
>+EventStateManager::CopyWidgetDragEvent(WidgetDragEvent* aDragEvent,
>+                                       nsIContent* aRelatedTarget,
>+                                       WidgetDragEvent* aOutDragEvent)
>+{
>+  aOutDragEvent->mRefPoint = aDragEvent->mRefPoint;
>+  aOutDragEvent->mModifiers = aDragEvent->mModifiers;
>+  aOutDragEvent->buttons = aDragEvent->buttons;
>+  aOutDragEvent->relatedTarget = aRelatedTarget;
>+  aOutDragEvent->pointerId = aDragEvent->pointerId;
>+  aOutDragEvent->inputSource = aDragEvent->inputSource;
>+}
>+
Ok, so WidgetDrapEvent::Duplicate() can't be used here easily, since that copies also the flags and that means
copying also preventDefault etc. state. AssignDragEventData might be useful, not sure.
But since you're just moving code, fine.

        Attachment #8827319 -
        Flags: review?(bugs) → review+

        Attachment #8827336 -
        Flags: review?(bugs) → review+

    
    

    
  


  
Refined patch with using AssignDragEventData to copy event attributes

        Attachment #8827319 -
        Attachment is obsolete: true

        Attachment #8829299 -
        Flags: review+

    
    

    
  


  
Updated the patch summary.

        Attachment #8827336 -
        Attachment is obsolete: true

        Attachment #8829300 -
        Flags: review+

    
  
Keywords: checkin-needed

    
    

    
  
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a37c536d93aa
Part 1: Refine EventStateManager::FireDragEnterOrExit. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee3a429c5878
Part 2: Forward eDragExit to remote target. r=smaug
Keywords: checkin-needed

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/a37c536d93aa
https://hg.mozilla.org/mozilla-central/rev/ee3a429c5878
Status: NEW → RESOLVED
Closed: 7 years ago

          status-firefox54:
          --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Flags: qe-verify+

    
    

    
  
I managed to reproduce the issue on Firefox 53.0a1 (2017-01-01), under Windows 10x64.

The issue is no longer reproducible on Firefox 54.0b2, using the STR from Comment 0.  
Tests were performed under Windows 10x64, Ubuntu 16.04x64, Mac OS X 10.12.1.
Status: RESOLVED → VERIFIED

          status-firefox54:
          fixed → verified
Flags: qe-verify+

    
  
Depends on: 1609903

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: