Closed
Bug 106536
Opened 23 years ago
Closed 23 years ago
Blank charset list in SaveAs charset of HTML composer
Categories
(Core :: Internationalization, defect)
Tracking
()
VERIFIED
FIXED
mozilla0.9.6
People
(Reporter: nhottanscp, Assigned: jbetak)
References
()
Details
(Keywords: regression)
Attachments
(2 files, 1 obsolete file)
19.54 KB,
image/jpeg
|
Details | |
796 bytes,
patch
|
Details | Diff | Splinter Review |
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?
Reporter | ||
Updated•23 years ago
|
Keywords: regression
Reporter | ||
Comment 1•23 years ago
|
||
Assignee | ||
Comment 2•23 years ago
|
||
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
Reporter | ||
Comment 5•23 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 : 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
Reporter | ||
Comment 6•23 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? 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 }
Assignee | ||
Comment 7•23 years ago
|
||
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 ;-)
Assignee | ||
Comment 8•23 years ago
|
||
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. ***
Assignee | ||
Comment 10•23 years ago
|
||
Reporter | ||
Comment 11•23 years ago
|
||
Juraj, does the patch fix the problem? If so, please get reveiews and check in for 0.9.6.
Assignee | ||
Comment 12•23 years ago
|
||
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...
Assignee | ||
Comment 13•23 years ago
|
||
cmanske: could you please r= this change?
Comment 14•23 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•23 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. r=cmanske if this type of fix is ok with XPFE folks.
Assignee | ||
Comment 16•23 years ago
|
||
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•23 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?
Assignee | ||
Comment 18•23 years ago
|
||
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=?
Assignee | ||
Comment 19•23 years ago
|
||
Comment on attachment 56219 [details] [diff] [review] proposed patch v1 M096 branch only
Attachment #56219 -
Flags: review+
Comment 20•23 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.
Assignee | ||
Comment 21•23 years ago
|
||
Assignee | ||
Comment 22•23 years ago
|
||
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 :-)
Assignee | ||
Updated•23 years ago
|
Attachment #56219 -
Attachment is obsolete: true
Comment 23•23 years ago
|
||
Fix the indentation (the lines under the if statements should be indented 2 spaces). sr=hyatt
Assignee | ||
Comment 24•23 years ago
|
||
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
You need to log in
before you can comment on or make changes to this bug.
Description
•