Closed Bug 73881 Opened 23 years ago Closed 23 years ago

Menu checkmark drawing problem in "Character Codiing" menu

Categories

(Core :: Internationalization, defect)

x86
Windows NT
defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla0.9.1

People

(Reporter: wesleyg, Assigned: nhottanscp)

References

Details

(Keywords: intl, regression, Whiteboard: Ready for check-in)

Attachments

(2 files)

<summary>
Default character coding is not indicated in the list for a new message.

<repro environment>
build 03-28-06 + NT4-JA

Not repro on:
build 03-28-06 + Mac OS 9.04-JA

<repro steps>
-open mail
-open a new message
-go to view|character coding

<result>
There is no indicator for the character coding setting.

<expected result>
There should be a dot next to the default character coding setting (which is 
specified in the Message Composition Preferences dialog).

Note:
If you manually make a character coding selection from the compose window,  the 
selection is indicated in the list.
Please check when did this start to happen.
Status: NEW → ASSIGNED
Keywords: intl, regression
Target Milestone: --- → mozilla0.9
Summary: Default character coding is not indicated in the list for a new message. → Default character coding for message compose is not indicated in the list for a new message.
It repros on build 03-23-06, but not on build 3-21-06.
i've noticed that there is no charset indication when you view the message but
as soon as you move your mouse over the first item of the Encoding menu ( wcich
is Customized) you'll see the proper encoding ( wasn't not a behavior for
View|encoding) in previous builds ( prior to 03-28)
Same problem for auto-detect submenu.
Please also check for the same problem in browser.
Browser has the same problem.
Please check Linux, so far only Windows has this problem, Macintosh is fine.
Linux build has this problem too.
Severity: normal → major
Keywords: nsbeta1
Product: MailNews → Browser
This is a drawing problem, checkmark doesn't appear until the mouse move to
other menu items. Change component to XP Toolkit.
Regression, started sometime between 3/21 and 3/23.

Assignee: nhotta → pinkerton
Status: ASSIGNED → NEW
Component: Internationalization → XP Toolkit/Widgets: Menus
Summary: Default character coding for message compose is not indicated in the list for a new message. → Menu checkmark drawing problem
blake landed a big change on the 21st. smells a lot to me like his regression.
feel free to punt back to me if that's not the case.
Assignee: pinkerton → blakeross
Target Milestone: mozilla0.9 → ---
*** Bug 77187 has been marked as a duplicate of this bug. ***
Blake, what is a status of this bug? Looks like it's a regression caused by your 
XUL change.
Removed intl keyword. Lisa, could somebody in core QA take this? Thanks.
Keywords: intl
QA Contact: ji → lchiang
Adding intl back, it's happening in "Character Codiing" menu and its submenus.
Keywords: intl
Summary: Menu checkmark drawing problem → Menu checkmark drawing problem in "Character Codiing" menu
Then QA contact to me again.
QA Contact: lchiang → ji
*** Bug 76572 has been marked as a duplicate of this bug. ***
Blake - Can we get a nsbeta1+ and M0.9.1 for this regression. i18n needs it to 
be fixed.

Adding Peter Trudelle to cc: list.
I don't understand how this could be mine.  If related to my change at all, it 
would have to be something deeper that they exposed, I think.  Reassigning back 
to default owner for now, cc'ing Dean and hyatt; I'll be glad to take this if 
someone shows that it's me (I wasn't the only one that checked in between 3/21 
and 3/23...)
Assignee: blakeross → pinkerton
I just remembered bug 75516, which seems like it could be related.  Then again, 
this apparently regressed before things were changed over to the newer command 
system :-/
Blake, 

I applied the patch - it apparently doesn't fix the problem... I'm currently 
looking into a purely JS-based fix in UpdateCurrentCharset(). I'll keep you 
posted about any promising progress.

http://lxr.mozilla.org/seamonkey/source/xpfe/global/resources/content/charsetOve
rlay.js#121

Disclaimer: I work with Naoki for the i18n folks
Juraj: yes, I had the same results; that patch had no effect.  I'm not sure I 
understand the current charset code.  Why must you go through and manually 
check the appropriate items in UpdateCurrentCharset() if the menuitems have a 
checked attribute (based off an rdf datasource: 
checked="rdf:http://home.netscape.com/NC-rdf#Checked")?  Am I missing something?
Blake, good question. I don't know... I'll try to dig around nsCharsetMenu.cpp 
to see, whether it's an abandoned route. I think the checked attribute might be 
superfluous at this point.

http://lxr.mozilla.org/seamonkey/source/intl/uconv/src/nsCharsetMenu.cpp#623

In the meantime I experimented some more and it seems to be an issue with the 
firing of "oncreate". The first time around, we are not able to find the 
charset menuitem to be marked. 

After the popup menu has been successfully rendered once, the "oncreate" popup 
menu handler is able to locate the offending menuitem. I tried to 
use "oncommand" instead and observed the same effect. I just veried, that this 
worked earlier builds.

http://lxr.mozilla.org/seamonkey/source/xpfe/global/resources/content/charsetOve
rlay.js#129

var menuitem = document.getElementById('charset.' + charset);
*** Bug 76300 has been marked as a duplicate of this bug. ***
this smells like hyatt's changes.
Assignee: pinkerton → hyatt
I downloaded old builds from 2001-03-22-06 to 2001-03-23-15.
It was working until 2001-03-22-17, the problem started from 2001-03-23-15.

Here is all check in between 03/22/2001 18:19 and 03/23/2001 13:52

http://bonsai.mozilla.org/showcheckins.cgi?&treeid=SeaMonkey&batchid=496

Note that blake and hyatt did not checked in that period. So reassign to
pinkerton, please check if your change is related.
Assignee: hyatt → pinkerton
my checkin was just an if check to fix a crasher. w/out this, it would have 
crashed.
Mike, in that list do you see any checkin which might be related to this menu
problem?
when I went over bug fixes referring to the "oncreate" event I spotted bug 
73108 and thought that this might be a good candidate to research. I just saw 
it Naoki's checkin log and wanted to pass the tidbit on...  
looks like Pink's checkin for bug 73108 didn't cause this. I just verified this 
on a 2001-03-22 17:00 build. 

The next most relevant checkin in this time frame seems to be the fix for bug 
71895. Adding DanM to the cc list.
another quick update: it seems like the problem started ocurring after 21:00 on 
that fateful day 03/22, which rules out both pink's and danm's checkin. 
So Macintosh does not have this problem. One difference is that it uses a real
checkmark instead of a bullet.
moving the target after 03/23 1:30 am
OK, it happened in the wee hours on 03/23/2001, more specifically - between 
1:30 am and 6:00 am.

Here are the checkins during that time period:

http://bonsai.mozilla.org/cvsquery.cgi?
treeid=default&module=SeaMonkeyAll&branch=HEAD&branchtype=match&sortby=Date&hour
s=2&date=explicit&mindate=03%2F23%2F2001+01%3A30%3A00&maxdate=03%2F23%2F2001+06%
3A00%3A00&cvsroot=%2Fcvsroot

There was a big landing for bug 71530, which is a change to RDF. The charset 
menu is RDF-based, so this might another good candidate possibly due to delayed 
(lazy) menu content initialization...
adding waterson - might might need help with the checkin for bug 71530 
(Implement RDF outliner)
getting off my radar. 
Assignee: pinkerton → waterson
Bonsai didn't show waterson's check in somehow.
Anyway, I tried incremental builds for 3/23's tree on my machine.
02:00 build does not have the checkmark problem, it started from 03:00 build.
Only check in that period was waterson's RDF change, other check in was not a
part of the build.
tentatively marking dependence on bug 78290. Setting the checkmark attribute in 
the RDF source without the need to do it from JS would contribute to a cleaner 
design and enhanced performance.
Depends on: 78290
waterson, please set a milestone to 0.9.1
Keywords: mozilla0.9.1
Sorry, I can't, since this depends on, and is probably a dup of, bug 78290. When
bug 78290 is fixed, I will re-examine this.
Status: NEW → ASSIGNED
Okay, I'm kicking this one back over to I18n. If you apply the above patch, 
you'll see clearly that the nsCharsetMenu datasource is _never_ setting the 
http://home.netscape.com/NC-rdf#Checked property. 

I spent a bit of time grovelling through nsCharsetMenu.cpp, and as far as I can 
tell, nobody is calling nsCharsetMenu::SetCharsetCheckmark(). I set a breakpoint 
in that method, and never hit it.

So you've got to do something in your back-end to make this work.
Assignee: waterson → nhotta
Status: ASSIGNED → NEW
Component: XP Toolkit/Widgets: Menus → Internationalization
QA Contact: ji → andreasb
QA Contact: andreasb → ylong
waterson,

Cata has never hooked up the SetCharsetCheckmark() method outside of his 
private build, since he couldn't get it working right. I would be very keen on 
researching some of the points you raised and do some backend work in 
nsCharsetMenu. BTW is there a good way to dump (visualize) contents of an RDF 
source? Would module logging be enough?
NHotta/Ftang- It's getting kinda late for this one. We have a workaround, and 
should release note it. Recommend we push it off to M0.9.2, unless the code is 
done today (e.g. Looks like there is a lot of research still to be done though).

Marking as nsCatFood and RTM.
Keywords: nsCatFood, rtm
The patch uses the workaround suggested by waterson (see bug 78290) and delay 
checking menu. It fixes the problem on Windows and Linux, and Macintosh 
continues to work with the patch.
Asked waterson for a super review, set milestone to 0.9.1.

Mail composer menu needs a separate change, I plan to do that for 0.9.2.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.1
Add a comment that says why we're doing this hack, so we'll remember where to
fix it later. Do that, and sr=waterson
Whiteboard: Ready for check-in
Checked in.
Filed bug 82272 for the remaining mail compose problem.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Verified as fixed.
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: