230608 - large window.opened window can cover and spoof taskbar if parent is maximized

Status: VERIFIED FIXED

Description

[Jesse Ruderman]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031218
Firebird/0.7+

Steps to reproduce:
1. Maximize your window.
2. javascript:window.open("","","width=,resizable=no")

Result: the new window covers your taskbar

Expected: the new window should not overlap with the taskbar, or at least go
under the taskbar where it overlaps.

This seems to be a z-order problem, not a positioning problem like bug 203621.

Comment 1

[Jesse Ruderman]

I discovered this security hole by pushing the big red button on
http://www.turnofftheinternet.com/.

Comment 2

[Johnny Stenback (:jst)]

Created attachment 141284 [details] [diff] [review]
Always size and position maximized windows.

This may be way off target, I don't know my way around window sizing code all
that well, so bear with me here. This attempts to fix this bug by always sizing
a maximized window to its creators size, and that seems to fix this bug.

Comment 3

[chris hofmann]

won't hold 1.7a for this since I'm guessing that a very high % of mozilla users
have popup blocking enabled, but we defintely would take a good fix if one
appears in time...

Comment 4

[chris hofmann]

cc'ing some more folks for reviews and ideas...

Comment 5

[Dan M]

Created attachment 141354 [details] [diff] [review]
unsizable windows are no longer born maximized

The problem is caused by some weird interaction between a maximized but
unmaximizable and unsizable window. With that combination of happenstance, the
system decides on its own to resize the window as it becomes visible.

The window has already been sized (in the chrome docshell's load handler), so
it's not as if it hasn't been given a size. Johnny's patch sets the window size
once again a little later. It pretty much just repeats the previous size,
though slightly larger. Both size commands happen before the window is made
visible (and resized by the system), so it seems the second attempt just
happens at a time more propitious for convincing the system to leave it alone.

Well if it works, be happy. That said, I think I'd prefer this patch, which
prevents a new window which can't be resized (or maximized) by the user from
being created in a maximized state in the first place.* I claim this will also
fix the bug, and seems more right to me. (I can't test or even compile the
patch as posted without a lot of trouble, but I did test a similar version that
works in my heavily modified tree.)

* Note the new window is still given the size of its maximized opener. I think
this is also a bug, but outside the scope of this bug (see bug 231843).

Comment 6

[Johnny Stenback (:jst)]

Comment on attachment 141354 [details] [diff] [review]
unsizable windows are no longer born maximized

This works, I like it much better than my attempt at this! sr=jst

Comment 7

[Brian Ryner (not reading)]

Comment on attachment 141354 [details] [diff] [review]
unsizable windows are no longer born maximized

- Why did you need to static_cast to nsIXULWindow?

- You might want to just add a short comment to the code about what this is
preventing.

Fix those (or not, if the cast is really necessary, but I'd like to know why)
and r=me.

Comment 8

[Dan M]

It's standard multiple inheritance ambiguous root class stuff. do_GetInterface
wants an nsISupports and it has four to choose from in class nsXULWindow. I
suppose this would be more correct (though identical in generated code):

+ nsCOMPtr<nsIWebBrowserChrome> chrome(do_GetInterface(
+                                      NS_ISUPPORTS_CAST(nsIXULWindow *, this)));

though this file is already full of the NS_STATIC_CAST kind used in this patch.

Comment 9

[Dan M]

Fix checked in to the trunk for Mozilla 1.7b / Firefox 0.9.

Comment 10

[Jesse Ruderman]

Took me a while to find the checkin.. was actually on Feb 21.

Comment 11

[Jesse Ruderman]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7b) Gecko/20040224
Firefox/0.8.0+

Firebird is fixed too :)  I guess that file isn't forked.

Comment 12

[Tracy Walker [:tracy]]

verified with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7b)
Gecko/20040421

Comment 13

[Daniel Veditz [:dveditz]]

Adding Jon Granrose to CC list to help round up QA resources for verification

Comment 14

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 141354 [details] [diff] [review]
unsizable windows are no longer born maximized

a=blizzard

Comment 15

[Christopher Aillon (sabbatical, not receiving bugmail)]

Checked in to the 1.4 branch.