1279562 - WebExtensions: chrome.windows.update does not accept negative values for 'left' and 'top' positions

Status: RESOLVED FIXED

(WebExtensions :: Untriaged, defect)

Description

[dw-dev]

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:47.0) Gecko/20100101 Firefox/47.0
Build ID: 20160604131506

Steps to reproduce:

I am migrating my 'Tile Tabs' add-on to the new WebExtensions API.

I tried executing the following code:

    chrome.windows.update(windowId, { left: -8, top: -8, width: (1920+8+8), height: (1080+8+8) },
    function(win)
    {
        .....
        .....
    });




Actual results:

chrome.windows.update() failed with the following message in the console:

    "Type error for parameter updateInfo (Error processing left:
     Integer -8 is too small (must be at least 0)) for windows.update."

Further testing showed that chrome.windows.update() always fails if the 'left' or 'top' parameters are negative.



Expected results:

chrome.windows.update() should have positioned the window at the requested position (left: -8, top: -8).

The behaviour in Firefox is incompatible with the behaviour in Chrome.

Chrome allows negative 'left' and 'top' parameters in chrome.windows.update(), although not in chrome.windows.create().

For add-ons that need to precisely position windows to the left or top edges of the screen, it is essential to be able to specify negative values for the 'left' and 'top' parameters.

For example, when using Windows 10, a window can only be positioned to the top-left corner of the screen by setting 'left: -8' and 'top: -1'.

Comment 1

[Kris Maglione [:kmag]]

We just need to change the default minimum value for integers from 0 to -Infinity (or Number.MIN_SAFE_INTEGER):

https://dxr.mozilla.org/mozilla-central/source/toolkit/components/extensions/Schemas.jsm#1272

Comment 2

[Martin Giger [:freaktechnik]]

Attachment: bug1279562-v1.patch

Fix as suggested in the comment above and a test that tries to move a window to negative coordinates.

Comment 3

[Kris Maglione [:kmag]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6047e2393bfd

Comment 4

[Kris Maglione [:kmag]]

Comment on attachment 8762680 [details] [diff] [review]
bug1279562-v1.patch

Review of attachment 8762680 [details] [diff] [review]:
-----------------------------------------------------------------

It looks good, but it also looks like you're going to have to skip this test on Linux. It's failing on try because there are constraints on where the window can be moved.

Comment 5

[Martin Giger [:freaktechnik]]

Attachment: bug1279562-v2.patch

Added check to decide if the test should be executed.

Comment 6

[Kris Maglione [:kmag]]

Comment on attachment 8762915 [details] [diff] [review]
bug1279562-v2.patch

Review of attachment 8762915 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/extensions/test/browser/browser_ext_windows_size.js
@@ +85,5 @@
> +
> +          return browser.windows.update(windowId, geom);
> +        }
> +      }).then(() => {
> +        return checkWindow(geom);

This handler should be attached to the `.update()` call above. Otherwise, looks good.

Comment 7

[Martin Giger [:freaktechnik]]

Attachment: bug1279562-v3.patch

Chained the window size check directly to the update call.

Comment 8

[Martin Giger [:freaktechnik]]

Attachment: bug1279562-v4.patch

Fix commit message to include r=kmag.

Comment 10

[Martin Giger [:freaktechnik]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a7b329a18abf

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0f7a2b698b7a
Accept negative 'left' and 'top' values in chrome.windows.update. r=kmag

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0f7a2b698b7a