1759157 - [Wayland] Add bookmark panel jumps to top left corner of window

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect, P3)

Description

[Kevin Locke]

With Firefox Nightly (Build 20220311094123) on Wayland, the "Add Bookmark" panel sometimes jumps to the top-left corner of the window when opening the Location drop-down menu. To reproduce:

1. Press the "Bookmark this page" star in the address bar or press Ctrl-D.
2. Click on the Location drop-down menu to open it.
3. If the panel does not jump to the top-left corner, click "Show all the bookmarks folders" to toggle the list of all bookmarks folders, then proceed to step 2. (The issue usually occurs on the first or second try. It can occur after showing or after hiding the list of all bookmarks folders.)

Pushlog https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=fc528b1586e59937e3b2fbfb4978c5617f10295e&tochange=b2b38f41ccbc233442ea106a0b0f33856d55fd97 suggests this was regressed by Bug 1755323.

Comment 1

[Kevin Locke]

Note: I'm using the sway compositor, version 1.6.1, on Debian testing.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1755323

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

:stransky, since you are the author of the regressor, bug 1755323, could you take a look?
For more information, please visit auto_nag documentation.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

I can't repro on kwin fwiw.

Comment 5

[Martin Stránský [:stransky] (ni? me)]

Please run on terminal with MOZ_LOG="WidgetPopup:5" env variable and attach the log here.
Thanks.

Comment 6

[Kevin Locke]

Attachment: MOZ_LOG="WidgetPopup:5" on build 20220314094248

Here's the log with MOZ_LOG="WidgetPopup:5". The actions shown in the log:

1. Click "Bookmark this page" star in address bar.
2. Click "Show all the bookmarks folders" button.
3. Click the Location drop-down list.

At this point the Add bookmark panel jumped to the top-left corner. I then clicked cancel and exited.

Comment 7

[Martin Stránský [:stransky] (ni? me)]

Please try latest nightly:
https://fedoraproject.org/wiki/How_to_debug_Firefox_problems#Testing_Mozilla_binaries

If you see this bug in latest nightly, try to set widget.wayland.use-move-to-rect to false and restart browser.
Thanks.

Comment 8

[Kevin Locke]

I can confirm the issue is reproducible with widget.wayland.use-move-to-rect=false on build 20220314094248.

Comment 9

[Martin Stránský [:stransky] (ni? me)]

(In reply to Kevin Locke from comment #8)

I can confirm the issue is reproducible with widget.wayland.use-move-to-rect=false on build 20220314094248.

Interesting. Can you attach MOZ_LOG="WidgetPopup:5" log of that?
Thanks.

Comment 10

[Kevin Locke]

Attachment: MOZ_LOG="WidgetPopup:5" with widget.wayland.use-move-to-rect:false on build 20220314094248

Sure thing. Here's the log with MOZ_LOG="WidgetPopup:5" and widget.wayland.use-move-to-rect:false.

Comment 11

[Martin Stránský [:stransky] (ni? me)]

I think I can reproduce it in some case under mutter.

Comment 12

[Martin Stránský [:stransky] (ni? me)]

It's actually a regression from https://phabricator.services.mozilla.com/D140263 (Bug 1757996).
We set mUntransformedAnchorRect even when IsAnchored() is false, i.e. we overwrite original anchor by point.

The sequence is:

1. anchor is correctly set by layout
2. popup is resized by move-to-rect, mositon is not updated by layout. When it's moved in callback , new popup origin is placed to anchor instead of the layout one.
3. we have wrong anchor now - it can be located outside of main window and xdg-positioner fails.

Comment 13

[Martin Stránský [:stransky] (ni? me)]

We also should use some assertion as anchor can't have negative coordinates on Wayland or be outside of parent window.

Comment 14

[Martin Stránský [:stransky] (ni? me)]

It happens only when size of popup is updated but we use position from previous move as anchor (we overwrite mUntransformedAnchorRect with popup position by nsMenuPopupFrame::MoveTo()).

Comment 15

[Emilio Cobos Álvarez (:emilio)]

Why would the position passed to MoveTo be wrong though? I guess it can get messy with flips and such?

Comment 16

[Emilio Cobos Álvarez (:emilio)]

(I have been running on sway yesterday and today and haven't seen the issue on Nightly, fwiw)

Comment 17

[Martin Stránský [:stransky] (ni? me)]

I have a patch almost ready.

Comment 18

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1759157 [Wayland] Save layout popup setup on Wayland r?emilio

When a popup is only resized (and not moved), we need original move-to-rect setup from layout from previous popup placement to replicate move-to-rect call with updated size.
We can't get complete popup setup from layout for resize only event. It's because we use nsMenuPopupFrame::MoveTo() to place popup from move-to-rect callback (which happened before the resize event) and such direct popup move temporary change popup anchor type to point as we place popup to specified location from move-to-rect callback.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6697eafee26f
[Wayland] Save layout popup setup on Wayland r=emilio

Comment 20

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/6697eafee26f