1. Load the testcase. 2. Click button #1: "Open an empty window" 3. Click button #5: "w.sizeToContent();" Result: window is extremely tiny. (Possible security implications: full-screen button covers the close button on Mac; window could "hide"; maybe spoofing or clickjacking.) Expected: window becomes 100x100 or so, like button #3 does. Strangely, it is clamped correctly at the upper end (you can see this with buttons #2, #5, and #4.)

This is moot if you haven't checked "Allow scripts to: Move or resize popup windows".

Is that MocOS only?

Comment on attachment 636241 [details] [diff] [review] Patch v1 r=me if you fix the comment in the IDL to reflect reality.

Mounir, anything preventing landing the patch (with the comment fixed)?

IIRC, I think the patch was failing some tests. I will send it again to try and see.

This is breaking layout/base/tests/test_bug458898.html Locally, if I open a page with a 100/200 div, sizeToContent() produce a size that has scrollbars while, before, it wouldn't. See: https://tbpl.mozilla.org/?tree=Try&rev=d878ab5527ea I will try to have a quick look at it later but I unfortunately doesn't have much cycles to dedicate :(

Olli, feel free to take this review from Boris ;) Locally, it is fixing the test breakage.

Hmm. So why is the new code better than the old?

(In reply to Boris Zbarsky (:bz) from comment #9) > Hmm. So why is the new code better than the old? My original patch was changing the behaviour of the method. nsIMarkupDocumentViewer.sizeToContent() was using nsIDocShellTreeOwner.sizeShellTo() to set the size and my patch was using nsIBaseWindow.setSize() because it was used in most sizing methods in nsGlobalWindow. I was assuming those two methods would likely do the same thing but obviously they are not. Actually, reading more the code, I realise SetInner{Width,Height} are using nsGlobalWindow::SetDocShellWidthAndHeight(), which I could use. This is actually the behiaiour sizeToContent() should have: the inner{Width,Height} should be changed, not the outer. I would assume nsIBaseWindow.setSize() does change the outher size.

Comment on attachment 682458 [details] [diff] [review] Fix test breakage Ah, yes. The basewindow thing off the treeowner would set the outer size, indeed. r=me if you use SetDocShellWidthAndHeight.

(this is a test + a reminder to fix the Windows errors)

(this is a test + a reminder to fix the Windows errors)

https://hg.mozilla.org/integration/mozilla-inbound/rev/204665ad4a24 (and now, my bugzilla dashboard can show me the pending "needinfo" ;))

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/ce7456dc02bd because the test timed out on Android.

Relanded with the test disabled on Android: https://hg.mozilla.org/integration/mozilla-inbound/rev/caab3921e4c2 there is no reason to have the test enabled on Android given that Firefox Android opens a tab everytime a popup is opened.

(In reply to Phil Ringnalda (:philor) from comment #15) > Backed out in > https://hg.mozilla.org/integration/mozilla-inbound/rev/ce7456dc02bd because > the test timed out on Android. Merge of backout: https://hg.mozilla.org/mozilla-central/rev/ce7456dc02bd

Documentation updated: https://developer.mozilla.org/en-US/docs/Web/API/Window.sizeToContent https://developer.mozilla.org/en-US/Firefox/Releases/20