bugzilla.mozilla.org will be intermittently unavailable on Saturday, March 24th, from 16:00 until 20:00 UTC.

Blank charset list in SaveAs charset of HTML composer

VERIFIED FIXED in mozilla0.9.6



17 years ago
17 years ago


(Reporter: nhottanscp, Assigned: jbetak@netscape.com (away - not reading bugmail))



Windows 2000

Firefox Tracking Flags

(Not tracked)




(2 attachments, 1 obsolete attachment)



17 years ago
The current trunk build, charset list is empty for SaveAs charset.
I see that also in 0.9.5.
Juraj, do you know anything about this? When is the last time you saw it's 


17 years ago
Keywords: regression

Comment 1

17 years ago
Created attachment 54931 [details]
Screen shot, empty charset list.
accepting as regression - will investigate what caused it. Targeting M096 for 
now, since this is quite urgent.
Target Milestone: --- → mozilla0.9.6

Comment 3

17 years ago
Today mailnews crashed when i clicked on mails.
I then discovered that the charset for the folder was empty.
Setting it to western, iso8859-1 cured the crash.

1: shouldn't crash on missing charset
2: i specifically set the charset ti iso8859-1 less than a week ago when i
discovered it for some unexplainable reason was set to arabic (which seems to be
some default for all mailnews folders also??)

Linux 2001102408

Comment 4

17 years ago
Changed QA contact to ylong@netscape.com.
QA Contact: teruko → ylong

Comment 5

17 years ago
It's broken using 9/21 trunk build. I can't find earlier daily builds but 
mozilla 0.9.4 does not have the problem.
Here is a list of check in between 9/4 (0.9.4 freeze date) and 9/21.

Checkins to directory mozilla/editor between 09/04/2001 00:00 and 09/21/2001 
11:00 : 


Comment 6

17 years ago
In EditorSaveAsCharset.js, I dumped charsetNode.Value and it has a right value.
I also see the string passed to AppendStringToTreelist() looks fine, and
AppendStringToTreelist() seems to be working fine because it is also used by
link property dialog and I see it's listing the links witout problem.
So no idea where it's broken. Juraj, any idea?
What about line 139, item.firstChild.firstChild.setAttribut?
What is this line supposed to do?


135     for (var i = 0; i < rdfContainer.GetCount(); i++) 
136         {
137       charsetNode =
138       item = AppendStringToTreelist(gDialog.charsetTree,
readRDFString(rdfDataSource, charsetNode, kNC_name));
139       item.firstChild.firstChild.setAttribute("value", charsetNode.Value);
140       if(charset == charsetNode.Value) 
141         selectedItem = item;
142     }

item.firstChild.firstChild.setAttribute sets the charset "ID" which as oppesed 
to the pretty name which is inserted using AppendStringToTreelist().

I tried a few things and it looks to me that the XUL syntax has changed and 
some little detail about the tree declaration in the XUL file and/or data 
population in JS is killing us. I'm still looking into it - what a pain! It's 
about time they freeze XUL once and for all ;-)
looks like an XPToolkit problem. Removing ensureElementIsVisible() will at 
least render the tree, although the scrollbar will be missing and there will 
also be a repaint problem. I'll try to figure out, what to change (or add) in 
the XUL tree definition to avoid this problem.


Comment 9

17 years ago
*** Bug 107975 has been marked as a duplicate of this bug. ***

Comment 11

17 years ago
Juraj, does the patch fix the problem? If so, please get reveiews and check in
for 0.9.6.
Naoki could you r= the patch? It fixes the problem, but the item will not be 
scrolled to. Seems like an XPToolkit issue to me. The patch should be good 
enough for M096...

could you please r= this change?

Comment 14

17 years ago
Comment on attachment 56219 [details] [diff] [review]
proposed patch v1

So we're not supporting the "rows" parameter for trees any more? 
I just want to be sure this fix isn't hiding a different problem in XUL.

Comment 15

17 years ago
Ok, I see from other comments that this is a "temporary" fix, right? Let's be sure
the appropriate toolkit people are aware of such problems.
if this type of fix is ok with XPFE folks.
thanks cmanske, I agree that this is most likely a regression in XPFE Toolkit 
and will file a follow-up bug against them.

Comment 17

17 years ago
Okay, I tracked this down to the window from 09/18/2001 08:00:00 to
09/21/2001 08:00:00. I didn't have builds for a narrower window. 
(Warning, if you do that bonsai query, be aware that you will pick 
up the great whack-the-license-then-back-it-out checkin, so be prepared
to wait a while :-]).

I could see (using the world's greatest tool, the DOM Inspector) that 
the treecells all had zero width/height.

Anyways, after reviewing the list, I backed out 
 cvs update -j1.78 -j1.77 content/shared/public/nsXULAtomList.h
and then 
 cvs update -j1.35 -j1.34 layout/xul/base/src/nsBox.cpp
both of which had to do with checking for values of (min)height/width.

After the second backout, this dialog was fixed without the band-aid fix
already attached above (and the first wasn't likely required, but I didn't
want to rebuild content again).

I propose that you check in the band-aid for the 0.9.6 _branch_, but that
we don't put it on the trunk, since this seems like a basic bug in box 
layout and probably should be fixed. Sound like a plan?
jrgm - thanks for the great work! Judging by the nature of this problem it 
seems quite reasonable to assume that this is related to hyatt's checkin (Make 
XUL boxes check maxheight/maxwidth attrs as well as minwidth. r/sr=hewitt,jst). 
Should we file a new bug for the trunk fix then?

I'll try to get a sr= and a= for the M096 branch since this is an i18n blocker. 
Chris could you help us out one more time with a sr=?
Comment on attachment 56219 [details] [diff] [review]
proposed patch v1

M096 branch only
Attachment #56219 - Flags: review+

Comment 20

17 years ago
Wait, let's fix this the right way.  I accidentally removed the 

if (val > etc.)
  blah blah

checks from nsBox.cpp's checking of minheight and minwidth.  Restoring those
checks should fix the problem.  I didn't actually mean to remove them.
Created attachment 56836 [details] [diff] [review]
patch v2 - based on hyatt's comments it adds two lines which have been accidentaly removed
thanks Dave - that really helped.

BTW what are the chances to get a r=/sr= before the milestone deadline wheezes 
by? I realize this is an 11th hour effort, yet I have to give it try :-)
Attachment #56219 - Attachment is obsolete: true

Comment 23

17 years ago
Fix the indentation (the lines under the if statements should be indented 2
spaces).  sr=hyatt
ugh, the -w -b diff options are nothing but trouble ;-) Well, it's fixed at 
last, so thanks everyone!

Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 25

17 years ago
Fixed verified on 11-07 trunk build.
You need to log in before you can comment on or make changes to this bug.