Closed Bug 554919 Opened 15 years ago Closed 14 years ago

Support panels which can be moved by dragging their background

Categories

(Core :: XUL, defect)

Product:
Core
Component:
XUL
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: enndeakin, Assigned: enndeakin)

References

Details

(Keywords: dev-doc-complete)

Attachments

(1 file, 2 obsolete files)

Address comments
9.50 KB, patch
dao
: review+
smaug
: review+
Details | Diff | Splinter Review 
This involves:
 - modifying WindowDraggingUtils.jsm to support panels as well as windows
 - hooking this up if some attribute is set on the panel
 popup's moveTo method

<method name="moveTo">
        <parameter name="aLeft"/>
        <parameter name="aTop"/>
        <body>
        <![CDATA[
          this.popupBoxObject.moveTo(aLeft, aTop);
        ]]>
        </body>
      </method>
  
uses popupBoxObject property

<property name="popupBoxObject">
        <getter>
          return this.boxObject.QueryInterface(Components.interfaces.nsIPopupBoxObject);
        </getter>
      </property>

which calls QueryInterface every time without a reason, making everything slow

so making lazygetter for popupBoxObject is needed for this too
Depends on: 552982
Depends on: 577377
No longer depends on: 552982
Attached patch implement this (obsolete) — — Splinter Review 
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch, with gtk support and test (obsolete)
        — 
          — Splinter Review
      

    


  

        Attachment #456350 -
        Attachment is obsolete: true

    
  

        Attachment #520629 -
        Flags: review?(dao)

    
    

    
  
Comment on attachment 520629 [details] [diff] [review]
patch, with gtk support and test

>   handleEvent: function(aEvent) {
>-#ifdef XP_WIN
>-    if (this.shouldDrag(aEvent))
>-      aEvent.preventDefault();
>-#else
>     switch (aEvent.type) {
>       case "mousedown":
>         if (!this.shouldDrag(aEvent))
>           return;
> 
>+        let isPanel = this.isPanel();
>+#ifdef XP_WIN
>+        if (!isPanel) {
>+          aEvent.preventDefault();
>+          return;
>+        }
>+#endif

This new code seems unreachable, since MozMouseHittest is used instead of mousedown in the XP_WIN case.

>--- a/toolkit/content/widgets/popup.xml
>+++ b/toolkit/content/widgets/popup.xml

>+      <field name="_alive">true</field>

That doesn't work this way anymore, we changed this in bug 624157.

smaug, sicking, jst or bz should probably review the change to beginWindowMove.

        Attachment #520629 -
        Flags: review?(dao) → review-

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Address comments
        — 
          — Splinter Review
      

    


  

        Attachment #520629 -
        Attachment is obsolete: true

        Attachment #521187 -
        Flags: review?(dao)

    
  

        Attachment #521187 -
        Flags: review?(Olli.Pettay)

    
    

    
  
Comment on attachment 521187 [details] [diff] [review]
Address comments

r+ for the dom/ part.


Is 'backdrag' such attribute that it can be set only before ctor has
run? Is there some way to disable the feature for sometime and then
enable it again?

        Attachment #521187 -
        Flags: review?(Olli.Pettay) → review+

    
    

    
  
There isn't a way to modify it currently. I can add that in a followup bug.

    
    

    
  
Comment on attachment 521187 [details] [diff] [review]
Address comments

WindowDraggingUtils.jsm doesn't seem to handle elements inside of panels in a sensible way. This can be taken care of in a separate bug (I could do it in bug 574971).

>+        if (isPanel) {
>+          var screenRect = this._elem.getOuterScreenRect();
>+          this._deltaX = aEvent.screenX - screenRect.left;
>+          this._deltaY = aEvent.screenY - screenRect.top;
>+        }

>+        if (this._draggingWindow) {
>+          var toDrag = this.isPanel() ? this._elem : this._window;
>+          toDrag.moveTo(aEvent.screenX - this._deltaX, aEvent.screenY - this._deltaY);
>+        }

s/var/let/

        Attachment #521187 -
        Flags: review?(dao) → review+

    
    

    
  
http://hg.mozilla.org/mozilla-central/rev/9c21ad3d7f72
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
Resolution: --- → FIXED

    
  
Keywords: dev-doc-needed

    
    

    
  
To clarify before I begin writing: this is about the new "backdrag" attribute on the XUL panel element, correct?

    
    

    
  
Yes, setting backdrag="true" allows the panel to be moved around on the screen by dragging on a background area of the panel.
See Also:  → 1455596

    
    

    
  
Moving to Core:XUL per https://bugzilla.mozilla.org/show_bug.cgi?id=1455336
Component: XP Toolkit/Widgets: XUL → XUL

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: