581863 - gtk/nsWindow::GetScreenBounds returns client window size instead of outer (frame) size

Status: RESOLVED FIXED

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

Description

[Karl Tomlinson (:karlt)]

nsIWidget::GetScreenBounds should return the "widget's outside dimensions in global coordinates. This includes any title bar on the window."

Correcting this will make test_screenPersistence.xul at least correctly notice that windows created with outer dimensions have the wrong size.

test_screenPersistance.xul will also fail because it opens a window with (inner, I assume) width/height = 300 and expects outerWidth/outerHeight to be 300.

Comment 1

[Karl Tomlinson (:karlt)]

gdk_window_get_frame_extents is the function to use here.

Comment 2

[Karl Tomlinson (:karlt)]

(In reply to comment #0)
> Correcting this will make test_screenPersistence.xul at least correctly notice
> that windows created with outer dimensions have the wrong size.

Filed Bug 581866.

Comment 3

[Jonathan Watt [:jwatt]]

Closing GTK2 related bugs since we removed GTK2 support at the beginning of 2018 in bug 1278282. Probably best to open a new bug in the unlikely event that any of these are still relevant.

Comment 4

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

The bug assignee didn't login in Bugzilla in the last 7 months.
:stransky, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 581863 - Fix nsWindow coordinates to make sense. r=stransky!

I stumbled upon this bug while realizing that some of the math I'm doing
for PiP windows doesn't work at all on Linux (bug 1934760), because
screenX != mozInnerScreenX, even though outerWidth == innerWidth.

I've tested this on X11 and Wayland (plasma and Gnome) with CSD and SSD,
and in general, this is the same thing Windows does, so should be fine.

The basic idea is:

• Keep mBounds being frame manager bounds (so, outer* and screen*),
  relative to mShell.

• Keep mClientMargin to translate from mBounds to client bounds.

This is both simpler and makes the math consistent.

Comment 6

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 581863 - [remote] Improve window resizing and positioning reliability. r=whimboo

Try a bit harder to set the window size and position.

The change to set_get_window_rect.html is justified as follows:

• The Window Position test already fails on Linux (actually only on
  Mutter, because of what I think is a bug... We request the right
  move to (150, 175), but the compositor moves us to (150, 137). My
  patch doesn't change that.

• However the second test (window size) relies on the position test to
  have moved the window to the right place, as it hard-codes the old
  window position. On the second request, Mutter actually does the
  right thing and moves us there. Instead, make sure not to trigger a
  move in the window size test.

• Use a slightly bigger size on the fractional sizes test, because
  otherwise it hits the minimum window with on wayland (where the outer
  size includes a somewhat large shadow).

• Also include the minimum screen size as well: The set.py change fails
  locally, but not on automation, without that change, but it is
  correct afaict.

• Some tests in set.py fail on automation still, but not locally. These are
  very likely mutter issues / bugs: The browser is not really required to
  honor our maximize / size requests as-is... I can't repro these locally at
  all even with virtually the same screen configuration as automation, so I
  suspect these have been fixed upstream already.

Comment 7

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 581863 - Web ext fixes. r=robwu,#extension-reviewers

• In ext-windows.js, set the position and size on creation. This moves
  the burden of doing so to widget, and could avoid flickering.

• In browser_ext_windows_size.js, we cope with the fact that on linux
  the specified size might be the inner rather than outer size. This
  was already the case before, but outer{Width,Height} reported always
  the inner size so it was papered.

• In browser_unified_extensions_overflowable_toolbar.js and the
  relevant head file, we make the code a bit more reliable. This is all
  to workaround a mutter but on automation, where if a window was
  already maximized and you request a resize, mutter will (correctly)
  restore() the window, but will lose the resize information. This
  doesn't happen on newer Mutter versions. Still, it seems easier to
  use window.maximize(), etc than manually trying to maximize the
  window.

• In head_unified_extensions.js we also remove the code to ensure
  stable positions since it was broken (it was checking
  win.screen.{top,left}, rather than win.screen{X,Y}). We can always
  bring it back fixed if really needed.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/99f2e9d8191d
Fix nsWindow coordinates to make sense. r=stransky,win-reviewers,rkraesig,devtools-reviewers,ochameau
https://hg.mozilla.org/integration/autoland/rev/fc49e83f5b43
[remote] Improve window resizing and positioning reliability. r=whimboo,webdriver-reviewers
https://hg.mozilla.org/integration/autoland/rev/9efb94e53350
Web ext fixes. r=robwu

Comment 9

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/50466 for changes under testing/web-platform/tests

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fd19a8dadb92
Fix screenshot test resize function when starting maximized.

Comment 11

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/99f2e9d8191d
https://hg.mozilla.org/mozilla-central/rev/fc49e83f5b43
https://hg.mozilla.org/mozilla-central/rev/9efb94e53350
https://hg.mozilla.org/mozilla-central/rev/fd19a8dadb92

Comment 12

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 13

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ee04e64da259
fix perma wd failures @set.py.

Comment 15

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/ee04e64da259

Comment 16

[Florin Bilt]

(In reply to Pulsebot from comment #8)

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/99f2e9d8191d
Fix nsWindow coordinates to make sense.
r=stransky,win-reviewers,rkraesig,devtools-reviewers,ochameau
https://hg.mozilla.org/integration/autoland/rev/fc49e83f5b43
[remote] Improve window resizing and positioning reliability.
r=whimboo,webdriver-reviewers
https://hg.mozilla.org/integration/autoland/rev/9efb94e53350
Web ext fixes. r=robwu

Perfherder has detected a devtools performance change from push 9efb94e533503b0cd1309c16d01d488a69941059.

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
2%	damp console.bulklog	linux1804-64-shippable-qr	e10s fission stylo webrender	821.49 -> 805.02

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.

You can run these tests on try with ./mach try perf --alert 43707

For more information on performance sheriffing please see our FAQ.