309251 - Set dom.disable_window_move_resize back to false for 1.5/2.0/trunk

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect)

Description

[Phil Ringnalda (:philor)]

bug 299424 brought back the Advanced JavaScript pref UI, with some pref tweaks,
including disabling resizing windows by default, based on Jesse's thoughts in
bug 299424 comment 7. While I agree completely with what Jesse said:

[[[
(1) Move or resize existing windows

Should be disallowed by default, except in windows opened by JavaScript with
only one tab.  It is frequently used to annoy visitors, and I don't think many
web applications need it.  (Bug 144069, bug 186708, bug 60323, bug 262660.)
]]]

that's not what we've done by setting dom.disable_window_move_resize to true.
With a fix for bug 144069, yes, but right now what we've done is also prevented
every (half) reasonable use of it, too. The most common one I know of is what
the site in the URL field does: open a popup to display a larger image when you
click the little image, with a single site-wide standard function, and then
resize the popup to fit that particular image, rather than having to build the
size of every larger image into the calling page. It's not the cleanest code,
but I suspect it's fairly common (I've got two bugs to dupe to this just from
today), and without a fix for bug 144069 I think we're better off telling people
using hostile sites to uncheck the pref than we are telling people trying to buy
air guns or a thousand other products to check the pref so they can see the
whole picture without resizing the popup.

Comment 1

[Phil Ringnalda (:philor)]

*** Bug 308894 has been marked as a duplicate of this bug. ***

Comment 2

[Phil Ringnalda (:philor)]

*** Bug 309234 has been marked as a duplicate of this bug. ***

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

Phil, to be clear, what you're saying is that fixing bug 144069 is the right way
to ensure that windows with > 1 tab aren't resized or moved, not changing the
default setting of this pref?

Also: is there some way to ignore this pref when dealing with windows that are
opened with size information or toolbars=no? I'm thinking of the same sort of
detection that's used by browser.link.open_newwindow.restriction which is able
to determine if a window.open() is called with or without restrictions.

Comment 4

[Phil Ringnalda (:philor)]

Either the bug 144069 (no resizing multiple tabs) or the bug 186708 (no resizing
if there are toolbars) approach seems safe to me, from a "don't break too many
honest sites" standpoint. Neither one strikes me as all that safe from a code
standpoint, at this point in the branch. If was up to me, I'd switch the pref
back to false by default for 1.5, and try to get one or the other of the better
fixes in for 2.0.

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

I'm not qualified to comment on code safety, so I'll take your word for it. In
terms of the right solution for the user, a fix to either of those bugs would be
my preference, but switching back the pref would be a suitable runner-up.

The site you referred to was designed by template, so yeah, it's probably pretty
common out there. Nominating for 1.5 ...

Comment 6

[Phil Ringnalda (:philor)]

*** Bug 309309 has been marked as a duplicate of this bug. ***

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #5)
> my preference, but switching back the pref would be a suitable runner-up.

Actually, I guess the answer involves switching the pref in either case, doesn't
it? As in, if we do get a fix in such that window.resize events aren't honoured
for anything other than windows opened by requested popups, the pref's gonna go
back to defaulting to "allow"

Comment 8

[Asa Dotzler [:asa]]

I think we'd need people on this who would be involved in fixing the real
problems to tell us what kind of risk is involved. Don't we break Gmail if we
switch this back?

Comment 9

[Phil Ringnalda (:philor)]

I'd love to cc: the people who will fix one or the other of the better ways, but
nobody's actually volunteered over the last three years. You're right, though,
that I don't know enough about it to assess the risk, much less fix it.

But I don't see how changing this pref back to the value it has always had, that
SeaMonkey and 1.7.x and 1.0.x still have, and that 1.5 will have with a migrated
Opera profile that allows moving and resizing, could break Gmail. So far we look
like we may well ship breaking Gmail, by setting dom.disable_window_flip to true
without fixing it so that focusing an iframe doesn't also raise the window
containing it, but other than bug 304746 where bryner initially thought it was
disable_window_move_resize and then duped to the window_flip bug, I don't see
any connection between moving and resizing and Gmail, certainly not any change
that we made after defaulting it to true that would cause breakage if we went
back to false. Am I missing something?

Comment 10

[Mike Connor [:mconnor]]

I'll review a patch if someone attaches it.

Comment 11

[Phil Ringnalda (:philor)]

Attachment: patch

Thanks, Mike. We'll get them next time, better than ever.

Comment 12

[Asa Dotzler [:asa]]

Comment on attachment 197504 [details] [diff] [review]
patch

OK. Thanks for clearing that up.

Comment 13

[:Gavin Sharp [email: [e-mail]]]

1.8 Branch:
mozilla/browser/app/profile/firefox.js; new revision: 1.71.2.12;

Comment 14

[Steffen Wilberg]

So should we check this in the trunk as well until bug 144069 is fixed?

Comment 15

[Mike Beltzner [:beltzner, not reading bugmail]]

Steffen, I would think so, yes. If someone does that, please then also mark this bug as fixed and create a new one that requests switching dom.disabled_window_move_resize to TRUE, which depends on 144069 and nominate it for blocking-firefox2.

Comment 16

[:Gavin Sharp [email: [e-mail]]]

Landed this on the trunk, and filed bug 328492.
mozilla/browser/app/profile/firefox.js; new revision: 1.98;