Closed Bug 1623974 Opened 3 years ago Closed 3 years ago

[wayland] Attach popups to anchor rects

Categories

(Core :: Widget: Gtk, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla77
Tracking Status
firefox77 --- fixed

People

(Reporter: jhorak, Assigned: jhorak)

References

Details

Attachments

(6 files, 1 obsolete file)

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

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.

Assignee: nobody → jhorak
Status: NEW → ASSIGNED

The patch has a significant regression on Fedora 32.

Attached file run-fedora32.txt

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

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

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

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

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'
Flags: needinfo?(jhorak)
Attached image Screenshot from 2020-04-06 20-04-52.png (obsolete) —

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

(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.

(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.

(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.

: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
Flags: needinfo?(jhorak)

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

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

Attachment #9138602 - Attachment is obsolete: true

(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.

Flags: needinfo?(robert.mader)

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!

(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.

Attached file 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.

Flags: needinfo?(robert.mader)
Pushed by shindli@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/86630db403c1
[wayland] Attach popups to anchor rects; r=stransky

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!

Flags: needinfo?(jhorak)

(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.

Pushed by btara@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/57400551129c
[wayland] Attach popups to anchor rects; r=stransky
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla77
Regressions: 1632399
Flags: needinfo?(jhorak)

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?

(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.

(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

Regressions: 1634404
Regressions: 1640567
Regressions: 1641174
No longer regressions: 1641174
Regressions: 1643993
Regressions: 1645693
Regressions: 1645695
Regressions: 1654499
Regressions: 1661192
Regressions: 1734160
You need to log in before you can comment on or make changes to this bug.