1623974 - [wayland] Attach popups to anchor rects

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Jan Horak [:jhorak]]

Popups are currently attached to the pixel, not anchor rect. This breaks flipping popups near the monitor edge.

Comment 1

[Jan Horak [:jhorak]]

Attachment: Bug 1623974 [wayland] Attach popups to anchor rects; r=stransky

If we want correct popup placement we need to use the right anchor rect
for gdk_window_move_to_rect under Wayland. Patch exports the anchor rect from the
nsMenuPopupFrame to be used in nsWindow.

This patch also fixes popup overflowing the screen by using the size returned from
gdk_window_move_to_rect for the nsMenuPopupFrame.

Comment 2

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

Attachment: Screenshot from 2020-03-23 19-24-52.png

The patch has a significant regression on Fedora 32.

Comment 3

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

Attachment: run-fedora32.txt

Jan, there is wayland/widget log from Fedora 32 when the oversized popup is shown.

Comment 4

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

There's the related part:

[Parent 68969: Main Thread]: D/Widget NativeMoveResizeWaylandPopupCallback [0x7f7c7c5e3c00] flipped_x 0 flipped_y 0
[Parent 68969: Main Thread]: D/Widget flipped_rect x=-561 y=121 width=1920 height=1020
[Parent 68969: Main Thread]: D/Widget final_rect x=1 y=23 width=1920 height=1020
[Parent 68969: Main Thread]: D/Widget orig mBounds x=14 y=46 width=3840 height=2160
[Parent 68969: Main Thread]: D/Widget new mBounds x=2 y=46 width=3840 height=2040

Comment 5

[Jan Horak [:jhorak]]

Thanks guys for testing it, I've fixed the patch to work with scale factor.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/61cc7740daa3
[wayland] Attach popups to anchor rects; r=stransky

Comment 7

[Bogdan Tara[:bogdan_tara | bogdant]]

Backed out changeset 61cc7740daa3 (bug 1623974) for bustages related to nsWindow.cpp

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&fromchange=0bd7ed7cea6077e4020f571d23f74a3a94ed852f&tochange=6762dd78491c50505a7d63bd67405e161f88f4a6&searchStr=linux%2Cbuild&selectedJob=295376496

Backout link: https://hg.mozilla.org/integration/autoland/rev/08164afbc089b3373fc7bc33f9da11b0b428a89a

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=295376496&repo=autoland&lineNumber=32303

task 2020-03-30T14:18:32.559Z] 14:18:32     INFO -  make[4]: Entering directory '/builds/worker/workspace/obj-build/widget/gtk'
[task 2020-03-30T14:18:32.559Z] 14:18:32     INFO -  /builds/worker/fetches/sccache/sccache /builds/worker/fetches/gcc/bin/g++ -std=gnu++17 -o nsWindow.o -c  -I/builds/worker/workspace/obj-build/dist/stl_wrappers -I/builds/worker/workspace/obj-build/dist/system_wrappers -include /builds/worker/checkouts/gecko/config/gcc_hidden.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -DNDEBUG=1 -DTRIMMED=1 -DOS_POSIX=1 -DOS_LINUX=1 -DCAIRO_GFX '-DMOZ_APP_NAME="firefox"' -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -DSTATIC_EXPORTABLE_JS_API -I/builds/worker/checkouts/gecko/widget/gtk -I/builds/worker/workspace/obj-build/widget/gtk -I/builds/worker/workspace/obj-build/ipc/ipdl/_ipdlheaders -I/builds/worker/checkouts/gecko/ipc/chromium/src -I/builds/worker/checkouts/gecko/ipc/glue -I/builds/worker/checkouts/gecko/layout/base -I/builds/worker/checkouts/gecko/layout/generic -I/builds/worker/checkouts/gecko/layout/xul -I/builds/worker/checkouts/gecko/other-licenses/atk-1.0 -I/builds/worker/checkouts/gecko/widget -I/builds/worker/checkouts/gecko/widget/headless -I/builds/worker/checkouts/gecko/widget/x11 -I/builds/worker/workspace/obj-build/dist/include -I/builds/worker/workspace/obj-build/dist/include/nspr -I/builds/worker/workspace/obj-build/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/obj-build/mozilla-config.h -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wduplicated-cond -Wimplicit-fallthrough -Wunused-function -Wunused-variable -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=coverage-mismatch -Wno-error=free-nonheap-object -Wformat -Wformat-overflow=2 -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fno-aligned-new -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -freorder-blocks -O2 -fno-omit-frame-pointer -funwind-tables -Werror -I/builds/worker/checkouts/gecko/widget/gtk/compat-gtk3 -pthread -I/usr/include/gtk-3.0 -I/usr/include/pango-1.0 -I/usr/include/gio-unix-2.0/ -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/freetype2 -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/libpng12 -I/usr/include/gtk-3.0/unix-print -I/usr/include/dbus-1.0 -I/usr/lib/x86_64-linux-gnu/dbus-1.0/include -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -Wno-error=shadow  -MD -MP -MF .deps/nsWindow.o.pp   /builds/worker/checkouts/gecko/widget/gtk/nsWindow.cpp
[task 2020-03-30T14:18:32.559Z] 14:18:32     INFO -  /builds/worker/checkouts/gecko/widget/gtk/nsWindow.cpp: In member function 'void nsWindow::NativeMoveResizeWaylandPopup(GdkPoint*, GdkRectangle*)':
[task 2020-03-30T14:18:32.559Z] 14:18:32    ERROR -  /builds/worker/checkouts/gecko/widget/gtk/nsWindow.cpp:1613:25: error: 'class nsMenuPopupFrame' has no member named 'GetPopupAlignment'; did you mean 'mPopupAlignment'?
[task 2020-03-30T14:18:32.559Z] 14:18:32     INFO -       switch (popupFrame->GetPopupAlignment()) {
[task 2020-03-30T14:18:32.559Z] 14:18:32     INFO -                           ^~~~~~~~~~~~~~~~~
[task 2020-03-30T14:18:32.559Z] 14:18:32     INFO -                           mPopupAlignment
[task 2020-03-30T14:18:32.559Z] 14:18:32     INFO -  /builds/worker/checkouts/gecko/widget/gtk/nsWindow.cpp: At global scope:
[task 2020-03-30T14:18:32.559Z] 14:18:32    ERROR -  /builds/worker/checkouts/gecko/widget/gtk/nsWindow.cpp:1449:19: error: 'GdkGravity PopupAlignmentToGdkGravity(int8_t)' defined but not used [-Werror=unused-function]
[task 2020-03-30T14:18:32.559Z] 14:18:32     INFO -   static GdkGravity PopupAlignmentToGdkGravity(int8_t aAlignment) {
[task 2020-03-30T14:18:32.559Z] 14:18:32     INFO -                     ^~~~~~~~~~~~~~~~~~~~~~~~~~
[task 2020-03-30T14:18:32.559Z] 14:18:32     INFO -  cc1plus: all warnings being treated as errors
[task 2020-03-30T14:18:32.559Z] 14:18:32     INFO -  /builds/worker/checkouts/gecko/config/rules.mk:750: recipe for target 'nsWindow.o' failed
[task 2020-03-30T14:18:32.559Z] 14:18:32    ERROR -  make[4]: *** [nsWindow.o] Error 1
[task 2020-03-30T14:18:32.559Z] 14:18:32     INFO -  make[4]: Leaving directory '/builds/worker/workspace/obj-build/widget/gtk'
[task 2020-03-30T14:18:32.559Z] 14:18:32     INFO -  make[4]: *** Waiting for unfinished jobs....
[task 2020-03-30T14:18:32.559Z] 14:18:32     INFO -  make[4]: Entering directory '/builds/worker/workspace/obj-build/widget'
[task 2020-03-30T14:18:32.559Z] 14:18:32     INFO -  widget/nsShmImage.o
[task 2020-03-30T14:18:32.559Z] 14:18:32     INFO -  make[4]: Leaving directory '/builds/worker/workspace/obj-build/widget'

Comment 8

[Robert Mader [:rmader]]

Attachment: Screenshot from 2020-04-06 20-04-52.png

As far as I can see there are still some serious issues with this that need to get ironed out before it can land :/

Comment 9

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

(In reply to Robert Mader [:rmader] from comment #8)

Created attachment 9138602 [details]
Screenshot from 2020-04-06 20-04-52.png

As far as I can see there are still some serious issues with this that need to get ironed out before it can land :/

That can be also regression from Bug 1627469.

Comment 10

[Robert Mader [:rmader]]

(In reply to Martin Stránský [:stransky] from comment #9)

That can be also regression from Bug 1627469.

I can see that with D67810 applied, not without.

Comment 11

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

(In reply to Robert Mader [:rmader] from comment #10)

(In reply to Martin Stránský [:stransky] from comment #9)

That can be also regression from Bug 1627469.

I can see that with D67810 applied, not without.

I see, thanks for the info.

Comment 12

[Jan Horak [:jhorak]]

:rmader - thanks a lot for testing. I've just attached updated version. Please feel free to look at it. What should also be fixed is RMB popup in the bottom right location (which triggered action from the window automatically on mouseup).
Known issues:

1. overflowing bookmarks in the Library menu - basically resizing widget is not positioned by gdk_window_move_to_rect.
2. wrong offset of popup menus in the camera/sound settings when trying to open webrtc page.
3. disappearing tooltips near the top-right screen boundaries

Comment 13

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

Attachment: a regression when all popups are truncated

Comment 14

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

Let's wait until the gtk3 fixes mentioned at https://bugzilla.mozilla.org/show_bug.cgi?id=1628793#c5 lands.

Comment 15

[Robert Mader [:rmader]]

Attachment: Screenshot from 2020-04-17 13-12-00.png

While the latest revision looks very good to me in many respects, unfortunately I still see misplaced popups.

Comment 16

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

(In reply to Robert Mader [:rmader] from comment #15)

Created attachment 9141228 [details]
Screenshot from 2020-04-17 13-12-00.png

While the latest revision looks very good to me in many respects, unfortunately I still see misplaced popups.

Robert, thanks for the report.
Can you please specify how to reproduce it? I have latest F32 and I don't see that:

gtk3-3.24.18-1.fc32.x86_64
mutter-3.36.1-4.fc32.x86_64
gnome-shell-3.36.1-4.fc32.x86_64

Also I have one 4K display with 200% scale.

Thanks.

Comment 17

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

Also it would be useful to have a log of the popup, can you run firefox with these env:

MOZ_LOG="Widget:5, WidgetWayland:5" WAYLAND_DEBUG=1 MOZ_ENABLE_WAYLAND=1

and attach the log here?
Thanks a lot!

Comment 18

[Jan Horak [:jhorak]]

(In reply to Robert Mader [:rmader] from comment #15)

Created attachment 9141228 [details]
Screenshot from 2020-04-17 13-12-00.png

While the latest revision looks very good to me in many respects, unfortunately I still see misplaced popups.

Looks like the panel has been flipped. Knowing the anchor rect would help there. It's also in the logs.

Comment 19

[Robert Mader [:rmader]]

Attachment: firefox.log

Ah, found the issue! I'm at 1920x1080 with scale 1, but my fonts are set to 1.25x in gnome-tweaks. When I set it back to 1.00 the issue is gone.

On Mutter, GS and GTK 3 HEAD - but the recent popup changes went to GTK 3.24.18, so you should have them.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/86630db403c1
[wayland] Attach popups to anchor rects; r=stransky

Comment 21

[Robert Mader [:rmader]]

Jan, as you now pushed without further changes, should we create a follow up bug for misplaced menus when using font scaling != 1.00? I guess it's far from uncommon to have bigger fonts and it's a regression compared to before this patch (and also to X11).

P.S.: nice to see this landing!

Comment 22

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

(In reply to Robert Mader [:rmader] from comment #21)

Jan, as you now pushed without further changes, should we create a follow up bug for misplaced menus when using font scaling != 1.00? I guess it's far from uncommon to have bigger fonts and it's a regression compared to before this patch (and also to X11).

Thanks for testing Robert, I created it as 1631458.

Comment 23

[Alexandru Michis [:malexandru]]

Backed out changeset 86630db403c1 for causing failures in browser_toolbox_zoom_popup.js and in wpt.

Backout link: https://hg.mozilla.org/integration/autoland/rev/68e09b2a466734fb4ec6a4bac3db89a0d5d2b99d

Push with failures:

• https://treeherder.mozilla.org/#/jobs?repo=autoland&fromchange=18205ec9467e95cdbe2d3ae3b29aa66a3f9c5e97&searchStr=linux%2C18.04%2Cx64%2Cquantumrender%2Cdebug%2Cweb%2Cplatform%2Ctests%2Cwith%2Cfission%2Cenabled%2Ctest-linux1804-64-qr%2Fdebug-web-platform-tests-fis-e10s-16%2Cw-fis%28wpt16%29&tochange=68e09b2a466734fb4ec6a4bac3db89a0d5d2b99d&selectedJob=298404980
• https://treeherder.mozilla.org/#/jobs?repo=autoland&fromchange=97c731887ed2421789d23bac3ba96301aba037dc&searchStr=linux%2C18.04%2Cx64%2Cdebug%2Cmochitests%2Ctest-linux1804-64%2Fdebug-mochitest-devtools-chrome-e10s-3%2Cm%28dt3%29&tochange=68e09b2a466734fb4ec6a4bac3db89a0d5d2b99d&selectedJob=298409768

Failure logs:

• https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=298404980&repo=autoland&lineNumber=20450
• https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=298409768&repo=autoland&lineNumber=9562

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/57400551129c
[wayland] Attach popups to anchor rects; r=stransky

Comment 25

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/57400551129c

Comment 26

[dm]

Hi, I usually use firefox with layout.css.devPixelsPerPx=1.6 and since 57400551129c73bf1a5b738e7fa03d331c02601d the menus are misplaced. Unsetting the variable resolve the issue.
Should I create a separate bug?

Comment 27

[Robert Mader [:rmader]]

(In reply to d.masserut from comment #26)

Hi, I usually use firefox with layout.css.devPixelsPerPx=1.6 and since 57400551129c73bf1a5b738e7fa03d331c02601d the menus are misplaced. Unsetting the variable resolve the issue.
Should I create a separate bug?

Yes, please. You can mark it as regressed by this bug.

Comment 28

[dm]

(In reply to Robert Mader [:rmader] from comment #27)

(In reply to d.masserut from comment #26)

Hi, I usually use firefox with layout.css.devPixelsPerPx=1.6 and since 57400551129c73bf1a5b738e7fa03d331c02601d the menus are misplaced. Unsetting the variable resolve the issue.
Should I create a separate bug?

Yes, please. You can mark it as regressed by this bug.

https://bugzilla.mozilla.org/show_bug.cgi?id=1634404