Closed Bug 368501 Opened 17 years ago Closed 17 years ago

Table Properties Windows Cut Off


(Core :: XUL, defect)

Not set





(Reporter: cbook, Assigned: smaug)



(Keywords: regression, verified1.8.0.12, verified1.8.1.4, Whiteboard: wait for more trunk baking -- test menus)


(3 files, 5 obsolete files)

Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv: Gecko/20070125 Thunderbird/2.0b2pre Mnenhy/ ID:2007012804

1. Create a new Message
2. Insert a Table
3. Double Click in the table to reach the table properties
4. The Window is cut off

Under Solaris Thunderbird 2 Beta 2 i can change the size of this window, under the Windows Build its not possible to change the Size of the Table Properties Window.
Under Windows (XP SP2) bug can be fixed by:
<Thunderbird program folder>\chrome\comm.jar\content\editor\EdTableProps.js
add string to Startup() function:
string 289:  SetWindowLocation();
so it's now: 
string 289:  SetWindowLocation();
string 290: window.sizeToContent();
Though under Linux (Slackware 10.1) it does not help.

PS. Got same trouble with:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20061207 Thunderbird/ ID: 2006120700
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv: Gecko/20070116 Thunderbird/2.0b2 ID:2007011615
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/20070128 Thunderbird/3.0a1 ID:2007012803
changing because comment #1
Version: 2.0 → Trunk
Changed screen resolution in Linux to 1600x1200 and Table Properties window was not cut off anymore.
Changed back to 1024x768 - the window is cut off again.

Linux Version: Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20070116 Thunderbird/2.0b2 ID:2007011615

Happens on Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20070111 SeaMonkey/1.1 -- not cut off while loading, becomes cut off when loaded completely.

WFM on Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a2pre) Gecko/20070128 SeaMonkey/1.5a
As Merlyel pointed out in #maildev on IRC, the table properties window fills some of its menulists in its onload handler. However, they don't reflow until after the window has loaded, so that the intrinsic size is miscomputed.
Don't we run sizeToContent() _after_ the onload handler?  Or is the problem that the events from bug 348304 are still outstanding?

If so, it sounds like what we need is a way for a flush of some sort to process said events...
Blocks: 348304
Flags: blocking-thunderbird2?
And this sounds like a core XUL bug to me.
Assignee: mscott → nobody
Component: Message Compose Window → XP Toolkit/Widgets: XUL
Flags: blocking-thunderbird2?
Product: Thunderbird → Core
QA Contact: message-compose → xptoolkit.xul
Severity: minor → normal
Component: XP Toolkit/Widgets: XUL → XP Toolkit/Widgets: Menus
Flags: in-testsuite?
Flags: blocking1.9?
Flags: blocking1.8.1.3?
Keywords: regression
QA Contact: xptoolkit.xul → xptoolkit.menus
Flags: blocking1.8.0.11?
Attached file This testcase shows the issue (obsolete) —
The menulist isn't being sized properly, as it has no child menupopup, and SizeToPopup returns early before firing the event. Adding an empty child <menupopup> inside the menulist in this testcase makes the sizing work properly.
Attachment #256677 - Attachment is patch: false
Attachment #256677 - Attachment mime type: text/plain → application/vnd.mozilla.xul+xml
Comment on attachment 256677 [details]
This testcase shows the issue

this is actually different bug, I think, because this happens also on
Attachment #256677 - Attachment is obsolete: true
Instead of using nsIRunnable it is better to take advantage of the 
existing nsIReflowCallback object posting.
nsIReflowCallbacks will be handled right after reflow.
Tested the regressions bug 348304 caused, and couldn't see any
problems, nor did I see any crashes using the crash testcases.
When using nsIReflowCallback, nsASyncMenuGeneration may not need to
block onload, but I left it there just to reduce the possibility for
new regressions. ((Un)BlockOnload could be removed from trunk
Attachment #257152 - Flags: review?(bzbarsky)
Attached patch for 1.8 (obsolete) — Splinter Review
Attachment #257162 - Attachment is patch: true
Attachment #257162 - Attachment mime type: text/x-patch → text/plain
Comment on attachment 257152 [details] [diff] [review]
possible patch, make async a bit less async

>Index: layout/xul/base/src/nsMenuFrame.cpp
>+  nsCOMPtr<nsIReflowCallback> cb = new nsASyncMenuInitialization(this);
>+  GetPresContext()->PresShell()->PostReflowCallback(cb);

This needs to handle out-of-memory.  Same for the other two callsites.

I'd like roc or dbaron to also look this over... but in general it seems ok.

r=bzbarsky with the OOM issue addressed.  Thanks for fixing this!
Attachment #257152 - Flags: review?(bzbarsky) → review+
Attachment #257152 - Flags: superreview?(roc)
Attached patch with OOM checks (obsolete) — Splinter Review
Assignee: nobody → Olli.Pettay
Attachment #257152 - Attachment is obsolete: true
Attachment #257162 - Attachment is obsolete: true
Attachment #257269 - Flags: superreview?(roc)
Attachment #257152 - Flags: superreview?(roc)
Attached patch for 1.8 (obsolete) — Splinter Review
You're not setting aFlushFlag, but you should.

I'm about to check in a patch that makes nsIReflowCallback a non-COM interface.
Roc, I decided to use the |delete this| you suggested. A bit ugly
but should work.
Otherwise I should have added virtual destructor to nsIReflowCallback
and add PresShell-owns logic to Post/HandleReflowCallback etc.
Attachment #257345 - Flags: superreview?(roc)
Attachment #257269 - Flags: superreview?(roc)
Attachment #257269 - Attachment is obsolete: true
This should be now ok for branch.
Attachment #257270 - Attachment is obsolete: true
Comment on attachment 257345 [details] [diff] [review]
for trunk, using deComified reflowcallback

OK. You should add to the comment for PostReflowCallback in nsIPresShell.h that we guarantee that these callbacks will be called, even if reflow fails. And can you please check that's true? :-)
Attachment #257345 - Flags: superreview?(roc)
Attachment #257345 - Flags: superreview+
Attachment #257345 - Flags: review+
Attachment #257347 - Flags: approval1.8.1.4?
Closed: 17 years ago
Resolution: --- → FIXED
Do we need a separate 1.8.0-branch patch?
Flags: blocking1.8.1.4?
Flags: blocking1.8.1.4+
Flags: blocking1.8.0.12?
Flags: blocking1.8.0.12+
1.8 patch works with 1.8.0 branch too
Attachment #257347 - Flags: approval1.8.0.12?
Whiteboard: wait for more trunk baking -- test menus
Comment on attachment 257347 [details] [diff] [review]
For 1.8, setting aFlushFlag

approved for and, a=dveditz for drivers
Attachment #257347 - Flags: approval1.8.1.4?
Attachment #257347 - Flags: approval1.8.1.4+
Attachment #257347 - Flags: approval1.8.0.12?
Attachment #257347 - Flags: approval1.8.0.12+
Flags: blocking1.9?
Depends on: 376155
verified fixed using my steps to reproduce from this bug with Build
Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv: Gecko/20070427 Thunderbird/ Mnenhy/ ID:2007043003

and verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv: Gecko/20070430 Thunderbird/ ID:2007043008

Table Properties are looking good now and the window is not cut off. Adding verified keyword.

The 1.8 patch for this bug seems to cause bug 383120. I don't know enough of firefox's internals to understand how it can cause this bug, but removing it from current cvs, solves the issue. Can someone knowledgeable look over this?
Depends on: 383120
Component: XP Toolkit/Widgets: Menus → XUL
QA Contact: xptoolkit.menus → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.