XUL persist overrides sizes set up to and including onload

RESOLVED INCOMPLETE

Status

()

defect
RESOLVED INCOMPLETE
14 years ago
7 years ago

People

(Reporter: ushankar+bugzilla, Unassigned)

Tracking

Trunk
x86
Linux
Points:
---
Bug Flags:
blocking1.9 -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [testday-20120713])

Attachments

(1 attachment)

Reporter

Description

14 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.10) Gecko/20050909 Fedora/1.7.10-1.3.2
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4

I have an extension that has both a separate XUL window and browser overlays.
From the XUL window I also open a browser window, but using its Chrome URL,
i.e., window.open("chrome://browser/content/browser.xul", ...). 

I have a hard time setting the width and height of this created window, which I
gather is because of permission restrictions on browser overlays (yes?). The
following do not work:
- Width and height features in window.open (called from the XUL window's JS)
- Setting the width and height immediately after window.open
- Setting the width and height in the load handler of the browser overlay

The one thing that does work is if I use the observer service to send a
notification from the XUL window to the overlay, which, in its observe()
function, resizes the window. 

Perhaps this has something to do with the XUL thread having privileges and
tranferring control into the overlay via the observer service.

1) Why do setting the width and height fail from privileged chrome:// URLs at
all? Clearly this would be best.
2) Why does it work in the notification case? Perhaps there is a bug related to
that, especially if it can lead to privilege escalation by unprivileged
observers. I don't know enought to assess this threat; marking security
sensitive just in case.

Reproducible: Always

Steps to Reproduce:
1. Call window.open("chrome://browser/content/browser.xul") from chrome:// JS
attached to a separate XUL window.
2. Try to set width/height using features or innerWidth/innerHeight.
3.

Actual Results:  
No change in size

Expected Results:  
Change in window size
Do you have a testcase? Is your "xul window" opening the additional window from
the chrome itself, or from a non-chrome content area?

Clearing security flag, this sounds over-restrictive, not an exploit.
Group: security
Component: Security → Security: CAPS
Whiteboard: [sg:needinfo]

Comment 2

14 years ago
What is the URI of your XUL window which is trying to open the browser window?
Whiteboard: [sg:needinfo] → needs testcase
Reporter

Comment 3

14 years ago
Here is an extension demonstrating the problem. After some experimentation it
seems to be a question of timing: attempts to set the width/height of the new
window work once the window is fully loaded (visually), but not before
(including in its onload handler). 

This extension has GUID {8b81cfe6-f985-475b-8461-2819a62e6a09} and should be
installed by untarring it and making a pointer file to that location.

To try it, select it from the Extensions dialog and hit Preferences. This
installs a browser overlay and opens a XUL window, which then does a
window.open on a new browser window.

Several methods of setting width/height are tried:
In the overlay XUL, width and height attributes are set on the window (fails)
In window.open, width and height features are specified (fails)
After window.open, the innerWidth(Height) properties are set (fails)
After window.open, a load handler is installed from the opener, which attempts
to set the width/height (fails)
In the overlay, in the load handler, set the width/height (fails)
In the overlay, try to add a load handler to the inner content window and do it
there (fails: handler never called)
setTimeout is used to wait 5 seconds, then set the height of the window (works,
from both the opener and the overlay)
 
Most of these methods are commented out for simplicity; simply uncomment the
method you want to try.

Comment 4

14 years ago
The only part of this that seriously concerns me is the window.open()
parameters, which should work. The rest of them are not guaranteed.
Reporter

Comment 5

14 years ago
How do we know what is guaranteed or not? I thought that once the load handler
was fired, the window was in a usable state. Is this not the case? Is there
another event we should be listening for instead? 

I am a bit concerned about the silent failure of these operations. If I try to
set the width (e.g.) and it doesn't work, I would normally expect an exception
or an error message in the log. Perhaps there is another mechanism I am not
aware of...
So is this extension installed as chrome:// ?  Or somewhere else?  What are detailed steps to reproduce (starting with a clean Firefox install with a brand new profile, and assuming no knowledge on my part of Firefox extension install anything -- this is a good approximation of reality)?
Keywords: qawanted
Flags: blocking1.9a2?
Reporter

Comment 7

13 years ago
(In reply to comment #6)
Yes, it's installed as chrome://. I haven't tried to package this up as an XPI. To install, you should untar the testcase into {ffprofile}/extensions/{8b81cfe6-f985-475b-8461-2819a62e6a09} . Then firefox should detect and install the extension when restarted.
With a current trunk build the extension in question is disabled because it's not compatible, according to the extension manager.
OK, I think I got past that with install.rdf messing...  Would have been nice to mention that part.
OK.  So I click "preferences".  A window opens.  Now what?  What are the expected results?  How do I tell whether I'm seeing this bug?
OK.  So the first thing I see is that passing the width/height to window.open _sorta_ works.  It works unless the XUL document being opened persists its width and height.  The 

780         newTreeOwner->SetPersistence(PR_FALSE, PR_FALSE, PR_FALSE);

call in nsWindowWatcher fails because at the point when we make that call there is no <window> element to mess with yet.  This is because for chrome windows we return to the window watcher before the load has happened.

So in this case, the order is:

1)  Open window
2)  Set size (based on width/height passed in).
3)  Persistence resets size based on persisted attributes of <window>

I believe that's actually the designed behavior...

I suspect most of the other cases fall into this too, since step 3 happens fairly late off the onload event; probably after all script onload handlers have fired.

In any case, this is so not a security issue.  Everything would have worked fine if you were not opening a window which persists its size.
Assignee: dveditz → nobody
Component: Security: CAPS → XP Toolkit/Widgets: XUL
QA Contact: toolkit → xptoolkit.xul
Summary: Chrome privilege applied inconsistently between XUL and browser overlay → XUL persist overrides sizes set up to and including onload
Reporter

Comment 12

13 years ago
I see...thanks for clearing this up.

So, for the case that I'm doing, i.e. opening browser.xul, I guess it's not possible to set the size except by waiting some amount of time until the window is expected to have loaded and to do it then?

Is there an equivalent of onLoad in which setting the size can work? I'd like not to put arbitrary time constants into my code.
Hmm...  I'm not sure whether we should change the ordering of onload and persist sizing.  mconnor, Neil, what do you think?

Note that I suspect that a 0ms timeout from onload will do the right thing for you.
I remember we had a similar issue with onload and document.title - as I recall the hack (which I guess would work here too, because persistence fires before onload) was that in onload you used to have to set the document element's title attribute in the onload handler.

As I recall unconstrained windows expect to be sized to content after the load handler has fired and not before, so if we changed this we would have to audit all the windows and add sizeToContent() to any that might change their content.
> because persistence fires before onload

It does?  That's not what I was seeing....
The showing of the XUL window happens after onload; persistence is just one way of getting height and width attributes into the DOM.
Oh, I'm sorry.  What I meant is that the sizing based on the window's height/width attrs happens after (or from) onload.

You're right that persistence happens before.

Perhaps we should make disabling persistence work even for cases when there is no root element yet (taking effect when the root element is finally there)?
Well, calling it persistence is a bit of a misnomer; what it should disable is intrinsic sizing and positioning. And z-ordering needs to be added too.
Actually, it does disable persistence.  Check the implementation...  The idea is to not only change what the window size is but also to not persist this script-set size to localstore.
Flags: blocking1.9a2? → blocking1.9-

Updated

11 years ago
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.xul → xptoolkit.widgets

Comment 20

7 years ago
Reporter, is problem still reproducible with current Firefox builds?
Whiteboard: needs testcase → [testday-20120713], [closeme 2012-08-01]
At this point I think we can just close this one. Please reopen if this is reproducible in the latest Firefox Nightly.
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
Keywords: qawanted
Resolution: --- → INCOMPLETE
Whiteboard: [testday-20120713], [closeme 2012-08-01] → [testday-20120713]
You need to log in before you can comment on or make changes to this bug.