Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto-centred dialogue is mispositioned on Xinerama supporting desktops #6

Closed
dphlin opened this issue Nov 10, 2017 · 1 comment
Closed

Comments

@dphlin
Copy link

dphlin commented Nov 10, 2017

The left and top offsets of chrome.windows.update() are relative to the display's origin. On a system with multiple screens, the value of screen.left and screen.top must be added to the value specified in chrome.windows.update().

resizeDialog(true) does not add the offset, leading to a window spanning multiple monitors.

diff --git a/extension/dialog.js b/extension/dialog.js
index a7a875b..b736f09 100644
--- a/extension/dialog.js
+++ b/extension/dialog.js
@@ -93,8 +93,8 @@ function resizeDialog(/*boolean*/ moveDialog) {
         chrome.windows.update(chrome.windows.WINDOW_ID_CURRENT, {
             width: WIDTH,
             height: HEIGHT,
-            left: Math.floor((screen.availWidth - WIDTH) / 2),
-            top: Math.floor((screen.availHeight - HEIGHT) / 2),
+            left: screen.left + Math.floor((screen.availWidth - WIDTH) / 2),
+            top: screen.top + Math.floor((screen.availHeight - HEIGHT) / 2),
         }, function() {
             resizeDialog(false);
         });
@Rob--W
Copy link
Owner

Rob--W commented Nov 10, 2017

Thanks for this suggestion! Do you want to open a pull request so you can get proper credit in the commit history for proposing this fix?

Rob--W added a commit that referenced this issue Nov 13, 2017
Credits to @davidf-woaf-net for reporting the issue and proposing the
solution.

Note that `screen.left` and `screen.top` are non-standard properties.
They are undefined on Chrome, and trying to use it would result in
attempting to call `windows.update` with `NaN` as a value. This is
OK, if I ever decide to port the extension to Chrome, then this
serves as a reminder that I need to test the extension on multi-display
setups and adjust the code as necessary.
@Rob--W Rob--W closed this as completed Nov 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants