Closed
Bug 1253133
Opened 8 years ago
Closed 8 years ago
Support changing size and position via browser.windows.update
Categories
(WebExtensions :: Untriaged, defect)
WebExtensions
Untriaged
Tracking
(firefox48 fixed)
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: kmag, Assigned: kmag)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
(Whiteboard: [windows][berlin][good first bug])
Attachments
(1 file)
No description provided.
Assignee | ||
Updated•8 years ago
|
Whiteboard: [windows][berlin] → [windows][berlin][good first bug]
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → kmaglione+bmo
Assignee | ||
Comment 1•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/40823/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/40823/
Attachment #8731772 -
Flags: review?(aswan)
Updated•8 years ago
|
Attachment #8731772 -
Flags: review?(aswan) → review+
Comment 2•8 years ago
|
||
Comment on attachment 8731772 [details] MozReview Request: Bug 1253133: [webext] Support changing window geometry via windows.update. r?aswan https://reviewboard.mozilla.org/r/40823/#review37533 Comments are all minor, looks good to me. ::: browser/components/extensions/ext-utils.js:670 (Diff revision 1) > > return "normal"; > }, > > + updateGeometry(window, options) { > + if (options.left !== null || options.top !== null) { I know this was cut&pasted but I thought our standard was to prefer == and != to === and !== ? ::: browser/components/extensions/ext-windows.js:170 (Diff revision 1) > return WindowManager.convert(extension, window); > }); > }, > > update: function(windowId, updateInfo) { > - // TODO: When we support size/position updates: > + if (updateInfo.state !== null && updateInfo.state != "normal") { The chrome docs actually say that geometry can't be supplied with sate "minimized", "maximized" or "fullscreen" but that seems to suggest that geometry could be supplied along with state "docked"? ::: browser/components/extensions/test/browser/browser_ext_windows_size.js:31 (Diff revision 1) > + browser.test.assertEq(expected[key], geom[key], `Expected '${key}' value`); > + } > + > + return browser.windows.get(windowId); > + }).then(geom => { > + browser.test.log("Check API-reported window size"); This confused me for a moment, I think that putting this log right next to the call to browser.windows.get() might be clearer to a reader? (Likewise the "Check actual window size" message above)
Assignee | ||
Comment 3•8 years ago
|
||
https://reviewboard.mozilla.org/r/40823/#review37533 > I know this was cut&pasted but I thought our standard was to prefer == and != to === and !== ? Not in WebExtension code, and not in cases where there's a reason to use `===`. In this case, omitted values should be normalized to `null`, so `undefined` values signal an error, and shouldn't be treated the same as `null`. > The chrome docs actually say that geometry can't be supplied with sate "minimized", "maximized" or "fullscreen" but that seems to suggest that geometry could be supplied along with state "docked"? I think it's just badly documented. I can't see how geometry would make sense in combination with the "docked" state. In any case, we currently treat "docked" as an alias for "minimized", so it doesn't make sense to treat them differently here. > This confused me for a moment, I think that putting this log right next to the call to browser.windows.get() might be clearer to a reader? (Likewise the "Check actual window size" message above) I guess. I added those logs so it would be easier to make sense of the test output just after them, but I guess it couldn't hurt to move them before we do the queries.
Assignee | ||
Comment 4•8 years ago
|
||
The tests for this are currently disabled on OS-X because, for some reason, we're winding up in fullscreen mode for some test runs, which prevents us from resizing even new windows.
Assignee | ||
Comment 5•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/3ad9e7d55c6e393323c4afb57d4ce32e8d8a89cc Bug 1253133: [webext] Support changing window geometry via windows.update. r=aswan
Comment 6•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3ad9e7d55c6e
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Updated•6 years ago
|
Product: Toolkit → WebExtensions
You need to log in
before you can comment on or make changes to this bug.
Description
•