Closed Bug 571640 Opened 14 years ago Closed 14 years ago

toolkit/content/tests/chrome/test_findbar.xul fails with bug 130078

Categories

(Core :: DOM: Copy & Paste and Drag & Drop, defect)

Product:
Core
Component:
DOM: Copy & Paste and Drag & Drop
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: tnikkel, Assigned: tnikkel)

References

Details

Attachments

(3 files)

Part 1. Add a source argument to synthesizeDrop.
13.63 KB, patch
enndeakin
: review+
Details | Diff | Splinter Review
Part 2. Fix test_findbar.xul.
2.49 KB, patch
enndeakin
: review+
Details | Diff | Splinter Review
Part 3. Fix test_browser_drop.xul.
1.06 KB, patch
enndeakin
: review+
Details | Diff | Splinter Review 
The testDrop part of test_findbar.xul fails with bug 130078. The reason for this is because the findbar is hidden when the test runs the synthesizeDrop ends up sending a mousedown and some mousemoves events to start a drag relative to 0,0 (because the client rect of the findfield is 0,0,0,0 since it is hidden). Without bug 130078 these events are over the subdocument frame for the browser, they do not descend because the viewmanager's are not linked for the content of the browser and the chrome. With bug 130078 these mouse events are over the actual subdocument and end up starting and creating a selection. Because there is a selection in progress nsEventStateManager::GenerateDragGesture bails early and never generates a drag.

If we make the findbar visible before synthesizeDrop then it fails with and without bug 130078 for the same reason: a selection is in progress.

Can we:
1) create a (test-only?) way from script to create a nsIDOMDataTransfer so we can skip sending the mouse events and getting the dataTransfer off the generated drag?
2) do something with js and events that I haven't thought of to prevent the selection from interfering with the drag?
3) add an optional source node to synthesizeDrop that we use to start the drag, something like an image that won't start a selection if we send those mouse events to it?
Falling under number 2) would be setting MozUserSelect to none when we do the synthesizeDrop.
I think either 1) or 3) is the way to go unless there is some other way to make 2) work. I'm not sure if 1) is a good idea or not. Neil, what do you think?
3 is something I've thought of implementing for other tests, so I'd go with that.
Assignee: nobody → tnikkel

        Attachment #452190 -
        Flags: review?(enndeakin)

    
    

    
  


  
Add an image to the content document and use that to start the drag. Also open and close the findbar.

        Attachment #452191 -
        Flags: review?(enndeakin)

    
    

    
  


  
test_browser_drop.xul has a similar problem in that the mouse events get targeted at the html element and a selection prevents a drag from happening. Here things are html so we can just use the draggable attribute.

        Attachment #452195 -
        Flags: review?(enndeakin)

    
  

        Attachment #452190 -
        Flags: review?(enndeakin) → review+

    
  

        Attachment #452191 -
        Flags: review?(enndeakin) → review+

    
    

    
  
Comment on attachment 452191 [details] [diff] [review]
Part 2. Fix test_findbar.xul.

>+        gBrowser.loadURI("data:text/html,<h2 id='h2'>" + SEARCH_TEXT + "</h2><h2><a href='" + SAMPLE_URL + "'>Link Test</a></h2><input id='text' type='text' value='" + SAMPLE_TEXT + "'></input><input id='button' type='button'></input><img id='img' width='50' height='50'/>");

No trailing slash should be used

    
    

    
  
Comment on attachment 452195 [details] [diff] [review]
Part 3. Fix test_browser_drop.xul.

> <browser id="chromechild" src="about:blank"/>
> <browser id="contentchild" type="content" width="100" height="100"
>-         src="data:text/html,&lt;body draggable='true' style='width: 100px; height: 100px;' ondrop='if (window.stopMode) event.stopPropagation(); if (window.cancelMode) event.preventDefault();'&gt;&lt;/body&gt;"/>
>+         src="data:text/html,&lt;html draggable='true'&gt;&lt;body draggable='true' style='width: 100px; height: 100px;' ondrop='if (window.stopMode) event.stopPropagation(); if (window.cancelMode) event.preventDefault();'&gt;&lt;/body&gt;&lt;/html&gt;"/>

Do both <html> and <body> need to be draggable? Does the test not work properly currently?

    
    

    
  
(In reply to comment #8)
> Do both <html> and <body> need to be draggable? Does the test not work properly
> currently?

The test doesn't work with bug 130078. The body needs to be draggable because of the expectLink(body,...) calls after the for loop in the test. The expeckLink(child,...) in the for loop end up hitting the html element inside the content browser. So the html element needs to be draggable to prevent a selection from starting and preventing a drag.

    
    

    
  
(In reply to comment #9)
> (In reply to comment #8)
> > Do both <html> and <body> need to be draggable? Does the test not work properly
> > currently?
> 
> The test doesn't work with bug 130078.

Why not? What behaviour has changed?

    
    

    
  
(In reply to comment #10)
> > The test doesn't work with bug 130078.
> 
> Why not? What behaviour has changed?

Sorry I should have made this more clear. It's for the same reason that test_findbar.xul fails. Before bug 130078 any mouse events over a content browser that are dispatched from chrome can't drill down into the content browser because the view for the content browser isn't linked into the view system for the chrome. So the mouse event is "over" the browser element, not whatever content it is actually over within the browser. After bug 130078 the views are linked, so when getting the frame the mouse event is over can drill down into the browser and it returns the html element.

    
  

        Attachment #452195 -
        Flags: review?(enndeakin) → review+

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: