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

Status: RESOLVED WORKSFORME

(Core :: Layout, defect)

Description

[Burak Yiğit Kaya [:BYK]]

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.

Comment 1

[Burak Yiğit Kaya [:BYK]]

Attachment: The test used to create the bug

Attaching the test file used to create the problem.

Comment 2

[aravindm]

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

Comment 3

[Burak Yiğit Kaya [:BYK]]

As a side note, hardware acceleration is enabled which might be related with the problem.

Comment 4

[Justin Dolske [:Dolske]]

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.

Comment 5

[Justin Dolske [:Dolske]]

Attachment: 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.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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...

Comment 7

[Justin Dolske [:Dolske]]

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?

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(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.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Is this a regression? It's a bad bug, but if it's also an old bug, it shouldn't block FF4.

Comment 11

[Makoto Kato [:m_kato]]

(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.

Comment 13

[Alistair Potts]

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.

Comment 14

[Keul]

Attachment: shorter testcase - resize to see bug - F5 to clear

Got same bui in my page. Done a shorter testcase from my page.

Comment 15

[Nikita Popov]

@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.

Comment 18

[Hans Nieser]

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

Comment 19

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: 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.

Comment 24

[Gyorgy Kapolnas]

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.

Comment 25

[Boris Zbarsky [:bzbarsky]]

> 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?

Comment 26

[Boris Zbarsky [:bzbarsky]]

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.

Comment 27

[Boris Zbarsky [:bzbarsky]]

Attachment: Test patch to do that

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.

Comment 29

[Boris Zbarsky [:bzbarsky]]

There's a test build at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-e76797db5a70/try-win32/

Comment 30

[soporte]

(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....

Comment 31

[Cork]

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

Comment 34

[niente9]

Same problem here, Win XP Pro 32bit - Firefox 5.0
After seven months, this bug is still here. Sigh!

Comment 37

[Alice0775 White]

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

Comment 38

[Alice0775 White]

Full zoom in/out also triggers the same problem.(Tested with attachment 523828 [details])

Comment 39

[Martijn Wargers (dead)]

I guess bug 670666 is related?

Comment 44

[jmbean]

Bug also shows up when a brakpoint is set on any statement in th code that is invoked by onresize event.

Comment 46

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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?

Comment 47

[Justin Dolske [:Dolske]]

(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.

Comment 48

[Roman]

(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?

Comment 49

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(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.

Comment 50

[Aleksander Heintz]

(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.

Comment 51

[Boris Zbarsky [:bzbarsky]]

> 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.

Comment 52

[Jesse Ruderman]

Regarding beforeunload, see bug 391834 for disabling extra alerts from it, and bug 578828 for disabling it entirely.

Comment 55

[[e-mail]]

I experienced something similar 

ff9, ubuntu 11

http://jsfiddle.net/ZvzWM/2/

http://crash-stats.mozilla.com/report/index/bp-bf2e6059-a9ba-4bb3-b579-74d562120111

Comment 58

[Bill Zaumen]

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>

Comment 59

[Karl Tomlinson (:karlt)]

Filed Bug 726483 on the GTK crash on close.

Comment 66

[Karl Tomlinson (:karlt)]

I understand that work in bug 539356 will mean that FlushPendingNotifications will no longer be called from native paint events.

Comment 67

[Cork]

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].

Comment 68

[f3000d]

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).

Comment 69

[Cork]

The commit is in trunk that's firefox 17. Testing of release is known to have the old behavior.

Comment 70

[Cork]

Ugg just noticed my browser changed the values; changing back.

Comment 73

[donrhummy]

2.5 years later and this bug is still not fixed. What can be done to help?

Comment 75

[donrhummy]

Again, I ask: what can I (or others) do to help fix this 3 year old bug?

Comment 76

[[e-mail]]

I can't recreate this bug on ff24.

Comment 77

[Mattias Lindholm]

(In reply to [e-mail] 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!

Comment 78

[B.J. Herbison]

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.

Comment 79

[Johan C]

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.

Comment 80

[Burak Yiğit Kaya [:BYK]]

>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.

Comment 81

[Mats Palmgren (inactive)]

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.

Comment 82

[Roman]

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.

Comment 83

[:Gijs (he/him)]

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.