Closed
Bug 565541
Opened 14 years ago
Closed 13 years ago
Web sites shouldn't be allowed to resize main window
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
VERIFIED
FIXED
mozilla7
Tracking | Status | |
---|---|---|
firefox7 | + | --- |
People
(Reporter: jhill, Assigned: mounir)
References
(Blocks 2 open bugs, )
Details
(Keywords: dev-doc-complete, relnote, testcase, Whiteboard: potential website fallout from new feature (for docs, see comment 24))
Attachments
(3 files, 4 obsolete files)
644 bytes,
text/html
|
Details | |
4.78 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
14.69 KB,
patch
|
bzbarsky
:
review+
jruderman
:
feedback+
|
Details | Diff | Splinter Review |
(Note: this is filed as part of the “Paper Cut” bugs — we assume that there may be multiple existing bugs on this. Please make them block this bug, and we will de-dupe if they are indeed exactly the same. Thanks!)
To reproduce:
1. go to a website that re-sizes the main window
2. be frustrated
Recommendation:
Change defaults that allow re-sizing of main window to disallow by default. Pop-up windows should be allowed to be re-sized.
Updated•14 years ago
|
Blocks: cuts-control
Comment 1•14 years ago
|
||
So basicly we need to split "dom.disable_window_move_resize" for main window and popups and set the main window one to false?
Comment 2•14 years ago
|
||
This needs the same solution as bug 369306: allow window A to move/resize window B only if A created B.
Updated•14 years ago
|
Target Milestone: --- → Firefox 4.0b8
Comment 3•14 years ago
|
||
Added simple testcase as URL above.
Comment 4•14 years ago
|
||
Solutions have been proposed in bug 186708 and bug 502561. But James might be right, again :)
Updated•14 years ago
|
Target Milestone: Firefox 4.0b8 → ---
Assignee | ||
Comment 5•14 years ago
|
||
Comment 2 seems to make a lot of sense.
I've implemented that and it seems to fit Jesse's proposal in bug 502561.
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
Attachment #519509 -
Flags: feedback?(jruderman)
Assignee | ||
Updated•14 years ago
|
Whiteboard: [needs feedback]
Assignee | ||
Comment 6•14 years ago
|
||
Assignee | ||
Comment 7•14 years ago
|
||
Basically, the patch makes impossible to resize a window if the caller didn't create it. I would say that most situations where you want to resize a window you didn't create is malicious. Only one seem valid, like Jesse said in bug 502561: when a popup tries to resize itself. Though, users are now able to resize the popups themself and the popup can always be resized by the main window so I would say it worths this small issue.
Assignee | ||
Comment 8•14 years ago
|
||
For those interested, you might find builds here (I just sent the patch to the try server so it might take some time):
https://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/mlamouri@mozilla.com-6a2bd5a96f2d/
Comment 9•14 years ago
|
||
(In reply to comment #5)
> Created attachment 519509 [details] [diff] [review]
> Simply check that the caller is the opener
If a popup gets redirected to a new tab instead of a new window, does this still prevent the resize call from the opener? (It should, because the opener and openee are both in the same "main" window.)
Assignee | ||
Comment 10•14 years ago
|
||
(In reply to comment #9)
> (In reply to comment #5)
> > Created attachment 519509 [details] [diff] [review]
> > Simply check that the caller is the opener
>
> If a popup gets redirected to a new tab instead of a new window, does this
> still prevent the resize call from the opener? (It should, because the opener
> and openee are both in the same "main" window.)
No, here the term "window" isn't equivalent to the one used by your window manager. Each tab has a different window so if you open a popup in another tab instead of another window, the caller and the opener wouldn't be the same.
Assignee | ||
Comment 11•14 years ago
|
||
This patch is adding a check to be sure that chrome context can still do whatever it wants.
In addition. I've added some tests for chrome and non-chrome.
Attachment #519509 -
Attachment is obsolete: true
Attachment #519630 -
Flags: review?(jst)
Attachment #519630 -
Flags: feedback?(jruderman)
Attachment #519509 -
Flags: feedback?(jruderman)
Assignee | ||
Updated•14 years ago
|
Whiteboard: [needs feedback] → [needs review][needs feedback]
Assignee | ||
Comment 12•14 years ago
|
||
(In reply to comment #9)
> (In reply to comment #5)
> > Created attachment 519509 [details] [diff] [review]
> > Simply check that the caller is the opener
>
> If a popup gets redirected to a new tab instead of a new window, does this
> still prevent the resize call from the opener? (It should, because the opener
> and openee are both in the same "main" window.)
Hmm, my previous comment (comment 10) is partially true: the caller isn't the opener because there are both on the same window but it's wrong because the caller and the opener can be the same tab. I wasn't very awake I guess...
AFAIK, this is only possible when the user sets this preference to 0:
browser.link.open_newwindow.restriction
If that's really the case, I don't think we should care about that now.
Though, I think we should always disable moving and resizing windows if browser.link.open_newwindow.restriction is set to 0 because all windows will be user created ones. I wonder what does this preference when set to 1...
Assignee | ||
Comment 13•14 years ago
|
||
Doc about browser.link.open_newwindow.restriction:
http://kb.mozillazine.org/Browser.link.open_newwindow.restriction
I assume we can't have access to a tab created with a link (and target=_blank), so with the default behavior, we can only resize the main window with window.open('about:blank', '', ''); and manipulating the created window...
I think this can be fixed easily. I'm going to update the patch.
Assignee | ||
Updated•14 years ago
|
Attachment #519630 -
Attachment description: Patch v1 → Part 1 - Limit move/resize actions to the opener of the window.
Assignee | ||
Comment 14•14 years ago
|
||
Harder than I thought but it's finally done :)
Attachment #519751 -
Flags: review?(jst)
Updated•14 years ago
|
Attachment #519630 -
Flags: review?(jst) → review+
Comment 15•14 years ago
|
||
Comment on attachment 519751 [details] [diff] [review]
Part 2 - Do not allow move/resize action if the window and the caller are in the same chrome window
- In docshell/base/nsIDocShellTreeItem.idl:
/*
Returns the root DocShellTreeItem of the same type. This is a convience
equivalent to getting the parent of the same type and its parent until
there isn't a parent.
*/
readonly attribute nsIDocShellTreeItem sameTypeRootTreeItem;
+ /*
+ Returns the parent if there is one and has the given type.
+ Returns null otherwise.
Make the indentation line up here, you'll need to use tabs, yuck!
- In nsGlobalWindow::CanMoveResizeWindows():
+ /**
+ * In some cases, the popup can be created as a new tab. In that case, we
+ * have to make sure that both windows aren't in the some chrome window.
+ * If they are, we do not fulfil the request.
+ */
+ nsCOMPtr<nsIDocShellTreeItem> treeItem = do_QueryInterface(mDocShell);
+ treeItem->GetRootTreeItemWithType(nsIDocShellTreeItem::typeChrome,
+ getter_AddRefs(treeItem));
I don't see any guarantees that mDocShell is non-null here, you should null check treeItem here before dereferencing.
+ nsCOMPtr<nsIDocShellTreeItem> callerTreeItem =
+ do_QueryInterface(caller->GetDocShell());
+ callerTreeItem->GetRootTreeItemWithType(nsIDocShellTreeItem::typeChrome,
+ getter_AddRefs(callerTreeItem));
Same here, I think the opener could be a closed window here, which means it has no docshell. Null check callerTreeItem.
- In embedding/browser/webBrowser/nsWebBrowser.cpp:
NS_IMETHODIMP nsWebBrowser::GetParent(nsIDocShellTreeItem** aParent)
{
*aParent = nsnull;
return NS_OK;
}
+NS_IMETHODIMP nsWebBrowser::GetParentWithType(PRInt32 aType, nsIDocShellTreeItem** aParent)
+{
+ nsresult rv;
This violates the use-the-indentation-style-of-the-surrounding-code rule, which would mean 3 space indentation. Geez, I'm tempted to say feel free to violate that here :)
r=jst with that. But I think bz should look over the docshell API change (addition) here as well.
Attachment #519751 -
Flags: review?(jst)
Attachment #519751 -
Flags: review?(bzbarsky)
Attachment #519751 -
Flags: review+
Comment 16•14 years ago
|
||
So for part 1, should we consider checking that opener and caller are same-origin instead of identical? Otherwise in a multi-frame page opening a window from one frame and resizing it from another frame would not work...
Comment 17•14 years ago
|
||
For part 2, fix "chcheck" in the comments?
The new code will totally fail under e10s, completely and utterly (because there is no type=chrome parent there). From an API point of view, I think it would make more sense to get the docshell's treeowner and add a new API on nsIDocShellTreeOwner to ask it whether it's in the same window as another tree owner.
But on a deeper level, this seems like the wrong place to solve the problem. Why is "check whether we're in the same chrome window" the right thing to check? It seems like the _really_ right thing to check is whether a window is resizable. That should be up to the embedding app to flag. That way if I drag one of the tabs involved to another window, things don't suddenly become resizable...
I'd be happy to talk about a decent API for implementing embedder control over this stuff if you want.
Assignee | ||
Comment 18•14 years ago
|
||
Boris, would you be fine with comments 16 and 17 resolved in follow-ups?
Assignee | ||
Comment 19•14 years ago
|
||
Updated with jst's comments.
Attachment #519751 -
Attachment is obsolete: true
Attachment #523036 -
Flags: review?(bzbarsky)
Attachment #519751 -
Flags: review?(bzbarsky)
Comment 20•14 years ago
|
||
I'm not ok with comment 16 being a followup, due to the website breakage potential.
For comment 17, I'm ok with a better API being a followup. For the e10s bit, we need to check with the fennec folks. Do they even allow resizing at all? If not, then for now it's probably ok to fix the e10s issues as a followup.
Assignee | ||
Comment 21•14 years ago
|
||
(In reply to comment #20)
> For comment 17, I'm ok with a better API being a followup. For the e10s bit,
> we need to check with the fennec folks. Do they even allow resizing at all?
> If not, then for now it's probably ok to fix the e10s issues as a followup.
They do not allow resizing and moving windows.
Comment 22•14 years ago
|
||
I'd prefer to use the heuristic in bug 502561. It's a combination of existing heuristics, so we don't have to worry about getting it wrong. And it avoids blocking resizes that the web page (or popup) can effectively do anyway by closing the old popup and opening a new one.
In particular, I think the "allow if same-origin with opener" approach in this bug would disallow popups from resizing themselves in response to clicks.
Assignee | ||
Comment 23•14 years ago
|
||
Attachment #519630 -
Attachment is obsolete: true
Attachment #523036 -
Attachment is obsolete: true
Attachment #525537 -
Flags: review?(bzbarsky)
Attachment #519630 -
Flags: feedback?(jruderman)
Attachment #523036 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 24•14 years ago
|
||
Two rules:
1. Can't resize a window/tab that hasn't been created by window.open.
2. Can't resize a tab if the tab is in a window with more than one tab.
This is fixing the issue Jesse mentioned (popup can now resize themselves).
Attachment #525540 -
Flags: review?(bzbarsky)
Assignee | ||
Updated•14 years ago
|
Attachment #525540 -
Flags: feedback?(jruderman)
Comment 25•14 years ago
|
||
Comment on attachment 525540 [details] [diff] [review]
Part 2 - Prevent abusively moving/resizing window
Sounds good. We can do the more restrictive stuff later in bug 502561.
Attachment #525540 -
Flags: feedback?(jruderman) → feedback+
Comment 26•14 years ago
|
||
Comment on attachment 525537 [details] [diff] [review]
Part 1 - Change nsIDocShellTreeOwner
> + unsigned long getTargetableShellsCount();
readonly attribute unsigned long targetableShellCount;
(note "shell" vs "shells"). r=me with that changed and a followup bug filed about adding e10s plumbing for this.
Attachment #525537 -
Flags: review?(bzbarsky) → review+
Comment 27•14 years ago
|
||
Comment on attachment 525540 [details] [diff] [review]
Part 2 - Prevent abusively moving/resizing window
> + // Don't allow scripts to move or resize windows that were not open by a
s/open/opened/
r=me
Attachment #525540 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 28•14 years ago
|
||
Unfortunately, I can't push these patches yet because the tests seem to be random on the try server. I'm still investigating why.
Whiteboard: [needs review][needs feedback] → [needs tests fixes]
Assignee | ||
Comment 29•13 years ago
|
||
I've finally fixed the tests... That was hard :)
Flags: in-testsuite+
Whiteboard: [needs tests fixes] → [fixed in cedar]
Assignee | ||
Comment 30•13 years ago
|
||
Pushed:
http://hg.mozilla.org/mozilla-central/rev/1c52f2d68d39
http://hg.mozilla.org/mozilla-central/rev/3c723f2fe07c
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in cedar]
Target Milestone: --- → mozilla7
Assignee | ||
Comment 31•13 years ago
|
||
tracking-firefox-7 given that this is a new feature and might break some websites.
tracking-firefox7:
--- → ?
Comment 33•13 years ago
|
||
sorry for the bug-spam.
i used to use a bookmarklet to resize and center the window for (emulating/)testing different window-sizes (mobile/media-queries).
is there any way to replicate the old behaviour, without writing a (xul) addon?
looks like the addon-sdk has some limitations there, as well.
Comment 34•13 years ago
|
||
You could use an existing addon such as https://addons.mozilla.org/en-US/firefox/addon/firesizer/.
Comment 35•13 years ago
|
||
(In reply to comment #34)
> You could use an existing addon such as
> https://addons.mozilla.org/en-US/firefox/addon/firesizer/.
true. while this is a possiblity, the old way of having a bookmarklet to change the size of the window was far more convenient (and one addon less to worry about). *sad to see it go*
Updated•13 years ago
|
Whiteboard: potential website fallout from new feature
Updated•13 years ago
|
Comment 36•13 years ago
|
||
I definitely agree with matthias koplenig, there should be a possibility in about:config to restore the old behavior. Please don't cut off us who love bookmarklets and do not force us to use addons for such tasks that used to be simple. :-( There's no easier way how to quickly set window size to precise width and height, or height that fits the screen's available height, or height that emulates the available height on for example 1024x768 screen…
Please, add an about:config option. O:-)
Comment 37•13 years ago
|
||
Mozilla/5.0 (Windows NT 6.1; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (Windows NT 6.1; rv:9.0a1) Gecko/20110818 Firefox/9.0a1
Setting resolution to VERIFIED FIXED.
Thanks.
Status: RESOLVED → VERIFIED
Comment 38•13 years ago
|
||
What about the about:config option? No discussion about that? :-(
Updated•13 years ago
|
Keywords: dev-doc-needed
Whiteboard: potential website fallout from new feature → potential website fallout from new feature (for docs, see comment 24)
Comment 39•13 years ago
|
||
There was really no way to accomplish this that would prevent a website from resizing the main window while still allowing users to accomplish the same using bookmarklets? This "solution" has effectively wiped out whole swaths of workflow for organizations that use bookmarklets to allow their Firefox users to organize thin client windows running within their browsers.
There needs to be an option to turn this off — at minimum.
Comment 40•13 years ago
|
||
(In reply to matthias koplenig from comment #35)
> (In reply to comment #34)
> > You could use an existing addon such as
> > https://addons.mozilla.org/en-US/firefox/addon/firesizer/.
>
> true. while this is a possiblity, the old way of having a bookmarklet to
> change the size of the window was far more convenient (and one addon less to
> worry about). *sad to see it go*
Not true. At its latest update, Firesizer is apparently not compatible with Firefox 7.
Comment 41•13 years ago
|
||
> while still allowing users to accomplish the same using bookmarklets?
The whole point of bookmarklets is to run with _exactly_ the same permissions as the currently loaded site. So no, there is no way to let bookmarklets do things that sites can't do.
An addon, of course, can do whatever resizing it wants. That's the right way to handle the use case described in comment 39.
Comment 42•13 years ago
|
||
I'm also here to whine about broken window moving/resizing bookmarklets. Firesizer addon doesn't do cool stuff like "Resize window to half the available width, all the available height and put it on the right side of any screen, no matter its size." I'm pretty annoyed that I have to become an addon developer to replace a homebrewed three-line javascript. I'm ok with a config to turn off this behavior.
Comment 43•13 years ago
|
||
Seeing as there are now very few resizable windows left (basically just popups that only contain their original tab) would it make any sense for the existing pref to override the restriction the other way, i.e. allow all windows?
Comment 44•13 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #41)
> The whole point of bookmarklets is to run with _exactly_ the same
> permissions as the currently loaded site. So no, there is no way to let
> bookmarklets do things that sites can't do.
I admit I'm not a developer. I only created my Bugzilla account yesterday, specifically to address this one issue. I couldn't have told you how Firefox handles permissions for bookmarklets vs. permissions for a site-loaded page.
From the far more general perspective of a user, however, I'd disagree with your assertion that the whole point of bookmarklets is that they should be handled the same way. Ideally, you'd clearly want Firefox to view actions taken by (possibly harmful) code embedded in a site one way, and the intentional actions taken by a user (like selecting a bookmark/bookmarlet from a Firefox menu) another way.
(In reply to Boris Zbarsky (:bz) from comment #41)
> An addon, of course, can do whatever resizing it wants. That's the right
> way to handle the use case described in comment 39.
I'm still looking for an add-on that handles resizing/repositioning, and I'm not in a position to create one. If you're aware of one that exists, please pass it on. Firesizer, mentioned above, isn't it — it apparently doesn't address repositioning, and isn't even FF7 compatible, anyway.
Comment 45•13 years ago
|
||
(In reply to Mounir Lamouri (:volkmar) (:mounir) from comment #24)
> Created attachment 525540 [details] [diff] [review] [diff] [details] [review]
> Part 2 - Prevent abusively moving/resizing window
>
> Two rules:
> 1. Can't resize a window/tab that hasn't been created by window.open.
> 2. Can't resize a tab if the tab is in a window with more than one tab.
>
> This is fixing the issue Jesse mentioned (popup can now resize themselves).
Sorry, but for us, the new resizing behaviour introduced in FF 7 seems to be BROKEN.
1. The TestCase (Attachment 519510 [details]) does NOT work (Windows 7 x64, same on Vista 32). Clicking 'Open Popup' opens a new window (full-screen). Clicking 'Resize popup' just does nothing.
2. We're not exactly a porn site, but a software development company that develops and sells a complex JEE application for use in legal departments, workers unions and Fortune 500 enterprise customers. The software makes extensive use of popup dialogs which auto-resize themselves after opening based on the content displayed. We cannot use JS overlay dialogs, because we need a 'true' window target for displaying error messages. We cannot easily pre-determine popup sizes, as some content (eg. selection lists) can be of variable length and/or width. Plus, a the app is themeable, the final size of the popup window is determined by several factors (font sizes, screen resolutions, dpi) and so on. Finally, we simply cannot recode our entire UI just because of FF 7 support.
Dropping the possibility of auto-resizing popup dialogs as it currently is in FF 7 totally breaks our app's usability. For us, this really is a show-stopper bug that will force us to drop FF 7 from our system requirements list and explicitly exclude it in any SLAs. This is particularly sad as we saw FF slowly, but steadily gaining ground in the enterprise area during the last year, commonly serving as the 'alternative' browser to the still dominant IE.
To sum it up, here is what we need:
1. Open a popup window on button/link click (window.open()).
2. After opening, it must be possible to resize this dialog window to dynamically fit its contents. We commonly use a <div> within the dialog to determine the exact dimensions, calling window.resizeBy(iW, iH) from within the popup contents.
3. Finally, the dialog is centered on screen.
Sadly, about:config is NOT an option in enterprise environments.
Comment 46•13 years ago
|
||
> The TestCase (Attachment 519510 [details]) does NOT work
That's quite odd. It works correctly over here (admittedly, on Mac). The use case you describe should absolutely work; the patch is very careful to keep it working.
Just to make sure, you're testing with Firefox 7 and a clean profile?
Comment 47•13 years ago
|
||
Many thanks for you comment, as it helped us to find the source of the problem and finally saved us some sleepless nights :-)
Well, we had tested the FF 7 upgrade on four different machines (one Vista 32, two Windows 7 x64 and one iMac running Snow Leopard. All FF7 installations were upgrades from FF 6.0.2, German version, some of them having a really long upgrade history since FF 2 or even before (as in the real world). We had not tried an entirely fresh installation yet, so we fired up a Virtual Box running the Windows 8 Developer Preview which had absolutely no Mozilla software on it before and installed FF 7 (English version, as the Preview is also English) from scratch. To our utter astonishment, FF 7 worked as expected on that box. Hooray!
So we set up a new profile from scratch on a Win 7 box and that also worked. Then we tried running an existing installation one of the developer machines in safe mode, and that also worked. So the culprit was not FF7, but some add-in we all had installed in common. Iteratively disabling all suspicious add-ins, both ColorZilla and WebDeveloper proved to be innocent. FireBug 1.8.3 indeed was not. Oddly enough, it had worked without any problems for years, surviving all the upgrades in-between, but we can easily fall back to Chrome if necessary.
So, for what it's worth, the resizing thing seems to be incompatible to FireBug, but works otherwise. Luckily, Firebug is nothing a corporate end user is expected to have installed. Management is content and happy again. Time to go home now and have a beer or three :-)
Thanks again and sorry for the inconvenience. FF 7 is great!
-- Ch.
Comment 48•13 years ago
|
||
Uh... Firebug does indeed add content-targetable docshells to the main window, which this patch thinks are open tabs. They're in firebugOverlay.xul (the fbPanelBar1-browser and fbPanelBar2-browser).
Jan, is there a reason those are content-targetable and not just content, exactly?
Comment 49•13 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #48)
> Jan, is there a reason those are content-targetable and not just content,
> exactly?
There were couple of issues with the attribute already:
http://code.google.com/p/fbug/issues/detail?id=4628
and bug 665369
Since Firebug now uses <browser> instead of <iframe> I think the type attribute can be 'content'. Patch committed to Firebug SVN:
http://code.google.com/p/fbug/source/detail?r=11993
Both mentioned issues tested (all ok)
Will be included in Firebug 1.9a4
(released soon here: http://getfirebug.com/releases/firebug/1.9/)
Thanks for the report!
Honza
Comment 50•13 years ago
|
||
Killing off the window resizing functionality was a bad idea.
I understand the usability implications for sites that choose to resize windows, and it is unfortunate that many sites *abuse* the ability to resize windows. I would like to propose the following functionality for resizing windows:
* A window may not be resized smaller than would allow for the display of the window handles and necessary buttons
* A window may not be resized larger than the largest screen it can occupy
* a page may resize itself and any pages it has created
* if a particular page shares a window with a page that it did *not* create, the page is popped out of the original window into its own window, and then resized
* if child pages existed within the original window, they are left where they were
As it is, there is an option to "Allow Scripts to move or resize existing windows". Currently this option does not work, and needs to be fixed. If the resizing windows functionality is removed completely, the option needs to be removed as well.
Comment 51•13 years ago
|
||
Comment 52•13 years ago
|
||
BTW, I wonder what the "tracking-firefox7:+" flag is expected to do for bugs like this.
Was there a release note, any documentation, a blog entry? Any tiny try to inform web developers about this before things break unexpectedly? I haven't seen it.
dev-doc-needed keyword was added 3 days before release.
Seriously: What's the point with that flag here?
Comment 53•13 years ago
|
||
(In reply to j.j. from comment #52)
> Any tiny try to
> inform web developers about this before things break unexpectedly? I haven't
> seen it.
For the most part, this only breaks web pages that are using this in ways that already break things. We are still allowing popup windows to resize themselves. We simply aren't allowing tabs in full-UI main browser windows to resize themselves, because they affect other tabs. In other words, we're actually fixing a lot more breakage than we're causing.
Comment 54•13 years ago
|
||
Frank, that's not my point. I'm personally happy with this fix.
Comment 55•13 years ago
|
||
(In reply to j.j. from comment #52)
> BTW, I wonder what the "tracking-firefox7:+" flag is expected to do for bugs
> like this.
Tracking means that the release team was interested in watching this as it made its way through the channels. It's not supposed to "do" anything.
Comment 56•13 years ago
|
||
> what the "tracking-firefox7:+" flag is expected to do for bugs like this.
Make sure that the release team will keep an eye out for regressions from the bug that would make shipping it not OK. For example, if we'd had widely-reported site breakage as a result of this change.
The definition of "widely" is somewhat open-ended there.
That also relies on people flagging actual reports of breakage as blocking the bug that caused it, of course.
Comment 57•13 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #56)
> That also relies on people flagging actual reports of breakage as blocking
> the bug that caused it, of course.
I bet that most people who actually do first-step-bug-tracking didn't realise that this is fixed, including me. (And if they did, very most of them woudn't add the dependency)
There is something broken with the process. I assume we have paid people to improve this.
And yes, this bug is not the best example or my complaint. There were others in the past.
</rant>
Comment 58•13 years ago
|
||
Comment 59•13 years ago
|
||
Makes me wonder if developers really use Firefox. Tools > Options > Content > Javascript: Advanced allows the user to prevent resizing. I almost always allow these to happen unless something causes a problem and tries to take over my machine. The days of user control and customization are numbered, bad programs will take over. I just lost 27 resize/move bookmarklets to put windows exactly where and how I want them and would have added to them.
Assignee | ||
Comment 60•13 years ago
|
||
(In reply to David McRitchie from comment #59)
> Makes me wonder if developers really use Firefox. Tools > Options > Content
> > Javascript: Advanced allows the user to prevent resizing.
This option gives the user the possibility [if unchecked] to prevent all move or resize even those made on popups which could be legitimate and needed for many reasons. If checked, it will allow all move or resize even those that are going to be really painful for the user like resizing the main window.
With this patch I believe this option is no longer required because we prevent known to be bad resize and move and allow the other ones.
Comment 61•13 years ago
|
||
> Tools > Options > Content > Javascript: Advanced allows the user to prevent resizing
See bug 690648
Comment 62•13 years ago
|
||
From previous comments, I understand my FF 7.0.1 upgrading from FF 6 can't work. And I've made sure that the fresh installation of FF 7.0.1 on my virtual machine works. I just worry about what would happen if user gets next FF release? Will window.resize* works fine in the new opened window? Or we need suggest them to install a fresh new Firefox?
Assignee | ||
Comment 63•13 years ago
|
||
(In reply to josephj from comment #62)
> From previous comments, I understand my FF 7.0.1 upgrading from FF 6 can't
> work.
What do you mean?
Comment 64•13 years ago
|
||
In comment #46, it mentions "Firefox 7 and a clean profile".
My situation is similar... that FF 7.0.1 upgrading from 6 can't do window.resizeBy() in a new opened window, but a new installed FF 7.0.1 does. It's difficult to ask our users to fix existing profile. Just wondering if it's possible to fix profile automatically in the next versions.
(In reply to Mounir Lamouri (:volkmar) (:mounir) from comment #63)
> (In reply to josephj from comment #62)
> > From previous comments, I understand my FF 7.0.1 upgrading from FF 6 can't
> > work.
>
> What do you mean?
Comment 65•13 years ago
|
||
The only reason I asked about a clean profile is to eliminate extensios as the culprit.
And as comment 47 and comment 48 say, there is indeed an old bug in Firebug that breaks window resizing as of Firefox 7. In a clean profile with no Firebug installed the problem goes away, of course.
So if in your case the upgraded Firefox 7 has Firebug installed, you're seeing that. If that's the case, it should not affect your users unless they have Firebug installed, and even if they do it should work once Firebug ships a fix for their issue.
If you do _not_ have Firebug installed but are seeing window resizing not work, please file a separate bug and cc me on it.
Comment 66•13 years ago
|
||
(it's fixed in Firebug 1.9a4) http://blog.getfirebug.com/
Comment 67•13 years ago
|
||
Comment #60 and #64 -- I sure hope "Alice White" or someone comes to our rescue with an extension to reverse all of this so we can resize, move with JavaScript, run Janascript from the location bar once more, and have the JavaScript controls in Tools > Options > Content still be able to allow or restrict "Move or resize existing windows", "Raise or lower windows", "Disable or replace context menus". I have hardly ever had a problem with sites because I can control the bad ones that that look like a virus scanner (twice). Never had any real problem with legitimate pages, and I resized windows thousands of times (several times a day) with bookmarks invoked from the location bar.
This change is not going to stop add-ons and programs from doing anything, and just makes Firefox a bad application like what it is being changed to stop. The other browsers are not doing this, at least not yet. Many of us switched to Firefox because we had control and could customize. The ability to place windows side by side in Windows 7 is a good feature, but my bookmarks invoked from the location bar can (could) make Firefox windows any size and position them where I wanted them.
Comment 68•13 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #65)
> If you do _not_ have Firebug installed but are seeing window resizing not
> work, please file a separate bug and cc me on it.
It's okay after I upgrade Firebug to 1.9.0a4.
Thanks for your help. :-D
Comment 69•13 years ago
|
||
> It's okay after I upgrade Firebug to 1.9.0a4.
Thanks for the update!
Honza
Comment 70•13 years ago
|
||
This change or one of its evil siblings also makes the AwesomeBar feature and bookmark sidebar/library searches not work properly. Can't find keyworded bookmarks that have JavaScript. or bookmarks with javascript.
Test case involving a typical JavaScript example with code that users all over the world use to test and learn JavaScript
Name: hello: howdy doody
Location: javascript:alert("Hello%20World")
Keyword: hello:
Try AwesomeBar type string search from location bar, bookmarks sidebar/library on any of the following: (last is valid it is strings not words)
javascript || hello world || alert || hello: || ello world
Comment 71•13 years ago
|
||
That has absolutely nothing to do with this bug. It does have to do with bug 656433, which has extensive discussion of the tradeoffs involved and the better suggested ways to test and learn JavaScript.
Comment 72•13 years ago
|
||
There seems to be no security issue here; the report was just about web site behavior that was perceived to be annoying. I would like to point out two solutions that do not break existing web site code:
1) Do not visit web sites that annoy you.
2) If you must visit a web site that resizes your window, and you are annoyed by that, then open a new window for that web site before going to it.
I think there needs to be some review process for changes like this (which are not bugs, and which break existing web sites) where web site developers (and perhaps users) could comment on the change before it is implemented.
Comment 73•13 years ago
|
||
> I think there needs to be some review process for changes like this
There should be some information process before posting such (to be frankly) nonsense, e.g. by reading this bug completely, including depending, blocking and duplicate bugs.
(If you feel a need to respond, please do it in private mail and don't comment in the bug.
See also
https://bugzilla.mozilla.org/page.cgi?id=etiquette.html
point 2.2.)
Assignee | ||
Comment 74•13 years ago
|
||
(In reply to Paul Lynch from comment #72)
> There seems to be no security issue here
If website A opens a popup P and changes the size of the main window (where A is) to make it smaller than P and behind P and then shows a message like "You have been victim of an attack that crashed your browser and stole your personal data, install this security software to prevent this to happen again!". That sounds like a serious security issue. And I'm not speaking of the DOS issues.
Comment 75•13 years ago
|
||
(In reply to Mounir Lamouri from comment #74)
If a setting already exists to enable/disable resizing and moving windows, there is no reason to remove the feature. Instead, the default should be changed such that they are disabled by default, or another setting should be added so that a user can specify whether a page can resize windows with multiple tabs, which could default to false.
Anyone foolish enough to fall for a security software scam isn't likely to change their default window resize settings.
Comment 76•13 years ago
|
||
Thanks for saving me from all the bad guys trying to move and resize my Firefox window! I'm the most stupid user, who cannot configure his own browser. I really need Big Brother and finally I shall love him.
What next? You could forbid the links because they probably lead to nasty pages.
Tanks a lot!
Really, great job!
Comment 77•13 years ago
|
||
(In reply to Mounir Lamouri (:volkmar) (:mounir) from comment #74)
> (In reply to Paul Lynch from comment #72)
> > There seems to be no security issue here
>
> If website A opens a popup P and changes the size of the main window (where
> A is) to make it smaller than P and behind P and then shows a message like
> "You have been victim of an attack that crashed your browser and stole your
> personal data, install this security software to prevent this to happen
> again!". That sounds like a serious security issue. And I'm not speaking of
> the DOS issues.
Thanks-- I had not thought of that. However, this is a case of a malicious website, in which case lots of bad things might happen. What if, to work around this change, website A resizes the pop-up to cover the original window? It might not be as visually convincing to an experienced user, but as long as the new window contained the same message, some people would still fall for it. My point is that this only eliminates some cases of malicious window resizing, and breaks legitimate uses of it.
I can modify my website to deal with the change, but I would have liked some warning that it was coming. (Was it somewhere, that I missed, other than buried in bugzilla?) Even better, I would have liked if there had been some opportunity to give some feedback about what features of my website were about to be broken.
Comment 78•13 years ago
|
||
Paul, I'm not sure about warning; there are things like commit logs and blogs about recent changes, but we obviously don't expect web developers to follow them... So I'm not sure what form of warning would have been useful here.
You always have the opportunity to give feedback, though: after any change is checked in there are 12 weeks of testing on the Aurora and Beta channels before it actually ships in a release (plus whatever testing it gets on the Nightly channel). I realize that testing the new Aurora build against your website every 6 weeks is a bit of a pain, though...
Assignee | ||
Comment 79•13 years ago
|
||
Reading https://hacks.mozilla.org/ might be a good idea to learn about new changes in Firefox/Gecko. Actually, this change has been mentioned in the Firefox 7 article: https://hacks.mozilla.org/2011/09/whats-new-for-web-developers-in-firefox-7/
As said Boris, running an Aurora or Beta build against your website sometimes might prevent bad surprises.
Comment 80•13 years ago
|
||
(In reply to Mounir Lamouri (:volkmar) (:mounir) from comment #79)
> Actually, this change has been mentioned in the
> Firefox 7 article:
> https://hacks.mozilla.org/2011/09/whats-new-for-web-developers-in-firefox-7/
Yes, and I should have paid more attention to that, because I do try to follow that blog. However, that article was released the day Firefox 7 was released (out of beta). At that point, web sites are already broken. It might even be too late to provide feedback when the change is in Aurora; at that point I gather that the change is approved and just being tested for bugs. What I was suggesting is that when a change to disable a certain feature is under consideration, and before it is implemented, there be some mechanism (whether a blog, a forum, or an email list) by which developers could be notified and have some opportunity to discuss the impact of the change.
Comment 81•13 years ago
|
||
Marking dev docs as complete now.
https://developer.mozilla.org/en/Firefox_7_for_developers#DOM
https://developer.mozilla.org/en/DOM/window.resizeBy
https://developer.mozilla.org/en/DOM/window.resizeTo
https://developer.mozilla.org/en/DOM/window.moveBy
https://developer.mozilla.org/en/DOM/window.moveTo
Keywords: dev-doc-needed → dev-doc-complete
Comment 82•13 years ago
|
||
I have a problem with this being marked VERIFIED FIXED without some kind of option, since allowing JavaScript to function as intended isn't a bug. Unilaterally disabling a specific function of a "supported" scripting language doesn't seem to be a fix to me.
This needs option support or a whitelist. See Bug 688841, comment 11 for a more detailed argument against this "fix".
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•