Closed
Bug 102026
Opened 23 years ago
Closed 23 years ago
Charset menu misplacing (.)
Categories
(Core :: Internationalization, defect, P1)
Core
Internationalization
Tracking
()
VERIFIED
FIXED
mozilla0.9.6
People
(Reporter: tracy, Assigned: jbetak)
References
Details
(Keywords: intl, regression, Whiteboard: [PDT-] requested sr by email on 11/02/2001)
Attachments
(2 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
nhottanscp
:
review+
kinmoz
:
superreview+
|
Details | Diff | Splinter Review |
seen on recent builds (trunk and branch)
-using an existing profile, browse international encoded pages
-check the character encoding by going to View | Character Encoding
after some time (browsing several different pages) the dot denoting which
charset is selected begins to be placed next to the incorrect encoding for that
page. sometimes changing it manually works. Other times the the dot will remain
on the wrong place (this especially happens if unicode UTF-8 happens to be the
one the dot is placed on) This effect does not affect the rendering of the
page. The browser displays the pages correctly even after this affect begins to
take place. We have discovered that this is rather diffucult to reproduce
initially. I ran with a new profile for a few days of smoketesting before it
began to occur again. But it did occur again on all platforms. Clearing cache
had no effect. And the prefs.js file user-pref "intl.charsetmenu.browser.cache"
seems to be fine too.
Comment 1•23 years ago
|
||
Seems this problem is charset menu dot is not marked correctly, the pages loading fine.
Keywords: intl
QA Contact: andreasb → ylong
Summary: Charset menu misplacing (.) → Charset menu misplacing (.)
Comment 2•23 years ago
|
||
Was this working before? If so, please add "regression" keyword.
Comment 4•23 years ago
|
||
accepting (regression)
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla0.9.5
Comment 5•23 years ago
|
||
tracy/ylong: can you be more specific? I need to reproduce this bug.
Comment 6•23 years ago
|
||
Basically I can not get this reproduced on my machines even use an old profile.
You might want go to Tracy's place to see the problem and the profile data, I
guess the profile that he is using maybe got this feature broken by some other
smoke test feature(s), but we don't know which one though.
Comment 7•23 years ago
|
||
Tracy: I'll mark this as WFM. Please reopen if you disagree.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → WORKSFORME
Reporter | ||
Comment 8•23 years ago
|
||
okay...I havent seen this on windows for two days and it wasn't on mac
today...*shrug* beats me what's going. I'll try to reproduce on yuying's
machines.
for now, verifying wfm
Status: RESOLVED → VERIFIED
Comment 9•23 years ago
|
||
Re-open, cause I, Xianglan and Tracy saw this problem recently, I'll send my
problematic profile to Roy by email for the investigation maybe on debug build,
and meanwhile I'll work on to get a reproduce steps.
Status: VERIFIED → REOPENED
Resolution: WORKSFORME → ---
Comment 10•23 years ago
|
||
Sent the problematic profile to Roy.
Add nsBranch keyword for envaluation, I think it's a serious prolem in real
world, a user always use same profile, and they won't get the charset marked
correctly once this got corrupt.
Keywords: nsbranch
Comment 11•23 years ago
|
||
ccing conrad.
Comment 12•23 years ago
|
||
seems that the functionality is working correctly, but the usability is poor.
PDT- per PDT, w/comsultation with Product Marketing. Pls re-nominate if you
truly believe this is a stop ship.
Whiteboard: [PDT-]
Comment 13•23 years ago
|
||
I see the charset menu is set to UTF8.
It may be picking up the charset of chrome window.
We may have a problem of getting a correct focusedWindow
charset as seen in line 123.
http://lxr.mozilla.org/seamonkey/source/xpfe/global/resources/content/charsetOverlay.js#123
121 function UpdateCurrentCharset()
122 {
123 var wnd = document.commandDispatcher.focusedWindow;
124 if ((window == wnd) || (wnd == null)) wnd = window._content;
125 126 var charset = wnd.document.characterSet;
127 var menuitem = document.getElementById('charset.' + charset);
128 129 if (menuitem) {
130 menuitem.setAttribute('checked', 'true');
131 }
132 }
Comment 14•23 years ago
|
||
Do we have a solution that works with the cause identified?
It is pretty bad to have an incorrect marking on the charset
menu. It confuses the user if the user has to engage a manual
override. We don't want users devleoping mistrust on the Character
Coding menu. For this correct feedback is essential.
I have seen this problem from time to time and frequently enough
that it should be fixed for the branch if there is time and the
solution is not complex or risky.
Assignee | ||
Comment 15•23 years ago
|
||
>I see the charset menu is set to UTF8.
>It may be picking up the charset of chrome window.
Roy - as discussed offline - let comment here as well that we've seen similar
things intermittently in the past. I remember e.g. that Frank reported similar
behavior after we implemented the charset "inheritance" mechanism, where a
child window should be initiated using its parent's charset. He complained
about the child window sometimes being set to UTF-8. The problem then surfaced,
faded and resurfaced again.
Perhaps we should get someone from XPFE involved and stop using the "focus
window" for our purposes - something they recommended a year ago or so. I did a
quick query and found several bugs that got addressed recently and might have
changed the app behavior: bug 97067, bug 91884 and bug 77675.
Comment 16•23 years ago
|
||
jbetak: thanks for your input.
I was digging little more on the charsetOverlay.js and
shouldn't we be using
function UpdateCurrentCharset()
{
- var wnd = document.commandDispatcher.focusedWindow;
- if ((window == wnd) || (wnd == null)) wnd = window._content;
-
- var charset = wnd.document.characterSet;
+ var charset = window._content.document.characterSet;
var menuitem = document.getElementById('charset.' + charset);
I guess my question is
shouldn't we always display the _content.docuement.characterSet, not
focused window?
Comment 17•23 years ago
|
||
m0.9.5 is out . too late. mark as m0.9.6
Target Milestone: mozilla0.9.5 → mozilla0.9.6
Comment 18•23 years ago
|
||
I thought that using _content.document.characterSet fixes
this issue instead of using commandDispatcher.focusedWindow
(see my comment 2001-10-11 09:56)
However after further digging, www.yahoo.com for instance will still
result to UTF-8. (www.yahoo.com doesn't have HTTP charset header nor
charset meta tag. )
Could someone shed light for me?
== cc'ing waterson, don, rick
Comment 19•23 years ago
|
||
as per ftang's suggestion.
-> jbetak
Assignee: yokoyama → jbetak
Status: REOPENED → NEW
Assignee | ||
Comment 20•23 years ago
|
||
accepting for M096. Tentatively marking dependency on bu 64146.
Status: NEW → ASSIGNED
Depends on: 64146
Assignee | ||
Comment 21•23 years ago
|
||
Assignee | ||
Comment 22•23 years ago
|
||
the last patch should improve both page load times and get rid of the "focused
window" annoyance. We still might be hurt be the fact that a window might lose
focus upon finishing its rendering, in such a case "var charset =
window._content.document.characterSet;" will result in the same problems we've
seen in the past.
I talked to ftang and nhotta and it seems reasonable to come up with a new
mechanism to handle this. The plan is to cache the charset menu content instead
of eagerly writing it out to preferences, we could store the current charset in
a similar fashion. The idea is to make this available through an interface,
which will be hopefully less ambiguous than finding the currently focused
window...
Assignee | ||
Comment 23•23 years ago
|
||
please note that when a page loads incorectly just once - the results of the
last used charset will be stored both in bookmarks (if the page has been
bookmarked) and in the cache. Botch cache and the bookmarks should be purged
when testing to make sure that the page is not affected by its past loading
results. THis is critical for pages w/o any charset information and w/o HTTP
header charset.
Assignee | ||
Comment 24•23 years ago
|
||
adding nhotta and sspitzer for potential r/sr
since they've implemented a similar approach for mailnews. Could you guys
please review this? I'm trying to get rid of the need to locate the currently
focused window and improve performance at the same time.
Assignee | ||
Comment 25•23 years ago
|
||
nhotta, sspitzer:
as already mentioned earlier, this is very similar to bug 67003 you both worked
on back in January - just wanted to point the bug number to you.
Comment 26•23 years ago
|
||
Is that possible to initialize gCharsetMenu as null then instanciate it in the
listener? Would that make it efficient?
Assignee | ||
Comment 27•23 years ago
|
||
thanks for looking at this nhotta! I do believe that the proposed approach is
viable - chrome JS is capable of keeping global variables on a per-window
basis. And we do save a few cycles by checking whether the charset has changed,
since we can avoid this call for almost all page loads:
- menu.SetCurrentCharset(charset);
The win is even more substantial for pages with frames, since we appear to get
a callback for each loaded frame. The small win then can compound to a larger
win for the overall page load time.
Anyhow, I just wanted to drop the dependency of finding the currently focused
window - if we can save a few cycles along the way, I think that would be great
news.
Comment 28•23 years ago
|
||
In UpdateCurrentCharset(), what if gLastBrowserCharset is null, or is that
already been set in somewhere else?
Assignee | ||
Comment 29•23 years ago
|
||
oh I see now what you were concerned about. UpdateCurrentCharset() is called
when someone is about to look at the charset menu. The patch assumes that a
pageload is performed before anyone does that, which is not necessarily bad. I
could include a fall back using the old (and somewhat fragile) code in for the
case that this condition is somehow not satisfied.
Assignee | ||
Comment 30•23 years ago
|
||
Comment 31•23 years ago
|
||
Comment on attachment 55808 [details] [diff] [review]
improved patch - nhotta would you feel comfortable giving an r=?
r=nhotta
Attachment #55808 -
Flags: review+
Assignee | ||
Comment 32•23 years ago
|
||
sspitzer:
could you help us out with a sr? this is very similar to bug 67003 you worked
on.
Whiteboard: [PDT-] → [PDT-] need sr
Assignee | ||
Updated•23 years ago
|
Whiteboard: [PDT-] need sr → [PDT-] requested sr by email on 11/02/2001
Comment 33•23 years ago
|
||
Comment on attachment 55808 [details] [diff] [review]
improved patch - nhotta would you feel comfortable giving an r=?
rs=kin@netscape.com
Attachment #55808 -
Flags: superreview+
Assignee | ||
Comment 34•23 years ago
|
||
resolved at last - thanks everyone!
Status: ASSIGNED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 35•23 years ago
|
||
*** Bug 98625 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 36•23 years ago
|
||
*** Bug 104474 has been marked as a duplicate of this bug. ***
Comment 37•23 years ago
|
||
I haven't seen this on recently trunk build, mark it as verified for now.
Please re-open in case it come back.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•