Closed Bug 102026 Opened 23 years ago Closed 23 years ago

Charset menu misplacing (.)

Categories

(Core :: Internationalization, defect, P1)

defect

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)

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.
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 (.)
Was this working before? If so, please add "regression" keyword.
yes, this had been working previously.
Keywords: regression
accepting (regression)
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla0.9.5
tracy/ylong: can you be more specific? I need to reproduce this bug.
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.
Tracy: I'll mark this as WFM. Please reopen if you disagree.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → WORKSFORME
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
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 → ---
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
ccing conrad.
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-]
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 }
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.
>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.
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?
m0.9.5 is out . too late. mark as m0.9.6
Target Milestone: mozilla0.9.5 → mozilla0.9.6
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
as per ftang's suggestion. -> jbetak
Assignee: yokoyama → jbetak
Status: REOPENED → NEW
accepting for M096. Tentatively marking dependency on bu 64146.
Status: NEW → ASSIGNED
Depends on: 64146
Depends on: 104302
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...
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.
Blocks: 107065
Keywords: nsbranch
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.
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.
Is that possible to initialize gCharsetMenu as null then instanciate it in the listener? Would that make it efficient?
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.
In UpdateCurrentCharset(), what if gLastBrowserCharset is null, or is that already been set in somewhere else?
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.
Comment on attachment 55808 [details] [diff] [review] improved patch - nhotta would you feel comfortable giving an r=? r=nhotta
Attachment #55808 - Flags: review+
sspitzer: could you help us out with a sr? this is very similar to bug 67003 you worked on.
Whiteboard: [PDT-] → [PDT-] need sr
Whiteboard: [PDT-] need sr → [PDT-] requested sr by email on 11/02/2001
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+
resolved at last - thanks everyone!
Status: ASSIGNED → RESOLVED
Closed: 23 years ago23 years ago
Resolution: --- → FIXED
*** Bug 98625 has been marked as a duplicate of this bug. ***
*** Bug 104474 has been marked as a duplicate of this bug. ***
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.

Attachment

General

Created:
Updated:
Size: