Bug 1786525 - Don't update untransformed anchor rect when moved by move-to-rect. r=stransky

Otherwise, it changes the move-to-rect inputs, which can change the
output as well, making us move the anchor all the way to the right.

You could, I guess, consider this a mutter bug of sorts, because it
feels weird that you pass it an anchor that has been a `move-to-rect`
output and you get another rect as an output.

But also, it's kinda silly that we're doing that to begin with, so avoid
it by telling the popup frame whether it's been positioned / moved by
move-to-rect (and keeping the anchor in that case).

The reason this works on my setup without "Large text" is just dumb luck
(the front-end computes a max-height for the panel that is small enough
to fit on the screen).

Differential Revision: https://phabricator.services.mozilla.com/D155406
This commit is contained in:
Emilio Cobos Álvarez 2022-08-29 21:47:34 +00:00
parent b25989bd51
commit cf74fba975
13 changed files with 48 additions and 29 deletions

View File

@ -819,6 +819,7 @@ void nsMenuPopupFrame::InitializePopup(nsIContent* aAnchorContent,
mHFlip = false;
mAlignmentOffset = 0;
mPositionedOffset = 0;
mPositionedByMoveToRect = false;
mAnchorType = aAnchorType;
@ -1537,12 +1538,16 @@ nsresult nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame,
// tell us which axis the popup is flush against in case we have to move
// it around later. The AdjustPositionForAnchorAlign method accounts for
// the popup's margin.
if (!mPositionedByMoveToRect) {
mUntransformedAnchorRect = anchorRect;
}
screenPoint = AdjustPositionForAnchorAlign(anchorRect, hFlip, vFlip);
} else {
// with no anchor, the popup is positioned relative to the root frame
anchorRect = rootScreenRect;
if (!mPositionedByMoveToRect) {
mUntransformedAnchorRect = anchorRect;
}
screenPoint = anchorRect.TopLeft() + nsPoint(margin.left, margin.top);
}
@ -1579,7 +1584,9 @@ nsresult nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame,
} else {
screenPoint = mScreenRect.TopLeft();
anchorRect = nsRect(screenPoint, nsSize());
if (!mPositionedByMoveToRect) {
mUntransformedAnchorRect = anchorRect;
}
// Right-align RTL context menus, and apply margin and offsets as per the
// platform conventions.
@ -2408,7 +2415,8 @@ nsMargin nsMenuPopupFrame::GetMargin() const {
return margin;
}
void nsMenuPopupFrame::MoveTo(const CSSPoint& aPos, bool aUpdateAttrs) {
void nsMenuPopupFrame::MoveTo(const CSSPoint& aPos, bool aUpdateAttrs,
bool aByMoveToRect) {
nsIWidget* widget = GetWidget();
nsPoint appUnitsPos = CSSPixel::ToAppUnits(aPos);
@ -2433,6 +2441,7 @@ void nsMenuPopupFrame::MoveTo(const CSSPoint& aPos, bool aUpdateAttrs) {
return;
}
mPositionedByMoveToRect = aByMoveToRect;
mScreenRect.MoveTo(appUnitsPos);
if (mAnchorType == MenuPopupAnchorType_Rect) {
// This ensures that the anchor width is still honored, to prevent it from

View File

@ -318,7 +318,8 @@ class nsMenuPopupFrame final : public nsBoxFrame,
// If aUpdateAttrs is true, and the popup already has left or top attributes,
// then those attributes are updated to the new location.
// The frame may be destroyed by this method.
void MoveTo(const mozilla::CSSPoint& aPos, bool aUpdateAttrs);
void MoveTo(const mozilla::CSSPoint& aPos, bool aUpdateAttrs,
bool aByMoveToRect = false);
void MoveToAnchor(nsIContent* aAnchorContent, const nsAString& aPosition,
int32_t aXPos, int32_t aYPos, bool aAttributesOverride);
@ -568,6 +569,11 @@ class nsMenuPopupFrame final : public nsBoxFrame,
// without margins applied, as GTK is what takes care of determining how to
// flip etc. on Wayland.
nsRect mUntransformedAnchorRect;
// Whether we were moved by the move-to-rect Wayland callback. In that case,
// we stop updating the anchor so that we can end up with a stable position.
bool mPositionedByMoveToRect = false;
// Store SizedToPopup attribute for MoveTo call to avoid
// unwanted popup resize there.
bool mSizedToPopup = false;

View File

@ -558,7 +558,8 @@ static nsMenuPopupFrame* GetPopupToMoveOrResize(nsIFrame* aFrame) {
return menuPopupFrame;
}
void nsXULPopupManager::PopupMoved(nsIFrame* aFrame, nsIntPoint aPnt) {
void nsXULPopupManager::PopupMoved(nsIFrame* aFrame, nsIntPoint aPnt,
bool aByMoveToRect) {
nsMenuPopupFrame* menuPopupFrame = GetPopupToMoveOrResize(aFrame);
if (!menuPopupFrame) {
return;
@ -591,7 +592,7 @@ void nsXULPopupManager::PopupMoved(nsIFrame* aFrame, nsIntPoint aPnt) {
} else {
CSSPoint cssPos = LayoutDeviceIntPoint::FromUnknownPoint(aPnt) /
menuPopupFrame->PresContext()->CSSToDevPixelScale();
menuPopupFrame->MoveTo(cssPos, false);
menuPopupFrame->MoveTo(cssPos, false, aByMoveToRect);
}
}

View File

@ -659,9 +659,9 @@ class nsXULPopupManager final : public nsIDOMEventListener,
/**
* Indicate that the popup associated with aView has been moved to the
* specified screen coordiates.
* specified screen coordinates.
*/
void PopupMoved(nsIFrame* aFrame, nsIntPoint aPoint);
void PopupMoved(nsIFrame* aFrame, nsIntPoint aPoint, bool aByMoveToRect);
/**
* Indicate that the popup associated with aView has been resized to the

View File

@ -919,10 +919,12 @@ static bool IsPopupWidget(nsIWidget* aWidget) {
PresShell* nsView::GetPresShell() { return GetViewManager()->GetPresShell(); }
bool nsView::WindowMoved(nsIWidget* aWidget, int32_t x, int32_t y) {
bool nsView::WindowMoved(nsIWidget* aWidget, int32_t x, int32_t y,
ByMoveToRect aByMoveToRect) {
nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
if (pm && IsPopupWidget(aWidget)) {
pm->PopupMoved(mFrame, nsIntPoint(x, y));
pm->PopupMoved(mFrame, nsIntPoint(x, y),
aByMoveToRect == ByMoveToRect::Yes);
return true;
}

View File

@ -444,7 +444,8 @@ class nsView final : public nsIWidgetListener {
// nsIWidgetListener
virtual mozilla::PresShell* GetPresShell() override;
virtual nsView* GetView() override { return this; }
virtual bool WindowMoved(nsIWidget* aWidget, int32_t x, int32_t y) override;
virtual bool WindowMoved(nsIWidget* aWidget, int32_t x, int32_t y,
ByMoveToRect) override;
virtual bool WindowResized(nsIWidget* aWidget, int32_t aWidth,
int32_t aHeight) override;
#if defined(MOZ_WIDGET_ANDROID)

View File

@ -1891,12 +1891,7 @@ void nsWindow::WaylandPopupPropagateChangesToLayout(bool aMove, bool aResize) {
}
if (aMove) {
LOG(" needPositionUpdate, bounds [%d, %d]", mBounds.x, mBounds.y);
NotifyWindowMoved(mBounds.x, mBounds.y);
if (nsMenuPopupFrame* popupFrame = GetMenuPopupFrame(GetFrame())) {
auto p = CSSIntPoint::Round(
mBounds.TopLeft() / popupFrame->PresContext()->CSSToDevPixelScale());
popupFrame->MoveTo(p, true);
}
NotifyWindowMoved(mBounds.x, mBounds.y, ByMoveToRect::Yes);
}
}

View File

@ -1641,9 +1641,10 @@ void nsBaseWidget::NotifyWindowDestroyed() {
}
}
void nsBaseWidget::NotifyWindowMoved(int32_t aX, int32_t aY) {
void nsBaseWidget::NotifyWindowMoved(int32_t aX, int32_t aY,
ByMoveToRect aByMoveToRect) {
if (mWidgetListener) {
mWidgetListener->WindowMoved(this, aX, aY);
mWidgetListener->WindowMoved(this, aX, aY, aByMoveToRect);
}
if (mIMEHasFocus && IMENotificationRequestsRef().WantPositionChanged()) {

View File

@ -355,7 +355,10 @@ class nsBaseWidget : public nsIWidget, public nsSupportsWeakReference {
void NotifyWindowDestroyed();
void NotifySizeMoveDone();
void NotifyWindowMoved(int32_t aX, int32_t aY);
using ByMoveToRect = nsIWidgetListener::ByMoveToRect;
void NotifyWindowMoved(int32_t aX, int32_t aY,
ByMoveToRect = ByMoveToRect::No);
void SetNativeData(uint32_t aDataType, uintptr_t aVal) override {}

View File

@ -21,8 +21,8 @@ nsView* nsIWidgetListener::GetView() { return nullptr; }
PresShell* nsIWidgetListener::GetPresShell() { return nullptr; }
bool nsIWidgetListener::WindowMoved(nsIWidget* aWidget, int32_t aX,
int32_t aY) {
bool nsIWidgetListener::WindowMoved(nsIWidget* aWidget, int32_t aX, int32_t aY,
ByMoveToRect) {
return false;
}

View File

@ -65,7 +65,9 @@ class nsIWidgetListener {
* Called when a window is moved to location (x, y). Returns true if the
* notification was handled. Coordinates are outer window screen coordinates.
*/
virtual bool WindowMoved(nsIWidget* aWidget, int32_t aX, int32_t aY);
enum class ByMoveToRect : bool { No, Yes };
virtual bool WindowMoved(nsIWidget* aWidget, int32_t aX, int32_t aY,
ByMoveToRect);
/**
* Called when a window is resized to (width, height). Returns true if the

View File

@ -113,11 +113,8 @@ using dom::AutoNoJSAPI;
using dom::BrowserHost;
using dom::BrowsingContext;
using dom::Document;
using dom::DocumentL10n;
using dom::Element;
using dom::EventTarget;
using dom::LoadURIOptions;
using dom::Promise;
AppWindow::AppWindow(uint32_t aChromeFlags)
: mChromeTreeOwner(nullptr),
@ -3330,7 +3327,8 @@ PresShell* AppWindow::WidgetListenerDelegate::GetPresShell() {
}
bool AppWindow::WidgetListenerDelegate::WindowMoved(nsIWidget* aWidget,
int32_t aX, int32_t aY) {
int32_t aX, int32_t aY,
ByMoveToRect) {
RefPtr<AppWindow> holder = mAppWindow;
return holder->WindowMoved(aWidget, aX, aY);
}

View File

@ -91,7 +91,8 @@ class AppWindow final : public nsIBaseWindow,
MOZ_CAN_RUN_SCRIPT_BOUNDARY
virtual mozilla::PresShell* GetPresShell() override;
MOZ_CAN_RUN_SCRIPT_BOUNDARY
virtual bool WindowMoved(nsIWidget* aWidget, int32_t x, int32_t y) override;
virtual bool WindowMoved(nsIWidget* aWidget, int32_t x, int32_t y,
ByMoveToRect) override;
MOZ_CAN_RUN_SCRIPT_BOUNDARY
virtual bool WindowResized(nsIWidget* aWidget, int32_t aWidth,
int32_t aHeight) override;