Bug 1368094 - Correct panel sliding on window resize r?bz draft
authorDoug Thayer <dothayer@mozilla.com>
Fri, 09 Jun 2017 10:49:53 -0700
changeset 593508 0907e92d681c3c9026aaee0fcb10f43a54d9b2eb
parent 591816 2d20b365eee19434657f6b365d310e8b70904d2b
child 633137 9889a59712aeb63ac539e1cc7d6c257cc187c77d
push id63724
push userbmo:dothayer@mozilla.com
push dateTue, 13 Jun 2017 20:26:09 +0000
reviewersbz
bugs1368094
milestone55.0a1
Bug 1368094 - Correct panel sliding on window resize r?bz This is in response to an issue that's affecting the new app update doorhangers on OSX, where the problem is more obvious. On OSX, the panel styling makes it so that the doorhanger overflows the window a little bit. This is fine until you enter fullscreen with ctrl+command+F. At this point, the doorhanger should come back onto the screen and the arrow should be rooted to its anchor element (in our case the hamburger menu icon), but instead it lags and the panel is not adjusted right away. This is because right after the window is resized, which ends up calling SetPopupPosition with aIsMove == false, SetPopupPosition is called again from CheckForAnchorChange with aIsMove set to true. There could be other solutions to this particular problem, but since the aIsMove boolean is intended to limit the visual noise when moving a window between screens, it seemed appropriate for it to only prevent sliding or flipping if the panel isn't already slid or flipped. There was another issue affecting specifically the arrow, where the logic for notifying observers of a positioning change in the panel doesn't account for changes only to the position of the anchor rect. This change adds tracking of that and sets aNotify to true when called from ReflowFinished, since this is where the position of the anchor element relative to the window can need to change, even when the screen position of the panel rect doesn't change. MozReview-Commit-ID: Lpfokwkgl33
layout/xul/nsMenuPopupFrame.cpp
layout/xul/nsMenuPopupFrame.h
--- a/layout/xul/nsMenuPopupFrame.cpp
+++ b/layout/xul/nsMenuPopupFrame.cpp
@@ -113,16 +113,17 @@ nsMenuPopupFrame::nsMenuPopupFrame(nsSty
   , mIsContextMenu(false)
   , mAdjustOffsetForContextMenu(false)
   , mGeneratedChildren(false)
   , mMenuCanOverlapOSBar(false)
   , mShouldAutoPosition(true)
   , mInContentShell(true)
   , mIsMenuLocked(false)
   , mMouseTransparent(false)
+  , mIsOffset(false)
   , mHFlip(false)
   , mVFlip(false)
   , mAnchorType(MenuPopupAnchorType_Node)
 {
   // the preference name is backwards here. True means that the 'top' level is
   // the default, and false means that the 'parent' level is the default.
   if (sDefaultLevelIsTop >= 0)
     return;
@@ -596,17 +597,17 @@ nsMenuPopupFrame::LayoutPopup(nsBoxLayou
     pc->PresShell()->PostReflowCallback(this);
     mReflowCallbackData.MarkPosted(aAnchor, aSizedToPopup);
   }
 }
 
 bool
 nsMenuPopupFrame::ReflowFinished()
 {
-  SetPopupPosition(mReflowCallbackData.mAnchor, false, mReflowCallbackData.mSizedToPopup, false);
+  SetPopupPosition(mReflowCallbackData.mAnchor, false, mReflowCallbackData.mSizedToPopup, true);
 
   mReflowCallbackData.Clear();
 
   return false;
 }
 
 void
 nsMenuPopupFrame::ReflowCallbackCanceled()
@@ -1553,20 +1554,24 @@ nsMenuPopupFrame::SetPopupPosition(nsIFr
         hFlip = FlipStyle_Outside;
     }
 #else
     // Other OS screen positioned popups can be flipped vertically but never horizontally
     vFlip = FlipStyle_Outside;
 #endif // #ifdef XP_MACOSX
   }
 
-  // If a panel is being moved or has flip="none", don't constrain or flip it. But always do this for
+  nscoord oldAlignmentOffset = mAlignmentOffset;
+
+  // If a panel is being moved or has flip="none", don't constrain or flip it, in order to avoid
+  // visual noise when moving windows between screens. However, if a panel is already constrained
+  // or flipped (mIsOffset), then we want to continue to calculate this. Also, always do this for
   // content shells, so that the popup doesn't extend outside the containing frame.
   if (mInContentShell || (mFlip != FlipType_None &&
-                          (!aIsMove || mPopupType != ePopupTypePanel))) {
+                          (!aIsMove || mIsOffset || mPopupType != ePopupTypePanel))) {
     int32_t appPerDev = presContext->AppUnitsPerDevPixel();
     LayoutDeviceIntRect anchorRectDevPix =
       LayoutDeviceIntRect::FromAppUnitsToNearest(anchorRect, appPerDev);
     LayoutDeviceIntRect rootScreenRectDevPix =
       LayoutDeviceIntRect::FromAppUnitsToNearest(rootScreenRect, appPerDev);
     LayoutDeviceIntRect screenRectDevPix =
       GetConstraintRect(anchorRectDevPix, rootScreenRectDevPix, popupLevel);
     nsRect screenRect =
@@ -1597,37 +1602,41 @@ nsMenuPopupFrame::SetPopupPosition(nsIFr
     }
 
     // Next, check if there is enough space to show the popup at full size when
     // positioned at screenPoint. If not, flip the popups to the opposite side
     // of their anchor point, or resize them as necessary.
     bool endAligned = IsDirectionRTL() ?
       mPopupAlignment == POPUPALIGNMENT_TOPLEFT || mPopupAlignment == POPUPALIGNMENT_BOTTOMLEFT :
       mPopupAlignment == POPUPALIGNMENT_TOPRIGHT || mPopupAlignment == POPUPALIGNMENT_BOTTOMRIGHT;
+    nscoord preOffsetScreenPoint = screenPoint.x;
     if (slideHorizontal) {
       mRect.width = SlideOrResize(screenPoint.x, mRect.width, screenRect.x,
                                   screenRect.XMost(), &mAlignmentOffset);
     } else {
       mRect.width = FlipOrResize(screenPoint.x, mRect.width, screenRect.x,
                                  screenRect.XMost(), anchorRect.x, anchorRect.XMost(),
                                  margin.left, margin.right, offsetForContextMenu.x, hFlip,
                                  endAligned, &mHFlip);
     }
+    mIsOffset = preOffsetScreenPoint != screenPoint.x;
 
     endAligned = mPopupAlignment == POPUPALIGNMENT_BOTTOMLEFT ||
                  mPopupAlignment == POPUPALIGNMENT_BOTTOMRIGHT;
+    preOffsetScreenPoint = screenPoint.y;
     if (slideVertical) {
       mRect.height = SlideOrResize(screenPoint.y, mRect.height, screenRect.y,
                                   screenRect.YMost(), &mAlignmentOffset);
     } else {
       mRect.height = FlipOrResize(screenPoint.y, mRect.height, screenRect.y,
                                   screenRect.YMost(), anchorRect.y, anchorRect.YMost(),
                                   margin.top, margin.bottom, offsetForContextMenu.y, vFlip,
                                   endAligned, &mVFlip);
     }
+    mIsOffset = mIsOffset || (preOffsetScreenPoint != screenPoint.y);
 
     NS_ASSERTION(screenPoint.x >= screenRect.x && screenPoint.y >= screenRect.y &&
                  screenPoint.x + mRect.width <= screenRect.XMost() &&
                  screenPoint.y + mRect.height <= screenRect.YMost(),
                  "Popup is offscreen");
   }
 
   // snap the popup's position in screen coordinates to device pixels,
@@ -1664,17 +1673,18 @@ nsMenuPopupFrame::SetPopupPosition(nsIFr
     // XXXndeakin can parentSize.width still extend outside?
     SetXULBounds(state, mRect);
   }
 
   // If the popup is in the positioned state or if it is shown and the position
   // or size changed, dispatch a popuppositioned event if the popup wants it.
   nsIntRect newRect(screenPoint.x, screenPoint.y, mRect.width, mRect.height);
   if (mPopupState == ePopupPositioning ||
-      (mPopupState == ePopupShown && !newRect.IsEqualEdges(mUsedScreenRect))) {
+      (mPopupState == ePopupShown && !newRect.IsEqualEdges(mUsedScreenRect)) ||
+      (mPopupState == ePopupShown && oldAlignmentOffset != mAlignmentOffset)) {
     mUsedScreenRect = newRect;
     if (aNotify) {
       nsXULPopupPositionedEvent::DispatchIfNeeded(mContent, false, false);
     }
   }
 
   return NS_OK;
 }
--- a/layout/xul/nsMenuPopupFrame.h
+++ b/layout/xul/nsMenuPopupFrame.h
@@ -631,16 +631,21 @@ protected:
   bool mGeneratedChildren; // true if the contents have been created
 
   bool mMenuCanOverlapOSBar;    // can we appear over the taskbar/menubar?
   bool mShouldAutoPosition; // Should SetPopupPosition be allowed to auto position popup?
   bool mInContentShell; // True if the popup is in a content shell
   bool mIsMenuLocked; // Should events inside this menu be ignored?
   bool mMouseTransparent; // True if this is a popup is transparent to mouse events
 
+  // True if this popup has been offset due to moving off / near the edge of the screen.
+  // (This is useful for ensuring that a move, which can't offset the popup, doesn't undo
+  // a previously set offset.)
+  bool mIsOffset;
+
   // the flip modes that were used when the popup was opened
   bool mHFlip;
   bool mVFlip;
 
   // How the popup is anchored.
   MenuPopupAnchorType mAnchorType;
 
   nsRect mOverrideConstraintRect;