Closed Bug 76350 Opened 23 years ago Closed 23 years ago

dialogs come up in mail/news too small

Categories

(Core :: XUL, defect)

x86
Linux
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla0.9

People

(Reporter: blizzard, Assigned: danm.moz)

References

Details

Build is from the morning of Apr 17.

For some dialogs in mail/news including the password dialog for my imap account
and the dialog for an SSL warning come up too small to include all of the
content.  In the case of the password dialog it means that the right side of the
text area and text are cut off.  In the case of the SSL warning dialog none of
the buttons are visible so I can never accept the warning to read my mail.

Danm and I think that this is because the OnLoad event is firing too early,
before all of the content has been loaded so the sizeToContent() doesn't work
correctly.

I filed this against XP Toolkit/Widgets since I'm not sure exactly where the
problem is.
Severity: normal → major
This occurs when visiting an https website (http://verisign.com/)- no obvious or
visible way to close the psm2 dialog that pops up.
->danm
Assignee: trudelle → danm
targetting 0.9, cc self
Whiteboard: wanted for 0.9
Target Milestone: --- → mozilla0.9
Thanks for that lovely spam.

Now for some news. We're pretty sure we know what's causing this. It is an early 
"document loaded" message, kind of. The problem is, the alert dialogs' XUL 
mentions no image and no size for the little icon in the upper left. It's in the 
onload handler that some JS triggers a CSS rule that loads an image for the 
first time. So it's no surprise that intrinsic sizing, which happens a little 
later in the onload handler, treats the icon as sizeless.

This patch, for instance, fixes the problem by triggering an image load along 
with the rest of the document:

Index: commonDialog.xul
===================================================================
RCS file: /cvsroot/mozilla/xpfe/global/resources/content/commonDialog.xul,v
retrieving revision 1.32
diff -u -r1.32 commonDialog.xul
--- commonDialog.xul	2001/04/12 05:41:56	1.32
+++ commonDialog.xul	2001/04/24 04:01:55
@@ -17,7 +17,7 @@
 
   <box flex="1">
     <box autostretch="never" valign="top">
-      <image id="info.icon" class="spaced"/>
+      <image id="info.icon" class="spaced question-icon"/>
     </box>
     <separator orient="vertical" class="thin"/>
     <box orient="vertical" flex="1">

and this patch fixes it by wiring in the image sizes

Index: themes/modern/global/global.css
===================================================================
RCS file: /cvsroot/mozilla/themes/modern/global/global.css,v
retrieving revision 1.31
diff -u -r1.31 global.css
--- global.css	2001/04/12 05:41:54	1.31
+++ global.css	2001/04/24 04:06:08
@@ -86,18 +86,22 @@
 
 .message-icon {
   list-style-image: url("chrome://global/skin/icons/alert-message.gif");
+  width:32px;height:32px;
 }
 
 .alert-icon {
   list-style-image: url("chrome://global/skin/icons/alert-exclam.gif");
+  width:32px;height:32px;
 }
 
 .error-icon {
   list-style-image: url("chrome://global/skin/icons/alert-error.gif");
+  width:32px;height:32px;
 }
 
 .question-icon {
   list-style-image: url("chrome://global/skin/icons/alert-question.gif");
+  width:32px;height:32px;
 }
 
 /* ::::: status bar ::::: */

(with similar changes to classic and other skins).

Neither patch is especially good. But they're both quite safe. I'm thinking of 
going with the second version after the 0.9 branch is cut, only on the branch, 
and searching for a more general solution for the trunk.

But on the other hand, how unexpected is it that a dialog that wants to change 
its size in its onload handler doesn't size to content very well? I mean, is 
that supposed to work? More thinking is wanted.
Wow, are you saying that we actually load the image from the OnLoad handler
instead of as part of the XUL?  I kind of think that we need to watch for size
changes in dialogs and always call sizeToContent anytime there is a size change.
 Is that possible?

In any case I suspect that your fix is probably the safest thing to do for the
0.9 branch.
Looks safe to me r=ksosez
Whiteboard: wanted for 0.9 → wanted for 0.9 - have fix, r=, need sr= and a=
Whiteboard: wanted for 0.9 - have fix, r=, need sr= and a= → wanted for 0.9 - have fix, r=, need sr= and a=, take hack on the branch
  I've been toying with the idea of resizing the window as new content dribbles 
in. Something like sending a special kind of onload event (not an onload event 
per se, of course) to the associated window every time a loadgroup winds down, 
and using it to resize windows that want to be resized. But this is not a great 
plan. At best you could expect the window to change its size after it's visible. 
Unless I caught the situation early on and suppressed visibility...
  Anyway, even Hyatt, who rips into stuff like this casually, tells me this is 
unfertile ground. Better to just realize that kicking off additional loads in an 
onload handler is a thing of consequence. If you write a window that does that, 
you need to deal with the repercussions.
  So along those lines, wiring the image sizes into the skin is a perfectly fine 
way to deal with the repercussions. I'm planning on going with the second patch 
as the real fix. (Note it's already been approved for the 0.9 branch).
Status: NEW → ASSIGNED
Whiteboard: wanted for 0.9 - have fix, r=, need sr= and a=, take hack on the branch → wanted for 0.9 - have fix
So why is it that we can't fix it so that the images are loaded as part of the
description for the dialog instead of from the OnLoad() handler?
Shouldn't chrome:// images load syncronyously and therefor be loaded when onLoad
fires?
  Christopher: that's what my first patch does. More or less. It would need a bit 
more code added, to replace the image loaded by default with the correct one. But 
it's ineffcient, loading a second icon, and alert windows in Mozilla are already 
too slow. I'd take the hardwired size approach over that.
  But I've filed a new bug 77755 against the XP Apps folks to consider designing 
commonDialogs to not have this problem.

  Jonas: Generally, yes. But see my comments above explaining why commonDialogs 
don't work that way.

  Hardwiring patch is checked in.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Whiteboard: wanted for 0.9 - have fix
Blocks: 76021
*** Bug 68352 has been marked as a duplicate of this bug. ***
Peter, I removed myself from the cc list, please read my reasoning in bug 68352.
*** Bug 65836 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.