Closed Bug 1423598 Opened 7 years ago Closed 6 years ago

[wayland] Use gdk_window_move_to_rect() to place popup/tooltip windows

Categories

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

Product:
Core
Component:
Widget: Gtk
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla68
Tracking Flags:
Tracking Status
firefox59 --- wontfix
firefox68 --- fixed

People

(Reporter: jhorak, Assigned: stransky)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

Bug 1423598 - [Wayland] Use gdk_window_move_to_rect() to place popup windows, r=ashie
47 bytes, text/x-phabricator-request
Details | Review
In wayland backend the tooltip and popup menu windows are created as subsurface. This breaks correction of their position which ensures that window is not drawn outside of the screen or workarea. This is done by gdk/wayland [1] (and later there [3]) by default when the window type is GDK_WINDOW_TEMP as tested there [2]. [1] https://git.gnome.org/browse/gtk+/tree/gdk/wayland/gdkwindow-wayland.c?h=gtk-3-22#n2345 [2] https://git.gnome.org/browse/gtk+/tree/gdk/wayland/gdkwindow-wayland.c?h=gtk-3-22#n2239 [3] https://git.gnome.org/browse/gtk+/tree/gdk/wayland/gdkwindow-wayland.c?h=gtk-3-22#n2011
Or better GDK_WINDOW_TYPE_HINT_POPUP_MENU, GDK_WINDOW_TYPE_HINT_DROPDOWN_MENU or GDK_WINDOW_TYPE_HINT_COMBO
Severity: normal → minor
Priority: -- → P5
Recent wayland code creates popup windows as subsurface. We need to implement popups as xdg_popup which means we need to use gdk_seat_grab() and show the popup here (Bug 1377084). We use subsurfaces for popups because of: https://bugzilla.gnome.org/show_bug.cgi?id=783957
Severity: minor → normal
Depends on: 1377084
Priority: P5 → P2
We need to make gdk_window_move_to_rect() symbol public first to fix this: https://bugzilla.gnome.org/show_bug.cgi?id=791845
gdk_window_move_rect() is now public in GTK 3.24 and GTK maste:. https://gitlab.gnome.org/GNOME/gtk/issues/997
(In reply to wvengen from comment #4) > gdk_window_move_rect() is now public in GTK 3.24 and GTK maste:. > https://gitlab.gnome.org/GNOME/gtk/issues/997 Good news everyone!
Just like to inform that on swaywm this is indeed happening on multi-screens displays with positive Y position. Issue is solved by RedSoxFan as being an deprecation of gtk_menu_popup(). https://github.com/swaywm/sway/issues/3135#issuecomment-448794177
Assignee: jhorak → stransky
No longer depends on: 1377084

Wayland refuses to open two popups with the same parent, so we use the subsurfaces window which are only an emulation of the proper popup behavior and causes the issues here.

When we create multilevel menus (like View -> Toolbars) we need to create "Toolbars" menu with "View" parent and not use a toplevel window as a parent of both.

I wonder if the xul popup code can be adjusted for it or we need to track popup creation at toolkit/widget level.

As a workaround we can use pointer grab/release - it seems to identify the active popup reliably.

Depends on: 1532643
Attached patch popup-wip.patch (obsolete) — — Splinter Review

WIP patch, almost working, needs better detection of anchor type and Patch from Bug 1532643.

Attachment #9049616 - Attachment is obsolete: true

This was working for a few builds in the last week or so, but has reverted back to drawing off screen again :(

Summary: [wayland] popup/tooltip windows can be drawn off the screen → [wayland] Use gdk_window_move_to_rect() to place popup/tooltip windows

Hi, it's ready as the dependent bug has landed. Thanks.

Flags: needinfo?(ashie)
Flags: needinfo?(ashie)

Pushed by btara@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c11577f6743e
[Wayland] Use gdk_window_move_to_rect() to place popup windows, r=ashie

Keywords: checkin-needed
Backout by rgurzau@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/90696b419826 Backed out changeset c11577f6743e for build bustage at /gtk/nsWindow.cpp. on a CLOSED TREE.

Backed out changeset c11577f6743e (bug 1423598) for build bustage at /gtk/nsWindow.cpp. on a CLOSED TREE.

Backout link: https://hg.mozilla.org/integration/autoland/rev/90696b419826b5b054132070bc93d91f19a9225d

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=c11577f6743e425a35ce19773b7552b84a38f70a&selectedJob=242540526

Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=242540526&repo=autoland&lineNumber=21471

Log snippet:

[task 2019-04-25T10:29:39.698Z] 10:29:39 INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/toolkit/components/remote'
[task 2019-04-25T10:29:39.814Z] 10:29:39 INFO - make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/extensions/spellcheck/src'
[task 2019-04-25T10:29:39.817Z] 10:29:39 INFO - /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/clang/bin/clang++ -m32 -o Unified_cpp_spellcheck_src0.o -c -I/builds/worker/workspace/build/src/obj-firefox/dist/stl_wrappers -I/builds/worker/workspace/build/src/obj-firefox/dist/system_wrappers -include /builds/worker/workspace/build/src/config/gcc_hidden.h -DDEBUG=1 -DOS_POSIX=1 -DOS_LINUX=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/builds/worker/workspace/build/src/extensions/spellcheck/src -I/builds/worker/workspace/build/src/obj-firefox/extensions/spellcheck/src -I/builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -I/builds/worker/workspace/build/src/ipc/chromium/src -I/builds/worker/workspace/build/src/ipc/glue -I/builds/worker/workspace/build/src/extensions/spellcheck/hunspell/glue -I/builds/worker/workspace/build/src/extensions/spellcheck/hunspell/src -I/builds/worker/workspace/build/src/dom/base -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/mozilla-config.h -Qunused-arguments -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++1z-compat -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=backend-plugin -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -Wno-return-type-c-linkage -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fcrash-diagnostics-dir=/builds/worker/artifacts -march=pentium-m -msse -msse2 -mfpmath=sse -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/build/src/obj-firefox/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -Os -fno-omit-frame-pointer -funwind-tables -Werror -Wno-error=shadow -MD -MP -MF .deps/Unified_cpp_spellcheck_src0.o.pp /builds/worker/workspace/build/src/obj-firefox/extensions/spellcheck/src/Unified_cpp_spellcheck_src0.cpp
[task 2019-04-25T10:29:39.817Z] 10:29:39 INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/extensions/spellcheck/src'
[task 2019-04-25T10:29:39.819Z] 10:29:39 INFO - make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/widget/gtk'
[task 2019-04-25T10:29:39.822Z] 10:29:39 INFO - /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/clang/bin/clang++ -m32 -o nsWindow.o -c -I/builds/worker/workspace/build/src/obj-firefox/dist/stl_wrappers -I/builds/worker/workspace/build/src/obj-firefox/dist/system_wrappers -include /builds/worker/workspace/build/src/config/gcc_hidden.h -DDEBUG=1 -DOS_POSIX=1 -DOS_LINUX=1 -DCAIRO_GFX '-DMOZ_APP_NAME="firefox"' -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/builds/worker/workspace/build/src/widget/gtk -I/builds/worker/workspace/build/src/obj-firefox/widget/gtk -I/builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -I/builds/worker/workspace/build/src/ipc/chromium/src -I/builds/worker/workspace/build/src/ipc/glue -I/builds/worker/workspace/build/src/layout/base -I/builds/worker/workspace/build/src/layout/generic -I/builds/worker/workspace/build/src/layout/xul -I/builds/worker/workspace/build/src/other-licenses/atk-1.0 -I/builds/worker/workspace/build/src/widget -I/builds/worker/workspace/build/src/widget/headless -I/builds/worker/workspace/build/src/widget/x11 -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/mozilla-config.h -Qunused-arguments -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++1z-compat -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=backend-plugin -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -Wno-return-type-c-linkage -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fcrash-diagnostics-dir=/builds/worker/artifacts -march=pentium-m -msse -msse2 -mfpmath=sse -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/build/src/obj-firefox/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -Os -fno-omit-frame-pointer -funwind-tables -Werror -I/builds/worker/workspace/build/src/widget/gtk/compat-gtk3 -pthread -I/usr/include/gtk-3.0 -I/usr/include/atk-1.0 -I/usr/include/at-spi2-atk/2.0 -I/usr/include/pango-1.0 -I/usr/include/gio-unix-2.0/ -I/usr/include/cairo -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/glib-2.0 -I/usr/lib/i386-linux-gnu/glib-2.0/include -I/usr/include/harfbuzz -I/usr/include/freetype2 -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/i386-linux-gnu/dbus-1.0/include -I/usr/include/glib-2.0 -I/usr/lib/i386-linux-gnu/glib-2.0/include -Wno-error=shadow -MD -MP -MF .deps/nsWindow.o.pp /builds/worker/workspace/build/src/widget/gtk/nsWindow.cpp
[task 2019-04-25T10:29:39.822Z] 10:29:39 ERROR - /builds/worker/workspace/build/src/widget/gtk/nsWindow.cpp:1135:66: error: C++ requires a type specifier for all declarations
[task 2019-04-25T10:29:39.822Z] 10:29:39 INFO - GdkWindow *, const GdkRectangle *, GdkGravity, GdkGravity, GdkAnchorHints,
[task 2019-04-25T10:29:39.822Z] 10:29:39 INFO - ^
[task 2019-04-25T10:29:39.822Z] 10:29:39 ERROR - /builds/worker/workspace/build/src/widget/gtk/nsWindow.cpp:1188:3: error: unknown type name 'GdkAnchorHints'
[task 2019-04-25T10:29:39.823Z] 10:29:39 INFO - GdkAnchorHints hints = GdkAnchorHints(GDK_ANCHOR_SLIDE | GDK_ANCHOR_FLIP);
[task 2019-04-25T10:29:39.823Z] 10:29:39 INFO - ^
[task 2019-04-25T10:29:39.823Z] 10:29:39 ERROR - /builds/worker/workspace/build/src/widget/gtk/nsWindow.cpp:1188:41: error: use of undeclared identifier 'GDK_ANCHOR_SLIDE'
[task 2019-04-25T10:29:39.823Z] 10:29:39 INFO - GdkAnchorHints hints = GdkAnchorHints(GDK_ANCHOR_SLIDE | GDK_ANCHOR_FLIP);
[task 2019-04-25T10:29:39.823Z] 10:29:39 INFO - ^
[task 2019-04-25T10:29:39.823Z] 10:29:39 ERROR - /builds/worker/workspace/build/src/widget/gtk/nsWindow.cpp:1188:60: error: use of undeclared identifier 'GDK_ANCHOR_FLIP'
[task 2019-04-25T10:29:39.823Z] 10:29:39 INFO - GdkAnchorHints hints = GdkAnchorHints(GDK_ANCHOR_SLIDE | GDK_ANCHOR_FLIP);
[task 2019-04-25T10:29:39.823Z] 10:29:39 INFO - ^
[task 2019-04-25T10:29:39.823Z] 10:29:39 ERROR - /builds/worker/workspace/build/src/widget/gtk/nsWindow.cpp:1190:36: error: use of undeclared identifier 'GDK_ANCHOR_RESIZE'; did you mean 'GDK_DECOR_RESIZEH'?
[task 2019-04-25T10:29:39.823Z] 10:29:39 INFO - hints = GdkAnchorHints(hints | GDK_ANCHOR_RESIZE);
[task 2019-04-25T10:29:39.823Z] 10:29:39 INFO - ^~~~~~~~~~~~~~~~~
[task 2019-04-25T10:29:39.823Z] 10:29:39 INFO - GDK_DECOR_RESIZEH
[task 2019-04-25T10:29:39.823Z] 10:29:39 INFO - /usr/include/gtk-3.0/gdk/gdkwindow.h:226:3: note: 'GDK_DECOR_RESIZEH' declared here
[task 2019-04-25T10:29:39.823Z] 10:29:39 INFO - GDK_DECOR_RESIZEH = 1 << 2,
[task 2019-04-25T10:29:39.823Z] 10:29:39 INFO - ^
[task 2019-04-25T10:29:39.823Z] 10:29:39 INFO - 5 errors generated.
[task 2019-04-25T10:29:39.823Z] 10:29:39 INFO - /builds/worker/workspace/build/src/config/rules.mk:805: recipe for target 'nsWindow.o' failed
[task 2019-04-25T10:29:39.823Z] 10:29:39 ERROR - make[4]: *** [nsWindow.o] Error 1
[task 2019-04-25T10:29:39.823Z] 10:29:39 INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/widget/gtk'
[task 2019-04-25T10:29:39.823Z] 10:29:39 INFO - make[4]: *** Waiting for unfinished jobs....

Flags: needinfo?(stransky)

Updated, Thanks.

Flags: needinfo?(stransky)
Keywords: checkin-needed

Pushed by btara@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/25176eee4a95
[Wayland] Use gdk_window_move_to_rect() to place popup windows, r=ashie

Keywords: checkin-needed

https://hg.mozilla.org/mozilla-central/rev/25176eee4a95

Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox68: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Regressions: 1556218
Regressions: 1558413
Regressions: 1562576
Regressions: 1562662
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: