Last Comment Bug 635673 - Content inside xul:iframe with type=content is always able to resize top chrome window
: Content inside xul:iframe with type=content is always able to resize top chro...
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: XUL (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla7
Assigned To: Jan Varga [:janv]
:
:
Mentors:
Depends on:
Blocks: 619991 726530
  Show dependency treegraph
 
Reported: 2011-02-21 02:12 PST by Alexandre Poirot [:ochameau]
Modified: 2012-02-13 15:32 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (4.24 KB, patch)
2011-02-28 14:17 PST, Jan Varga [:janv]
bzbarsky: review-
Details | Diff | Splinter Review
updated patch (3.05 KB, patch)
2011-06-01 02:59 PDT, Jan Varga [:janv]
bzbarsky: review+
Details | Diff | Splinter Review

Description Alexandre Poirot [:ochameau] 2011-02-21 02:12:18 PST
There is a common issue when you are writing firefox extension and we faced it in Jetpack too.
There is multiple usecases where you want to display a remote content that you doesn't necessary trust. For example: sidebars, panels, widgets, ... 
So you use XUL Iframe with type attribute set to "content" in order to have a secured iframe, like the browser do with websites. 
But as websites are able to move and resize browser windows, your extension components (sidebar, widgets,...) is going to resize the browser window too.
And we really don't want that a sidebar resize the browser window!

We would really apprieciate having a new attribute on docShell, like allowWindowControl that would allow to block all move and resize commands coming from iframe document!
https://developer.mozilla.org/en/nsIDocShell

Here is a piece of code that highlight this behavior:
var win = window.top.opener;
var doc = win.document;
var iframe = doc.createElement("iframe");
iframe.setAttribute("type","content");
iframe.setAttribute("src","data:text/html,<script>window.resizeTo(200,200);</script>");
doc.documentElement.appendChild(iframe);

You may simply paste this code in a freshly opened JS console (not an automatically opened one with -jsconsole argument)
Comment 1 Myk Melez [:myk] [@mykmelez] 2011-02-24 14:06:32 PST
Boris: is this feasible (and is this the right Bugzilla component for this bug)?
Comment 2 Boris Zbarsky [:bz] (still a bit busy) 2011-02-25 11:43:15 PST
This is definitely feasible.  The simplest approach is probably to change nsGlobalWindow::CanMoveResizeWindows to not be static and have it check a property on the docshell.  The IsFrame() check that always goes hand in hand with it could go into that method too, or even into the docshell thing (so force subframes to not allow this on the docshell level).

As for component... there are several pieces here.  One is adding the docshell API; the other is exposing it to XUL authors.
Comment 3 Myk Melez [:myk] [@mykmelez] 2011-02-25 12:24:45 PST
Hmm, I suspect the docshell API by itself would be sufficient for Jetpack's needs, even without extra exposure through XUL (f.e. via a "bad-content-no-donut" iframe type), since the Add-on SDK has access to the docshells of the iframes it creates.

*Randomly guessing that jst is the guy to suggest someone to take that on.*

jst: can you recommend someone to do the work bz described in comment 2?
Comment 4 Boris Zbarsky [:bz] (still a bit busy) 2011-02-25 12:36:03 PST
Fwiw, if we don't have someone else who obviously needs a small project, I could just do that.  Lemme know.
Comment 5 Jan Varga [:janv] 2011-02-26 00:02:01 PST
Boris, I'll take this bug, if you don't mind.
Comment 6 Boris Zbarsky [:bz] (still a bit busy) 2011-02-26 07:56:30 PST
Go for it!
Comment 7 Jan Varga [:janv] 2011-02-28 14:17:37 PST
Created attachment 515735 [details] [diff] [review]
patch
Comment 8 Jan Varga [:janv] 2011-02-28 14:23:22 PST
Here is modified JS for testing:
var win = window.top.opener;
var doc = win.document;
var iframe = doc.createElement("iframe");
iframe.setAttribute("type","content");
iframe.setAttribute("src","data:text/html,<button onclick='window.resizeTo(200,250)'>Resize me</button>");
doc.documentElement.appendChild(iframe);
iframe.docShell.allowWindowControl = false;

I didn't add a check for iframes (subframes) for now.
Comment 9 Myk Melez [:myk] [@mykmelez] 2011-03-31 12:19:05 PDT
Jan: is your patch ready for review?
Comment 10 Jan Varga [:janv] 2011-03-31 13:54:32 PDT
myk: yes, I think so
Comment 11 Boris Zbarsky [:bz] (still a bit busy) 2011-04-04 23:07:31 PDT
Comment on attachment 515735 [details] [diff] [review]
patch

>+  nsIDocShell* docShell = nsContentUtils::GetDocShellFromCaller();

Why is that the right docshell to use, as opposed to mDocShell?
Comment 12 neil@parkwaycc.co.uk 2011-04-05 13:29:52 PDT
Surely it would be easiest to drop an mPrimary check into nsContentTreeOwner::SetSize and any related methods? This would also stop background tabs from inadvertently changing the window size.
Comment 13 Jan Varga [:janv] 2011-06-01 02:59:57 PDT
Created attachment 536554 [details] [diff] [review]
updated patch
Comment 14 Myk Melez [:myk] [@mykmelez] 2011-06-23 00:30:18 PDT
Comment on attachment 536554 [details] [diff] [review]
updated patch

Erm, it looks like there's a patch sitting around waiting for review again.
Comment 15 Boris Zbarsky [:bz] (still a bit busy) 2011-06-23 15:26:01 PDT
Comment on attachment 536554 [details] [diff] [review]
updated patch

r=me
Comment 16 Jan Varga [:janv] 2011-06-23 21:30:40 PDT
I will land it and thanks for the review
Comment 18 Alexandre Poirot [:ochameau] 2011-06-30 10:35:04 PDT
I'm now using this feature in jetpack and it works like a charm,
Thanks Jan!
Comment 19 Eric Shepherd [:sheppy] 2011-08-05 13:03:20 PDT
Added the new attribute to the reference documentation:

https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsIDocShell#Attributes

And mentioned on Firefox 7 for developers.

Note You need to log in before you can comment on or make changes to this bug.