Closed
Bug 72152
Opened 23 years ago
Closed 23 years ago
sizeToContent() crashes on properly sized window
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
VERIFIED
FIXED
mozilla0.9.4
People
(Reporter: colinp, Assigned: waterson)
References
Details
(Keywords: crash, Whiteboard: fixed-on-trunk, critical for 0.9.4)
Attachments
(3 files, 1 obsolete file)
261 bytes,
text/html
|
Details | |
1.58 KB,
patch
|
Details | Diff | Splinter Review | |
709 bytes,
patch
|
Details | Diff | Splinter Review |
From Bugzilla Helper: User-Agent: Mozilla/5.0 (X11; U; Linux 2.2.16 i686; en-US; 0.8.1) Gecko/20010314 BuildID: stable 0.8 from cvs when I call window.sizeToContent on a window that is already the correct size (ie, window.sizeToContent doesn't have to redraw anything becuase the window hasn't changed size), then Mozilla seg faults: /package/bin/run-mozilla.sh: line 72: 2630 Segmentation fault Reproducible: Always Steps to Reproduce: 1. open a window 2. call window.sizeToContent once 3. call window.sizeToContent twice Actual Results: segmentation fault Expected Results: nothing should happen (the window hasn't changed size, so it shouldn't redraw anything)
Comment 1•23 years ago
|
||
Mozilla build from 2001-03-16 (debug build). I get no segmentaion fault (that was fixed, wasn't it?) I _do_ get GTK is bailing on us (possibly because we pass it bad data). After running sizeToContent() three times (two is not enough) I get: Gdk-ERROR **: BadAlloc (insufficient resources for operation) serial 8530 error_code 11 request_code 53 minor_code 0 Gdk-ERROR **: BadDrawable (invalid Pixmap or Window parameter) serial 8536 error_code 9 request_code 66 minor_code 0 Program exited with code 01. status -> new, severity -> critical, ccing blizzard in case he has any insight.
Severity: minor → critical
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: crash
Summary: sizeToContent() seg faults on properly sized window → sizeToContent() crashes on properly sized window
Comment 2•23 years ago
|
||
Could someone create a testcase for this if it's still crashing? Reassigning to danm since he's the developer who works on this code, I think...
Assignee: jst → danm
Comment 3•23 years ago
|
||
Comment 4•23 years ago
|
||
Worksforme on windows95 mozilla0.8. Maybe blizzard's latest patch for Gdk events did some good to this?
Comment 5•23 years ago
|
||
still crashes for me in a morning build from cvs, 2001-03-20
Reporter | ||
Comment 6•23 years ago
|
||
This never crashed for me under windows, though the window changes size quite bizarrely (if that's a word). :) Still crashes for me under linux.
It won't crash on Windows because Windows is too smart to fall for it. What's happening is, sizeToContent is determining that the window should be 71582962 pixels square. Goofballs that we are, we dutifully try to make the window that big, and X falls down, hard. Yay X. Seems like, step one, we need a little error checking on the window sizing routine. But the main problem is the bad size. Eric? This one yours? I'm in DocumentViewerImpl::SizeToContent. After presShell->ResizeReflow, presContext-> GetVisibleArea returns the rect (0, 0, 0x40000000, 0x40000000). It happens on Windows, too, you just don't get a crash there. The test case basically goes <html><body> <a href="javascript:window.sizeToContent()">click three times to crash</a> </body></html>
Assignee: danm → evaughan
Comment 8•23 years ago
|
||
X uses 16 bit coordinates. Not much I can do about that and if I do then I'm lying to layout about my actual size. I'd rather see the bug fixed for real and don't try to resize to any unconstrained size, thanks.
Comment 9•23 years ago
|
||
I tested this one on 04-10-05 on Linux & i found really bizzare results. On first click it reduces window size to content size. On second click It maximizes window height back to original height, but it maximizes window width beyond screen limits. Its maximizes window width to infinity I guess because no matter how much you try to drag window to left side, you'll never see end.
Updated•23 years ago
|
Target Milestone: --- → mozilla0.9.1
Comment 10•23 years ago
|
||
This could be applicable for nsbeta1 because 1] Its a crash with visibility to user. [& crash is really bizzare.] 2] Method sizeToContent()'s main purpose is getting defeated when window is allready correct size. nominating for nsbeta1. Adding keywords mozilla0.9.1, nsbeta1.
Keywords: mozilla0.9.1,
nsbeta1
Comment 11•23 years ago
|
||
Hmm. Very few users call window.sizeToContent, even once. I need to see exact steps for users to reproduce this crash (in the current product or on a top100 webpage) without writing any code. Until then, ->0.9.2
Whiteboard: Need steps for user to repro in product
Target Milestone: mozilla0.9.1 → mozilla0.9.2
Comment 12•23 years ago
|
||
The bug this blocks involves calling sizeToContent over and over again on a dialog. That is not currently being done because sizeToContent crashes; therefore the dialog is mis-sized and hard to use. So steps to reproduce: 1) Write the new folder dialog the way it should be written if this bug were not present. 2) Try to add new folders on some types of IMAP servers.
Comment 13•23 years ago
|
||
Since it doesn't seem to crash in current product, moving ->0.9.3, limbo candidate.
Target Milestone: mozilla0.9.2 → mozilla0.9.3
Reporter | ||
Comment 14•23 years ago
|
||
This crashes all the time under 0.9!!! It's brutal! If we're going to be building apps on xul, this has to work no matter how many times in a row it may get called (like in a recursive function).
Comment 15•23 years ago
|
||
Using the testcasein attachment 27902 [details], and clicking where it says 3 times, I got
this stack dump, using a linux cvs build from today:
#0 0x406bff21 in nsWindow::Resize ()
from mozilla/dist/bin/components/libwidget_gtk.so
#1 0x4111ae2b in nsView::SetDimensions ()
from mozilla/dist/bin/components/libgkview.so
#2 0x411212d2 in nsViewManager::SetWindowDimensions ()
from mozilla/dist/bin/components/libgkview.so
#3 0x41123ab9 in nsViewManager::DispatchEvent ()
from mozilla/dist/bin/components/libgkview.so
#4 0x4111a1ad in HandleEvent ()
from mozilla/dist/bin/components/libgkview.so
#5 0x406ba32a in nsWidget::DispatchEvent ()
from mozilla/dist/bin/components/libwidget_gtk.so
#6 0x406ba255 in nsWidget::DispatchWindowEvent ()
from mozilla/dist/bin/components/libwidget_gtk.so
#7 0x406b92d9 in nsWidget::OnResize ()
from mozilla/dist/bin/components/libwidget_gtk.so
#8 0x406c0231 in nsWindow::Resize ()
from mozilla/dist/bin/components/libwidget_gtk.so
#9 0x4111ae2b in nsView::SetDimensions ()
from mozilla/dist/bin/components/libgkview.so
#10 0x411212d2 in nsViewManager::SetWindowDimensions ()
from mozilla/dist/bin/components/libgkview.so
#11 0x41123ab9 in nsViewManager::DispatchEvent ()
from mozilla/dist/bin/components/libgkview.so
#12 0x4111a1ad in HandleEvent ()
from mozilla/dist/bin/components/libgkview.so
#13 0x406ba32a in nsWidget::DispatchEvent ()
from mozilla/dist/bin/components/libwidget_gtk.so
#14 0x406ba255 in nsWidget::DispatchWindowEvent ()
from mozilla/dist/bin/components/libwidget_gtk.so
#15 0x406b92d9 in nsWidget::OnResize ()
from mozilla/dist/bin/components/libwidget_gtk.so
Repeats until I got bored at #5500 or so.
Reporter | ||
Comment 16•23 years ago
|
||
I am still seeing the same thing as Prashant Desale was seeing in April. If you create a page that resizes smaller upon the first click, and then back to original size on the second click, this bug appears. When the window is resized the first time it works, however, when you want to resize it back to it's original size, the window becomes (apparently) infinitely large.
Updated•23 years ago
|
Target Milestone: mozilla0.9.3 → mozilla0.9.4
Updated•23 years ago
|
Status: NEW → ASSIGNED
Whiteboard: Need steps for user to repro in product → have possible fix. Testing
Comment 17•23 years ago
|
||
Ok I have a fix for this one. It also removes one resize reflow from every window that opens so it might improve performance. Please test this patch to make sure it doesn't break anything.
Whiteboard: have possible fix. Testing → Have fix. Please test
Comment 18•23 years ago
|
||
Comment 19•23 years ago
|
||
need R and SR
Reporter | ||
Comment 20•23 years ago
|
||
Is this fix in the latest build? I'll gladly test it once it's in a build.
Comment 21•23 years ago
|
||
Set milestone to mozilla0.9.5 added nsbranch keyword
Keywords: nsbranch
Target Milestone: mozilla0.9.4 → mozilla0.9.5
Comment 22•23 years ago
|
||
sr=blizzard
Comment 23•23 years ago
|
||
*** Bug 98100 has been marked as a duplicate of this bug. ***
Comment 24•23 years ago
|
||
OK. I tried applying that last patch. On startup I get: Error reading file /home/bzbarsky/mozilla/debug/mozilla/dist/bin/chrome/skins/classic/linkToolbarOverlay.css Error reading file jar:resource:///chrome/comm.jar!/content/navigator/LinkToolbarHandler.js WARNING: CSSLoaderImpl::DidLoadStyle: Load of URL 'chrome://linktoolbar/skin/linkToolbarOverlay.css' failed. Error code: 18, file nsCSSLoader.cpp, line 933 Failed to load jar:resource:///chrome/comm.jar!/content/navigator/LinkToolbarHandler.js Error reading file jar:resource:///chrome/comm.jar!/content/navigator/LinkToolbarItem.js Failed to load jar:resource:///chrome/comm.jar!/content/navigator/LinkToolbarItem.js Error reading file jar:resource:///chrome/comm.jar!/content/navigator/LanguageDictionary.js Failed to load jar:resource:///chrome/comm.jar!/content/navigator/LanguageDictionary.js After which, of course, the toolbar does not work. I'm not sure why I'm getting those error messages; the paths looks fine to me.... This is a linux debug build from 2001-09-10
Comment 25•23 years ago
|
||
> OK. I tried applying that last patch. On startup I get:
Try unapplying the patch and see if you get them same errors. I think you just
have a bad build.
Comment 26•23 years ago
|
||
*** Bug 96401 has been marked as a duplicate of this bug. ***
Updated•23 years ago
|
Whiteboard: Have fix. Please test → Have fix. Please test, critical for 0.9.4
Comment 27•23 years ago
|
||
This patch seems to fix problems with XUL dialogs in embedding being resized to enormous sizes.
Assignee | ||
Comment 28•23 years ago
|
||
I think this patch will break printing: I'm pretty sure that we do a resize reflow at the same w&h to force form controls to sync up properly. See nsDocumentViewer.cpp:2917.
Assignee | ||
Comment 29•23 years ago
|
||
sizeToContent() calls ResizeReflow() with an unbounded width & height. It relies on SyncFrameViewAfterReflow() causing a recursive resize-reflow at the correct width/height to get the visible area properly updated in the pres context. nsView will stifle window resizing when the width and height do not change, so the unfortunate call to ResizeReflow() that ends up being non-recursive leaves the display area unconstrained. Now it turns out (on Linux at least) that the first size-to-content ends up being slightly different (off by one pixel) than the second size-to-content. So the first two end up recurring through ResizeReflow(), and the third one doesn't. I think that the right way to fix this is to copy the pattern from InitialReflow() and ensure that we reset the visible area to the desired size that resulted from this activation of ResizeReflow().
Assignee | ||
Comment 30•23 years ago
|
||
Assignee | ||
Comment 31•23 years ago
|
||
Comment on attachment 48950 [details] [diff] [review] be sure to SetVisibleArea() after getting the desiredSize so failure to recurse when syncing view doesn't leave visible area horked Oops! Don't look at this!
Attachment #48950 -
Attachment is obsolete: true
Assignee | ||
Comment 32•23 years ago
|
||
Assignee | ||
Comment 33•23 years ago
|
||
Taking.
Assignee: evaughan → waterson
Status: ASSIGNED → NEW
Target Milestone: mozilla0.9.5 → mozilla0.9.4
r=dbaron. (InitialReflow does it, and SetVisibleArea isn't used for anything too scary -- like anything the view system does, which is more like what I expected from the name.)
Comment 35•23 years ago
|
||
Anyone, dbaron in particular: is the off-by-one pixel "jitter" between the first and second rezize-reflow worth filing as a bug? I know little about how such errors arise, but if it's just due to twips-to-pixels and back math, there are surely fixes. /be
Comment 36•23 years ago
|
||
Chris, your last patch doesn't fix the embedding crash that I'm seeing. I'm stell getting resize requests that are way off the scale: *** size_to_cb 361 214 *** size_to_cb 76695849 76695848 load_finished_cb *** size_to_cb 76695849 76695848 visibility_cb 1 ( of course, the fact that I'm getting three resize requests isn't great either. )
Comment 37•23 years ago
|
||
Oh, here's the URL I'm using as a test case: https://www.theneals.net/irc/irc.cgi
Assignee | ||
Comment 38•23 years ago
|
||
Using what harness? This seems to work in gtkEmbed.
Yeah, it's probably worth filing a bug on the off-by-one (and attaching a testcase!).
Assignee | ||
Comment 40•23 years ago
|
||
Ok, TestGtkEmbed seems to be fixed with this patch on the trunk; blizzard says that he's still seeing the problem on the 0.9.4 branch. I'll investigate there...
No longer blocks: 99225
Status: NEW → ASSIGNED
Assignee | ||
Comment 41•23 years ago
|
||
blizzard, I just tried the patch on the mozilla-0.9.4 branch, and it seems to fix the problem for me. I ran: LD_LIBRARY_PATH=`pwd` ./TestGtkEmbed https://www.theneals.net/irc/irc.cgi Without the patch, I crash the app, and once wedged the X server so bad I had to reboot. With the patch, things appear to work properly.
Comment 42•23 years ago
|
||
OK, the patch is working here on the 0.9.4 branch. I must have been on crack or something.
Comment 43•23 years ago
|
||
Chris, can I check this into the 0.9.4 branch?
Comment 44•23 years ago
|
||
sr=attinasi
Assignee | ||
Comment 45•23 years ago
|
||
Fix checked in on trunk.
Whiteboard: Have fix. Please test, critical for 0.9.4 → fixed-on-trunk, critical for 0.9.4
Assignee | ||
Comment 46•23 years ago
|
||
Checked in on mozilla-0.9.4 branch, taking blizzard's request as a=.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 47•23 years ago
|
||
what is the complete patch to this bug?
Assignee | ||
Comment 48•23 years ago
|
||
I believe that attachment 48951 [details] [diff] [review] should be a complete fix.
Reporter | ||
Comment 49•23 years ago
|
||
really? a one liner? almost seems too easy. :)
Comment 50•23 years ago
|
||
Verified on 2001-09-21-04-0.9.4 branch. Needs verification on trunk. Adding keyword vtrunk.
Keywords: vtrunk
Comment 51•23 years ago
|
||
Verified on trunk 2001092011.
Comment 52•23 years ago
|
||
verified on trunk too according to Philip Langdale. Marking verified.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•