Closed Bug 626963 Opened 12 years ago Closed 1 year ago

Modal dialog (alert) on window.onresize handler causes many alert windows to open

Categories

(Core :: Layout, defect)

x86_64
All
defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME
blocking-basecamp -
Tracking Status
blocking2.0 --- -

People

(Reporter: BYK, Unassigned)

References

Details

(Keywords: regression)

Attachments

(5 files)

User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b9) Gecko/20100101 Firefox/4.0b9
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b9) Gecko/20100101 Firefox/4.0b9

I was messing up with the new modal dialog style and prepared a test case which horribly crashed my Firefox rendering and caused issues while closing the browser which lead to a loss of all the information of currently opened tabs and the tab groups kept in Panaroma.

The test case is simply firing a "confirm" dialog on the windows.onresize event. The test file is attached.

Reproducible: Always

Steps to Reproduce:
1. Open the test page
2. Resize the window
Actual Results:  
The "page content" area went black, all tabs, toolbars, controls etc. excluding the original Windows window frame and control buttons were lost. The "Firefox Button" on the top left was also lost. Ctrl + W to close the current tab either did not work or simply did not brought the rendering back though the window kept responding my resize attempts with its current state and when I tried to close it, it asked me that I have multiple tabs open etc. with a traditional Windows modal prompt.

Expected Results:  
In Chromium, the same test simply keeps the prompt box and the browsers keeps responding to the user perfectly well. I expect the same behavior.

This issue is an edge case, I accept but an attacker can use this to annoy the users as in the case of an alert loop.
Attaching the test file used to create the problem.
Build worked : 
Mozilla/5.0 (Windows NT 6.1; rv:2.0b5pre) Gecko/20100827 Minefield/4.0b5pre

Build broken : 
Mozilla/5.0 (Windows NT 6.1; rv:2.0b5pre) Gecko/20100828 Firefox/4.0b5pre

Pushlog: 
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e1d55bbd1d1d
&tochange=6e3f6d18c124
Status: UNCONFIRMED → NEW
Ever confirmed: true
As a side note, hardware acceleration is enabled which might be related with the problem.
Moving per dbaron. On Windows 7, has a pretty bad effect on the Firefox window (everything disappears except the glass and a black content area). Additionally, the mouse pointer gets stuck as a resize pointer and it's difficult to switch apps. Pressing ESC will get you out (since the prompt is actually running, but not painted).

Window-modal prompts have the same problem with the Firefox window's appearance, although the dialog window is visible and the mouse pointer / behavior is otherwise ok.
blocking2.0: --- → ?
Component: General → Layout
Product: Firefox → Core
QA Contact: general → layout
Version: unspecified → Trunk
Attached file Stacktrace
Here's the stack of this happening (with a tab-modal prompt).

Hilights:

main event loops --> processing a windows message --> nsWindow::OnPaint --> PresShell::FlushPendingNotifications --> PresShell::FireResizeEvent --> JS --> nsGlobalWindow::Confirm --> new event loop.

I think dbaron's running-out-the-door thought was was to not fire the event synchronously, and instead make the refresh driver aware of resize event listeners.
Well, so that was more in the "once painting is done off the refresh driver", we could make resize events be a phase managed by the refresh driver (post-layout-causing-things), such that after firing the resize events we'd flush layout again, and resize events would run only off the main refresh loop.

I'm less sure what to do about it now, though.  I believe resize events are synchronous because pages want them to get handled before we paint, so that incorrect layouts don't flash in-between paints.  I wish we didn't flush before painting...
One option, from the prompting side, is that we could just refuse to show a prompt (throw) if it's requested in this state. Calling alert() from a resize handler doesn't seem terribly useful or common to me, at least.

Is is possible to detect when we're in this state from JS? Or flip a property somewhere on the chrome window?
(In reply to comment #7)
> One option, from the prompting side, is that we could just refuse to show a
> prompt (throw) if it's requested in this state. Calling alert() from a resize
> handler doesn't seem terribly useful or common to me, at least.
> 
> Is is possible to detect when we're in this state from JS? Or flip a property
> somewhere on the chrome window?

I think we should add a getter to nsIWidget which says "don't open model windows now", make that return true during OnPaint, and check it where we open modal windows.
Is this a regression? It's a bad bug, but if it's also an old bug, it shouldn't block FF4.
Duplicate of this bug: 627311
(In reply to comment #9)
> Is this a regression? It's a bad bug, but if it's also an old bug, it shouldn't
> block FF4.

On Firefox 3.6, prompt dialog is appeared on this test case.  But on Firefox 4, it doesn't draw prompt dialog and content becomes black since alert dialog is changed from window modal dialog to content modal dialog.

So this is a regression as user/web developer.
Keywords: regression
Duplicate of this bug: 643777
I picked this up writing a resize-end event (settimer/cleartimer on the resize event). Popped in an alert at the end to debug that it was actually done. Black screen etc.

My point being that a prompt triggered by resize event might not be especially useful in the real world, but isn't always bogus.
Severity: normal → major
Got same bui in my page. Done a shorter testcase from my page.
@Keul125: If you want something short just use data:text/html,<script>onresize=alert</script>

@all: This bug should be blocking whatever-can-be-blocked-currently. It
a) makes debugging resize events a real pain for developers and
b) could be used as a really easy way (onresize=alert;resizeTo(x,y)) to break the browser and let people lose their current session.
Duplicate of this bug: 649413
Duplicate of this bug: 650588
Hrm, I just ran into this bug while trying to make sure I was hooking up the onresize event correctly by calling alert() in the onresize handler. Not something anyone would do 'in the real world' I suppose but it can sure trip up developers doing debugging.

Happens on Linux+Gnome as well by the way, and I couldnt reproduce it with 3.6
Attached file another testcase
Written while writing http://dbaron.org/log/20110422-matchMedia .  Resize the window across 600px (i.e., from wider than to narrower than, or the other way around) to see the bug.
Summary: Modal dialog on window.onresize handler causes rendering to stop → Modal dialog (alert) on window.onresize handler causes rendering to stop
Duplicate of this bug: 655316
Duplicate of this bug: 656974
Duplicate of this bug: 659275
Duplicate of this bug: 649296
The bug is present in FF5 as well. The nice thing about it is that any tutorial on how to use the onresize event demonstrates how it works using an alert. I was pretty sure onresize is completely broken before finding this bug as any example I found on the net froze FF.
> I wish we didn't flush before painting...

We flush to avoid painting intermediate states, I thought.

Can we avoid flushing during painting specifically if there's a resize pending?
Given that the flush from WillPaint() is interruptible, I'd hope there are no invariants it's expected to maintain for painting, by the way.
On Mac, this is still broken because we think the resizer is still being dragged once the modal dialog comes up.  But could someone on Windows test this, please?

If there's someone willing to test who can't compile themselves, please let me know and I'll spin up a Windows test build.
Duplicate of this bug: 670117
(In reply to comment #29)
> There's a test build at
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bzbarsky@mozilla.
> com-e76797db5a70/try-win32/
Do not work in Windows... same issue... but this time I can see the page under..

Regards....
Not sure if this is of interest, but I got this crash when closing the browser after hitting this bug.

bp-85123a50-5487-446f-9738-86a172110714
Duplicate of this bug: 672088
Whiteboard: [sg:low][sg:dos]
Duplicate of this bug: 676758
Same problem here, Win XP Pro 32bit - Firefox 5.0
After seven months, this bug is still here. Sigh!
Duplicate of this bug: 348980
Duplicate of this bug: 677110
Keywords: regression
OS: Windows 7 → All
Tested with attachment 523828 [details]

#1 Content and Browser UI fail to redraw when resize window, but alert box appears.
Regression window(cached m-c hourly):
Works:
http://hg.mozilla.org/mozilla-central/rev/01fa971e62ee
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b5pre) Gecko/20100827 Firefox/4.0b5pre ID:20100827190212
Fails:
http://hg.mozilla.org/mozilla-central//rev/6e3f6d18c124
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b5pre) Gecko/20100828 Firefox/4.0b5pre ID:20100828230738
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=01fa971e62ee&tochange=6e3f6d18c124
In local build:
Build from 7804b5cf4313 : fails
Build from 4722b491cd0d : works
Regressed by:
Bug 130078 - integrate iframe into chrome view hierarchy (link view managers / trees between chrome and content)

#2 Content and Browser UI fail to redraw and black when resize window, and no alert box appears.
Regression window(cached m-c hourly):
Fails as #1:
http://hg.mozilla.org/mozilla-central/rev/b89d1824a762
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101119 Firefox/4.0b8pre ID:20101119193855
Fails:
http://hg.mozilla.org/mozilla-central/rev/b1f6e5f93eb8
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101119 Firefox/4.0b8pre ID:20101119220651
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b89d1824a762&tochange=b1f6e5f93eb8
Regressed by:
Bug 59314 - JavaScript alerts should be content-modal, not window-modal
Blocks: 130078, 59314
Keywords: regression
Full zoom in/out also triggers the same problem.(Tested with attachment 523828 [details])
I guess bug 670666 is related?
Duplicate of this bug: 680014
Duplicate of this bug: 684693
Duplicate of this bug: 686166
Duplicate of this bug: 686885
Blocks: 685457
Bug also shows up when a brakpoint is set on any statement in th code that is invoked by onresize event.
Duplicate of this bug: 693569
The same bug occurs if we flush pending onscroll events during painting (which we want to do in bug 696248).

What if we just cause any modal event loop, including alert(), to fail if triggered in an onscroll or onresize handler?
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #46)

> What if we just cause any modal event loop, including alert(), to fail if
> triggered in an onscroll or onresize handler?

That's an option. We're talking about doing something similar for onbeforeunload, because sites doing that are generally being abusing [eg onbeforeunload="alert('click Stay on Page!'); return true;"]

OTOH, I'm surprised by how common prompts in onresize() appear to be. I assume this is mostly from webdevs accidentally running across this? Or is there some actual usecase relevant to end-users? Still, now that we natively support console.log() maybe that's a better pattern to push people towards.
(In reply to Justin Dolske [:Dolske] from comment #47)
> OTOH, I'm surprised by how common prompts in onresize() appear to be. I
> assume this is mostly from webdevs accidentally running across this?

My dupe was indeed accidental - can't think of any use case for this. The ratio of "seen" to "reported" might be unusually high for this bug because it's unmissable.

Not showing the alert isn't ideal because it might seem that onresize didn't get triggered. Perhaps you could at least log the message to the console with a brief explanation if you choose this route?
(In reply to Justin Dolske [:Dolske] from comment #47)
> OTOH, I'm surprised by how common prompts in onresize() appear to be. I
> assume this is mostly from webdevs accidentally running across this?

I assume webdevs hit this when debugging their scripts.
(In reply to Justin Dolske [:Dolske] from comment #47)
> That's an option. We're talking about doing something similar for
> onbeforeunload, because sites doing that are generally being abusing [eg
> onbeforeunload="alert('click Stay on Page!'); return true;"]
> 
> OTOH, I'm surprised by how common prompts in onresize() appear to be. I
> assume this is mostly from webdevs accidentally running across this? Or is
> there some actual usecase relevant to end-users? Still, now that we natively
> support console.log() maybe that's a better pattern to push people towards.

Can I just comment that several sites use onbeforeunload to alert users who are trying to leave with unsaved changes? I know several sites who are using this, and I also know that as it stands if this possibility is removed it will effectively force us to remove firefox from the list of supported browsers as it would not support one of our features.
> several sites use onbeforeunload to alert users who are trying to leave with unsaved
> changes?

Presumably via the normal onbeforeunload mechanism for this (which consists of returning the prompt string, etc), yes?  That would be unaffected by suppression of alert() calls inside the handler.
Regarding beforeunload, see bug 391834 for disabling extra alerts from it, and bug 578828 for disabling it entirely.
Duplicate of this bug: 706772
Duplicate of this bug: 717115
Duplicate of this bug: 718578
Duplicate of this bug: 719891
Blocks: 723532
Have a very short file that will cause Firefox 10.0 to hang (and pop up a dialog box indicating a crash when the window is closed).  It defines a javascript function
rs() that calls alert("resized") and adds onresize="rs()" to the BODY tag.

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" 
	  "http://www.w3.org/TR/html4/strict.dtd">
<HTML>
<HEAD>
<META http-equiv="Content-Type" content="text/html; charset=UTF-8">
<META http-equiv="Content-Style-Type" content="text/css">
<META http-equiv="Content-Script-Type" content="text/javascript">
<TITLE>Test</TITLE>
<SCRIPT>
function rs() {
  alert("resized");
}
</SCRIPT>
</HEAD>
<BODY id="body" onresize="rs()">

<H1 id="heading">Hello</H1>

</BODY>
</HTML>
Blocks: 726483
Filed Bug 726483 on the GTK crash on close.
Duplicate of this bug: 729566
Duplicate of this bug: 747541
Blocks: 727874
Duplicate of this bug: 740323
Duplicate of this bug: 727874
Duplicate of this bug: 749911
blocking-basecamp: --- → -
Duplicate of this bug: 764404
I understand that work in bug 539356 will mean that FlushPendingNotifications will no longer be called from native paint events.
Depends on: dlbi
Tested nightly 20120820030522 that should have the landings from bug 539356. Things has gotten a lot better, it mostly works find now.

I can occasionally trigger the old bug ,where the ui stops updating until you reload the page. Haven't been able to find an str that doesn't involve trying it a few times though.

Tested this with attachment 505013 [details].
Target Milestone: --- → mozilla12
Version: Trunk → 14 Branch
This bug still bugs in FF 14.0.1.

Behavior depends on what OS you're using, but this bug doesn't crash the browser anymore (assumption based on light testing).

Test scenario contains one alert() inside a function called by onresize.

On Windows 7, UI gets locked up if you resize the window from border-drag. Cursor cannot be moved out of the area that browser window COULD cover (I cannot move the cursor over the bottom-bar where Win-menu is), and cursor's appearance is stuck to the resize-cursor. Yet pressing esc-key saves the day. Using the resize-button next to that red X causes onresize to be called twice, assuming so because the alert pops twice. Otherwise it seems to work ok.

On Ubuntu 12.4 (virtualbox), window resizing with border-drag will cause alert() to be triggered several times, like once per ten pixels of dragging. Stacking enough alerts will cause the window to stop refreshing the view. Resizing from resize-button fixes that situation.

On Ubuntu 10.4, window resizing from either resize-button or border-drag will cause the window to stop refreshing after closing the alert, with exception of right-button menu. When the refreshing "dies first time", the alert-box wont disappear (OK button visual is left to "pushed"). After refresh-stop, resizing the window refreshes the view twice: once after resize, and once after closing the alert (alert box disappears), but then it's dead again, unless you have several alerts from border-drag, then you're able to close all of the alert-boxes (but that's it). Cursor reacts to the content, as example moving cursor above url-field makes it "text-cursor". You can still interact with the window content, because switching tab changes window title. There's no way to get things back to normal on that window, only round-around is moving tab(s) into new window(s). Reload doesn't help, if it happens at all (no visual/actional proof). Only exception to abovementioned behavior is when you use resize-button to resize from "full screen" - that first resizing doesn't yet break anything, but next one, no matter if it's resize-button or border-drag, your view refreshing is gona die. (I'm still with 10.4 because 12.4 BROKE few features I'm using (like webdavs with self-signed cert))

It indeed disturbs mainly webdevs (like myself) by affecting debugging (thru alerts).
The commit is in trunk that's firefox 17. Testing of release is known to have the old behavior.
Ugg just noticed my browser changed the values; changing back.
Target Milestone: mozilla12 → ---
Version: 14 Branch → Trunk
Duplicate of this bug: 813624
Duplicate of this bug: 866098
2.5 years later and this bug is still not fixed. What can be done to help?
Duplicate of this bug: 880934
Again, I ask: what can I (or others) do to help fix this 3 year old bug?
I can't recreate this bug on ff24.
(In reply to joshua.r.marshall.1991@gmail.com from comment #76)
> I can't recreate this bug on ff24.

Reproduce steps for ff24:

1) Go to about:blank
2) Open up the JS console and enter the statement:
window.onresize = function(e) { alert(1); };
3) Attempt to resize the browser window!
Duplicates: bug 626963 and bug 846960.

I'm not sure which one to dupe as they are placed under different components an I'm not sure which component is correct.
Blocks: 730646
Can you still reproduce this Burak?

I've closed bug 723532, which is a similar bug (if not the same bug) due to not being able to reproduce it any longer.
Flags: needinfo?(madbyk)
>Can you still reproduce this Burak?

I tried this on my initial test page using Firefox Developer Edition 36.0a2 (22-12-2014) and the browser did NOT freeze. That said I ended up a bazillion confirmation box overlays which were practically impossible to get rid of.

So I think the "attack vector" is somewhat compensated since the browser does not freeze. It does slow down a bit tho.
Flags: needinfo?(madbyk)
No freeze or painting issue when using the STR in comment 77 in Nightly on Linux.
A lot of alert windows are opened but it's a minor annoyance now that they are
tab-modal.  I don't think this is a security issue.
Severity: major → normal
Keywords: sec-low
Summary: Modal dialog (alert) on window.onresize handler causes rendering to stop → Modal dialog (alert) on window.onresize handler causes many alert windows to open
Whiteboard: [sg:low][sg:dos]
Agreed, it's no longer a major DoS/dataloss issue.

It can still be a major annoyance in a rare corner case: while the testcase tab is open, it's not possible to resize or move the Firefox window, even if another tab is active. It's going to be very difficult to figure out why Firefox is suddenly no longer movable or resizable if you forgot that you opened this tab, but at the same time, it _is_ rather theoretical at this time.
Blocks: 1225520

I'm closing out old many-dupes bugs that I believe are fixed, to make it easier to find more recent issues with a lot of dupes. AFAICT this is a lot better, and even this:

(In reply to Roman from comment #82)

Agreed, it's no longer a major DoS/dataloss issue.

It can still be a major annoyance in a rare corner case: while the testcase
tab is open, it's not possible to resize or move the Firefox window, even if
another tab is active. It's going to be very difficult to figure out why
Firefox is suddenly no longer movable or resizable if you forgot that you
opened this tab, but at the same time, it is rather theoretical at this
time.

I cannot actually reproduce - I can resize Firefox fine with a content prompt open, even if opened from a resize handler.

Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.