1493081 - popups incorrectly placed and sized

Status: VERIFIED FIXED

(Core :: Widget: Gtk, defect)

Description

[Cameron McCormack (:heycam)]

On my 2ppx display, I get incorrect placement and sizing of popups (they are placed at 50% of the (x, y) position I expect them to be, and have half the expected width/height).

I confirmed that backing out bug 1490297 resolves the issue for me.  I am not using Wayland.

Comment 1

[Cameron McCormack (:heycam)]

FWIW I only have a single monitor setup.

Comment 2

[Jan Horak [:jhorak]]

Thanks for the report, what distro and its version do you have? Or what gtk version does your system have.

Comment 3

[Cameron McCormack (:heycam)]

I am using Linux Mint 18.3 Sylvia, with libgtk-3-0 package version 3.18.9-1ubuntu3.3.

Comment 4

[Brian Birtles (:birtles)]

I'm not at my computer now so I can't give an GTK version but I was seeing this on Ubuntu 18.04 today with a dual monitor setup.

Comment 5

[Jan Horak [:jhorak]]

Attachment: Bug 1493081 - Don't use GetParentWindowScaleFactor for X11 as it fails on hidpi monitors; r?mattwoodrow

Using GetParentWindowScaleFactor also for the X11 led to wrong placement of the popup menus
on hidpi monitors. This is because it returns always 1 instead of 2 or so on the hidpi
monitors. Changing the implementaion of gtk/nsWindow code could lead to serious regressions,
so we're returning to call GetDesktopToDeviceScale as it was before fix for bug 1432502 landed.

Also for detecting Wayland environment we use GetNativeData(NS_NATIVE_DISPLAY) which returns
nullptr in Wayland.

Comment 6

[Kittipun Khantitrirat]

same thing happens to me too. I'm using gtk3 3.24.1 on Antergos Linux with resolution 3200x1800@200% scaling.

Comment 8

[Daniel Bodea [:danibodea]]

The issue that's set as duplicate of this one has this pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=08592337ced1b16cdea73ea94fdb0af9c8927b31&tochange=5347c7e4811a1a4d80bcee4119cead9192fdff69

May be of some help. Thanks.

Comment 9

[Jan Horak [:jhorak]]

Attachment: Bug 1493081 - Added nsIWidget::GetDesktopToDeviceScaleByScreen for scale factor lookup by window position; r?mattwoodrow

We've added nsIWidget::GetDesktopToDeviceScaleByScreen which will return scale factor of the newly placed window
according to its position on the display. This change is to move implementation to the nsIWidget derived classes.
We need that for GTK Wayland, because on the Wayland we cannot determine absolute position of the window, we
need to use parent's window scale factor. For other platforms the GetDesktopToDeviceScaleByScreen is implemented
in nsBaseWidget.

Comment 10

[Jan Horak [:jhorak]]

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=87d19b453b6fb163385a79ca28808a7e77475220&selectedJob=203969283

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3bc82182725e
Added nsIWidget::GetDesktopToDeviceScaleByScreen for scale factor lookup by window position; r=mattwoodrow

Comment 12

[Narcis Beleuzu [:NarcisB]]

Backed out for android bustages.

Backout link: https://hg.mozilla.org/integration/autoland/rev/e96bcfe8669abdb7eaa9f034daba53d44d8c3e51
Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=204211449&repo=autoland&lineNumber=24396

Comment 13

[Jan Horak [:jhorak]]

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=885d817bc8d02c32b0ea9f9536626850081bd672
Hopefully that should fix all the stuff.

Comment 14

[Daniel Bodea [:danibodea]]

:heycam,

Can you check this binary (try build) to see if your issues were fixed?

Comment 15

[Cameron McCormack (:heycam)]

With that try build I don't see any incorrect placement of popups, thanks!

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9a3a43e8743d
Added nsIWidget::GetDesktopToDeviceScaleByScreen for scale factor lookup by window position; r=mattwoodrow

Comment 17

[Natalia Csoregi [:nataliaCs]]

Backed out for bustages on AccessibleWrap.cpp

backout: https://hg.mozilla.org/integration/autoland/rev/5d7093d30ed3cb97eb09034826bfb64f4f080e0a

push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=9a3a43e8743da65561b9ba4aa1f47777b6a61528&searchStr=android

failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=206924815&repo=autoland&lineNumber=25244

[task 2018-10-22T08:55:57.112Z] 08:55:57     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/accessible/base'
[task 2018-10-22T08:55:58.707Z] 08:55:58     INFO -  make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/accessible/android'
[task 2018-10-22T08:55:58.707Z] 08:55:58     INFO -  /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/clang/bin/clang++ --target=i686-linux-android -o AccessibleWrap.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 -DNDEBUG=1 -DTRIMMED=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/accessible/android -I/builds/worker/workspace/build/src/obj-firefox/accessible/android -I/builds/worker/workspace/build/src/accessible/base -I/builds/worker/workspace/build/src/accessible/generic -I/builds/worker/workspace/build/src/accessible/html -I/builds/worker/workspace/build/src/accessible/ipc -I/builds/worker/workspace/build/src/accessible/ipc/other -I/builds/worker/workspace/build/src/accessible/xul -I/builds/worker/workspace/build/src/dom/base -I/builds/worker/workspace/build/src/widget -I/builds/worker/workspace/build/src/widget/android -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/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 -isystem /builds/worker/workspace/build/src/android-ndk/sysroot/usr/include/i686-linux-android -isystem /builds/worker/workspace/build/src/android-ndk/sysroot/usr/include -gcc-toolchain /builds/worker/workspace/build/src/android-ndk/toolchains/x86-4.9/prebuilt/linux-x86_64 -D__ANDROID_API__=16 -Qunused-arguments -Wall -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 -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -Wno-return-type-c-linkage -fno-sized-deallocation -fno-short-enums -fno-exceptions -I/builds/worker/workspace/build/src/android-ndk/sources/cxx-stl/llvm-libc++/include -I/builds/worker/workspace/build/src/android-ndk/sources/android/support/include -I/builds/worker/workspace/build/src/android-ndk/sources/cxx-stl/llvm-libc++abi/include -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pipe -g -Oz -fno-omit-frame-pointer -funwind-tables -Werror -Wno-error=shadow  -MD -MP -MF .deps/AccessibleWrap.o.pp   /builds/worker/workspace/build/src/accessible/android/AccessibleWrap.cpp
[task 2018-10-22T08:55:58.708Z] 08:55:58     INFO -  /builds/worker/workspace/build/src/accessible/android/AccessibleWrap.cpp:334:65: error: expected unqualified-id
[task 2018-10-22T08:55:58.708Z] 08:55:58     INFO -            collectionInfo, "isHierarchical", java::sdk::Boolean::TRUE());
[task 2018-10-22T08:55:58.708Z] 08:55:58     INFO -                                                                  ^
[task 2018-10-22T08:55:58.709Z] 08:55:58     INFO -  /builds/worker/workspace/build/src/obj-firefox/dist/include/unicode/umachine.h:240:18: note: expanded from macro 'TRUE'
[task 2018-10-22T08:55:58.709Z] 08:55:58     INFO -  #   define TRUE  1
[task 2018-10-22T08:55:58.709Z] 08:55:58     INFO -                   ^
[task 2018-10-22T08:55:58.710Z] 08:55:58     INFO -  1 error generated.
[task 2018-10-22T08:55:58.710Z] 08:55:58     INFO -  /builds/worker/workspace/build/src/config/rules.mk:1126: recipe for target 'AccessibleWrap.o' failed
[task 2018-10-22T08:55:58.710Z] 08:55:58     INFO -  make[4]: *** [AccessibleWrap.o] Error 1
[task 2018-10-22T08:55:58.710Z] 08:55:58     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/accessible/android'
[task 2018-10-22T08:55:58.711Z] 08:55:58     INFO -  make[4]: *** Waiting for unfinished jobs....

Comment 18

[Jan Horak [:jhorak]]

Hm, there are changes in moz.build solving the android build issue: https://phabricator.services.mozilla.com/D7290 These wasn't pushed to inbound, how come? How to proceed now?

Comment 19

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

The changes in the raw diff provided by phabricator at https://phabricator.services.mozilla.com/D7290 and what got pushed as https://hg.mozilla.org/integration/autoland/rev/9a3a43e8743da65561b9ba4aa1f47777b6a61528 match.

Comment 20

[Jan Horak [:jhorak]]

I've rewritten the patch to not touch the header files which caused the problem on Android. Hope that fix all the issues, see latest try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e4e8f6797f184a58f27512854975cee8a93fcbdf

Comment 21

[Jan Horak [:jhorak]]

Hope that will finally land successfully. Sorry for the trouble.

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/33523dc590fe
Added nsIWidget::GetDesktopToDeviceScaleByScreen for scale factor lookup by window position; r=mattwoodrow

Comment 23

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/33523dc590fe

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Hey Cam, can you confirm that this is fixed for you now?

Comment 25

[Cameron McCormack (:heycam)]

Yes, popup positions seem fine lately.

Comment 26

[Daniel Bodea [:danibodea]]

Based on the comment above, I will mark this bug as verified.

Thank you!

Comment 27

[Ryan VanderMeulen [:RyanVM]]

Please request Beta approval on this when you get a chance.

Comment 28

[Jan Horak [:jhorak]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #27)
> Please request Beta approval on this when you get a chance.

I don't think that's necessary. The regression has been resolved by backout of the bug 1490297. This bug actually solves the bug 1490297 by different approach.

Comment 29

[Ryan VanderMeulen [:RyanVM]]

Ah, I see. Thanks for the info.

Comment 30

[Scott Palmer]

Perhaps this bug is back or I have found another issue.

When clicking and holding on the back button, the list of previous sites is not opened in the correct location. (it open much further down)

Additionally, the right click popup menu while inside the webpage content area also opens in the incorrect location.

Running:

• Ubuntu 19.04 (gnome-shell 3.32.1, mutter 3.32.1)
• Firefox 67.0.1

Dual monitor: yes

Scaling: both monitors set to 100%

I have a screenshot if that helps, but can't see where to attach one.

Comment 31

[Scott Palmer]

Should this be re-opened as a regression?

Comment 32

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

Scott, please file a new bug for it.