sizeToContent summons new NS_ERROR_FAILURE exceptions

RESOLVED FIXED in mozilla1.9.1b3

Status

()

defect
P2
normal
RESOLVED FIXED
11 years ago
3 years ago

People

(Reporter: erappleman, Assigned: roc)

Tracking

(Blocks 2 bugs, {fixed1.9.1})

Trunk
mozilla1.9.1b3
x86
All
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

(Reporter)

Description

11 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.4) Gecko/2008111616 Ubuntu/9.04 (jaunty) Firefox/3.0.4
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.4) Gecko/2008111616 Ubuntu/9.04 (jaunty) Firefox/3.0.4

The bug manifests itself differently across platforms and Firefox versions.

For example, almost all sizeToContent calls in Firefox 3.1 (all platforms) are not handled properly and cause rendering and/or redraw errors.

Examples:
https://bugzilla.mozilla.org/attachment.cgi?id=276818
https://bugzilla.mozilla.org/attachment.cgi?id=304326
https://bugzilla.mozilla.org/attachment.cgi?id=276997

However, when using a non-default theme (such as ifox graphite) on Firefox 3.0.x (all platforms EXCEPT Linux are affected), a sizeToContent call for a new nsIDOMWindow will create a dimensionless, 0 by 0 window that cannot be resized or closed.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.
(Reporter)

Updated

11 years ago
OS: Linux → All
(Reporter)

Comment 1

11 years ago
Also, I hate to be a bother, but until I can find or make a testcase for the latter set of symptoms, here's the steps to replicate on a non-Linux installation of 3.0.x

1. Install the Text-to-Image addon.
2. Install and select iFox Graphite theme.
3. Restart Firefox.
4. Find an image URL: http://www.mozilla.com/img/tignish/home/feature-logo.png
5. Enable Text-to-Image by clicking once on the red hurricane icon in the status bar.
6. Double click on the image that has now appeared.
7. Observe the invisible, persistent "window" that results.

Note:

In Firefox 3.1 on ALL platforms, regardless of theme, the end result will be either a zero-dimension popup that can't be closed OR a popup that is only as tall as the window bar (observed on Linux).
(Reporter)

Comment 2

11 years ago
(btw, before anyone complains, i am the author of text-to-image)
(Reporter)

Updated

11 years ago
Flags: blocking-firefox3.1?
OS: All → Linux
Version: unspecified → 3.1 Branch
(Reporter)

Updated

11 years ago
OS: Linux → All
(Reporter)

Comment 3

11 years ago
I just realized I forgot the browser string for 3.1

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b2) Gecko/20081128 Ubuntu/9.04 (jaunty) Shiretoko/3.1b2 ID:20081128200629
(Reporter)

Comment 4

11 years ago
Breakpoint 1, DocumentViewerImpl::SizeToContent (this=0xac0bf6a0) at nsDocumentViewer.cpp:3137
3137 nsDocumentViewer.cpp: No such file or directory.
in nsDocumentViewer.cpp
Current language: auto; currently c++

Run till exit from #0 DocumentViewerImpl::SizeToContent (this=0xac0bf6a0) at nsDocumentViewer.cpp:3137
0xb704eb88 in nsGlobalWindow::SizeToContent (this=0xaafee330) at nsGlobalWindow.cpp:4546
4546 nsGlobalWindow.cpp: No such file or directory.
in nsGlobalWindow.cpp
Value returned is $1 = 2147500037
(Reporter)

Comment 5

11 years ago
I'm splitting this bug into 2 as per a request.
--> Core::DOM
Component: General → DOM
Flags: blocking-firefox3.1?
Product: Firefox → Core
QA Contact: general → general
Version: 3.1 Branch → Trunk
Component: DOM → Layout
Flags: blocking1.9.1?
QA Contact: general → layout
(In reply to comment #5)
> I'm splitting this bug into 2 as per a request.

So what does this bug cover, and what's the other bug number?
Assignee: nobody → roc
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2
The bug only seems to happen when you load one of those testcases as a content window. If you load them as top-level chrome windows, they work fine.

The bug seems to be that after one call to resizeToContent, in nsXULWindow::SizeShellTo shellAsWin->GetSize returns the wrong (old) size but this->GetSize returns the right size. For example if the sizeToContent-width is 50px and the window started at 400px, we get width==400, aCX=-50, winCX==50 so widthDelta==-350 and we call SetSize with a width of -300. Then all kinds of bad stuff happens.

But this could just be because nsDocumentViewer::SizeToContent isn't expecting a non-toplevel documents. The code looks like this:

   nsCOMPtr<nsIDocShellTreeItem> docShellParent;
   docShellAsItem->GetSameTypeParent(getter_AddRefs(docShellParent));

   // It's only valid to access this from a top frame.  Doesn't work from
   // sub-frames.
   NS_ENSURE_TRUE(!docShellParent, NS_ERROR_FAILURE);

Here the content document is the topmost content document so docShellParent is null. Should we be checking for *any* parent? Do we actually need sizeToContent to work in content subframes?
I'd say that sizeToContent in a non-chrome window should just be a no-op (just like all other window resizing methods, honestly).  I'd have no problems with making it so, personally.
This seems to have regressed between Firefox 2 and Firefox 3.

We do allow window resizing methods from content, by default. nsGlobalWindow::SizeToContent checks CanMoveResizeWindows so that can be disabled. I think we should probably fix this so it works.
OK, so the *real* reason this doesn't work is aptly described by this comment in SizeShellTo:

  // XXXTAB This is wrong, we should actually reflow based on the passed in
  // shell.  For now we are hacking and doing delta sizing.  This is bad
  // because it assumes all size we add will go to the shell which probably
  // won't happen.

It turns out that the toplevel window refuses to shrink its contents below a width of about 400px on my Mac. So here's what happens:
-- In the first sizeToContent call, we start with a 600px wide content area and window width and try to narrow the content area to 50px of size-to-content width. nsXULWindow::SizeShellTo computes a widthDelta of -550px and sets the window width to 50px.
-- However, because the toplevel window refuses to shrink below 400px, the content area gets a width of 400px.
-- In the next sizeToContent call, we start with a 400px wide content area and a 600px wide window and try to narrow the content area to 50px. So we compute a widthDelta of -350px and try to set the window width to -300px. This is of course a case where changing the window width does not flow through directly to the content area width.

So there are two bugs here:
1) A bug (that we've hit before) in the Firefox chrome where it refuses to let the content area shrink below some (largish) minimum width
2) As a fallback, nsXULWindow::SizeShellTo should at least not try to make the window smaller than the content area it's trying to achieve. Ideally it would compute the actual window size required assuming the content area has the fixed size-to-content size, but that does seem like more work than it's worth.
Posted patch fixSplinter Review
This seems worth doing, at least since there's a test here.
Attachment #354672 - Flags: superreview?(bzbarsky)
Attachment #354672 - Flags: review?(bzbarsky)
Whiteboard: [needs review]
Blocks: 392417
Blocks: 439323
Comment on attachment 354672 [details] [diff] [review]
fix

r+sr=bzbarsky with s/try/trying/ and a followup bug filed to sort out the seamonkey/windows stuff.
Attachment #354672 - Flags: superreview?(bzbarsky)
Attachment #354672 - Flags: superreview+
Attachment #354672 - Flags: review?(bzbarsky)
Attachment #354672 - Flags: review+
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 14

11 years ago
So, is my follow up bug (https://bugzilla.mozilla.org/show_bug.cgi?id=467853) redundant?

It has more test cases than this one.

I'll repost them here for your convenience.

https://bugzilla.mozilla.org/attachment.cgi?id=276818
https://bugzilla.mozilla.org/attachment.cgi?id=304326
https://bugzilla.mozilla.org/attachment.cgi?id=276997
https://bugzilla.mozilla.org/attachment.cgi?id=349968 (this one causes complete destruction of the firefox window if you fiddle with the buttons)
(Reporter)

Comment 15

11 years ago
(my mistake, the 4th testcase was the only new one.)
(Reporter)

Comment 16

11 years ago
one last thing, how can i check for when this fix makes it into a nightly or hourly?
Whiteboard: [needs review] → [needs landing]
Pushed 4e1a2c3bc40d.

The fix for this will be available in the next nightly build.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [needs landing] → [needs 191 landing]
Pushed 0de153e2246f to 1.9.1
Keywords: fixed1.9.1
Whiteboard: [needs 191 landing]
(In reply to comment #18)

Well, whilst SeaMonkey does (still) have bug 471959,

the test added by this bug does *not* fail, at least on 1.9.1:
{
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1231467839.1231473189.13721.gz
Linux comm-central dep unit test on 2009/01/08 18:23:59
*** 42442 ERROR TEST-UNEXPECTED-PASS | /tests/layout/base/tests/test_bug465448.xul | innerWidth
*** 42443 ERROR TEST-UNEXPECTED-PASS | /tests/layout/base/tests/test_bug465448.xul | innerHeight
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1231469596.1231476862.21343.gz
MacOSX 10.4 comm-central dep unit test on 2009/01/08 18:53:16
*** 42460 ERROR TEST-UNEXPECTED-PASS | /tests/layout/base/tests/test_bug465448.xul | innerWidth
*** 42461 ERROR TEST-UNEXPECTED-PASS | /tests/layout/base/tests/test_bug465448.xul | innerHeight
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1231469281.1231475027.17708.gz
Win2k3 comm-central dep unit test on 2009/01/08 18:48:01
*** 42466 ERROR TEST-UNEXPECTED-PASS | /tests/layout/base/tests/test_bug465448.xul | innerHeight

http://hg.mozilla.org/releases/mozilla-1.9.1/pushloghtml?fromchange=0ce3af5127dc&tochange=0c1f894cd595
}

Could you remove the SeaMonkey workaround ?
Target Milestone: --- → mozilla1.9.1b3

Updated

11 years ago
Blocks: 472813
(In reply to comment #19)
> Could you remove the SeaMonkey workaround ?

KaiRo filed bug 472813...
You need to log in before you can comment on or make changes to this bug.