Closed Bug 106536 Opened 23 years ago Closed 23 years ago

Blank charset list in SaveAs charset of HTML composer

Categories

(Core :: Internationalization, defect)

x86
Windows 2000
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla0.9.6

People

(Reporter: nhottanscp, Assigned: jbetak)

References

()

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

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 
working?
Keywords: regression
accepting as regression - will investigate what caused it. Targeting M096 for 
now, since this is quite urgent.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.6
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
Changed QA contact to ylong@netscape.com.
QA Contact: teruko → ylong
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 : 

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&bra
nchtype=match&dir=mozilla%2Feditor&file=&filetype=match&who=&whotype=match&sortb
y=Date&hours=2&date=explicit&mindate=09%2F04%2F2001+00%3A00%3A00&maxdate=09%2F21
%2F2001+11%3A00%3A00&cvsroot=%2Fcvsroot
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?


http://lxr.mozilla.org/mozilla/source/editor/ui/dialogs/content/EditorSaveAsCharset.js#135

135     for (var i = 0; i < rdfContainer.GetCount(); i++) 
136         {
137       charsetNode =
availableCharsets.getNext().QueryInterface(Components.interfaces.nsIRDFResource);
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     }
Naoki,

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.

http://lxr.mozilla.org/mozilla/source/editor/ui/dialogs/content/EditorSaveAsChar
set.js#147
*** Bug 107975 has been marked as a duplicate of this bug. ***
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...
cmanske: 

could you please r= this change?
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.
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.
r=cmanske
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.
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+
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.
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
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!

Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Fixed verified on 11-07 trunk build.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: