Closed Bug 31708 Opened 24 years ago Closed 24 years ago

[nsbeta2] No check mark on selected character coding menu items

Categories

(Core :: Internationalization, defect, P3)

x86
All
defect

Tracking

()

CLOSED DUPLICATE of bug 11774

People

(Reporter: blee, Assigned: cata)

References

Details

(Whiteboard: [nsbeta2+] fix in hand, all JS, see patch in the bug description)

To see this,
Upon loading a page, select a charset from charset group sub menu (top)
of under View| Character Coding and go back to see the same menu content.

==> The selected charset appears at the bottom of Character Coding submenu
    under Western (ISO-8859-1), but there's no check mark next to it, indicating
    the current selection of that menu item.

Win and Mac specific. Fine in Linux.
blds tested: Win 03-13-14-M15, Mac 03-13-08-M15-nb1b, Linux 03-13-03-M15-nb1b
Keywords: pp
QA Contact: teruko → blee
cata- can you take a look at this ?
Assignee: ftang → cata
Status: NEW → ASSIGNED
Target Milestone: M15
Target Milestone: M15 → M17
Keywords: beta2
blee,  Is this still a problem?
This still happens in Win 040415-M15 & JAb1(033112) and Mac 040410-M15 blds.
Keywords: nsbeta2
blee, can you try in M16?
Keywords: beta2
Still happens in Win 04-21-09M16, Mac 04-21-10M16, Linux 04-21-09M16 blds.
Fixed.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Not fixed yet in Win 04-26-11-M16 bld.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Putting on [nsbeta2-] radar. Not critical to beta2.  But please continue to try 
to fix by 5/16 per lyecies.
Whiteboard: [nsbeta2-]
Status: REOPENED → ASSIGNED
Work-around: force the focus to be on the HTML document window. One way to get 
that is by selecting some text in the window. 

The problem is that my JS code that is supposed to return the focused HTML 
window, returns the focused "any" window! That usually means the Location Bar 
(the focus may be there because people are editing there and the carret is 
there), whose charset is UTF-8. We try to check this one, but it is not in the 
menu. Then we end up with no check on the menu or the check left on the previous 
charset.

I will create a bug assigned to Hyatt and make this one dependent on it.
Depends on: 39776
*** Bug 39312 has been marked as a duplicate of this bug. ***
I have a fix for this, thanks to David Hyatt:

S:\mozilla\xpfe\global\resources\content>cvs diff -c *.js
Index: charsetOverlay.js
===================================================================
RCS file: /cvsroot/mozilla/xpfe/global/resources/content/charsetOverlay.js,v
retrieving revision 1.11
diff -c -r1.11 charsetOverlay.js
*** charsetOverlay.js   2000/05/17 06:06:02     1.11
--- charsetOverlay.js   2000/05/18 22:28:17
***************
*** 98,104 ****

  function UpdateCurrentCharset()
  {
!     var charset = document.commandDispatcher.focusedWindow.document.characterS
et;
        dump("Update current charset: " + charset + "\n");

      var menuitem = document.getElementById('charset.' + charset);
--- 98,107 ----

  function UpdateCurrentCharset()
  {
!     var wnd = document.commandDispatcher.focusedWindow;
!     if (window == wnd) wnd = window.content;
!
!     var charset = wnd.document.characterSet;
        dump("Update current charset: " + charset + "\n");

      var menuitem = document.getElementById('charset.' + charset);

Bob, how can we get this reevaluated and checked in? It is quite an important 
bug...
Whiteboard: [nsbeta2-] → [nsbeta2-] I have fix in my tree, you can see the diff here
*** Bug 39776 has been marked as a duplicate of this bug. ***
Re-nominated for nsbeta2.
No user feedback is a bad usability bug.
Safe and small JS fix -- see diff above.
Whiteboard: [nsbeta2-] I have fix in my tree, you can see the diff here → I have fix in my tree, you can see the diff here
I would also support nsbeta2+ nomination, this bug has two more quite serious 
dependents. I verified the code patch and it would fix three bugs. 

36136: view source on pages with meta tags is currently broken, this alone was 
       nominated PDT+ for Beta1

37543: we are not able to select the proper encoding for nested, unlabeled        
       frames and iFrames, this is quite serious since we partially render 
       "garbage" for some top international sites

Some comments for Cata & Hyatt:

hey - I believe we should also reset the state of the browser instance proxy 
object. The menu is changing its state OK, but that's just a superficial state 
information, if I'm not missing anything, then the browser core also needs to be 
set accordingly. I verified both depended bugs with the modified patch; we might 
need to polish the appCore creation, when we decide to go for it.


cvs diff -c charsetOverlay.js 
Index: charsetOverlay.js
===================================================================
RCS file: /cvsroot/mozilla/xpfe/global/resources/content/charsetOverlay.js,v
retrieving revision 1.11
diff -c -r1.11 charsetOverlay.js
*** charsetOverlay.js 2000/05/17 06:06:02 1.11
--- charsetOverlay.js 2000/05/19 01:43:08
***************
*** 1,3 ****
--- 1,15 ----
+ var appCore = null;
+ 
+ function createBrowserInstance()
+ {
+     appCore = Components
+                 .classes[ 
"component://netscape/appshell/component/browser/instance" ]
+                   .createInstance( Components.interfaces.nsIBrowserInstance );
+     if ( !appCore ) {
+         alert( "Error creating browser instance\n" );
+     }
+   }
+ 
  function MultiplexHandler(event)
  {
    var node = event.target;
***************
*** 98,110 ****
  
  function UpdateCurrentCharset()
  {
!     var charset = 
document.commandDispatcher.focusedWindow.document.characterSet;
   dump("Update current charset: " + charset + "\n");
  
      var menuitem = document.getElementById('charset.' + charset);
  
      if (menuitem) {
          menuitem.setAttribute('checked', 'true');
      }
  }
  
--- 110,157 ----
  
  function UpdateCurrentCharset()
  {
!     
!   var wnd = document.commandDispatcher.focusedWindow;
!   if (window == wnd) wnd = window.content;
! 
!   try 
!   {
!     createBrowserInstance();
! 
!     // Initialize browser instance..
!     appCore.setWebShellWindow(window);
!     if ( window.content ) 
!     {
!         dump("Setting content window\n");
!         appCore.setContentWindow( window.content );
!     }
!   }
! 
!   catch(ex) 
!   {
!       dump("Failed to create and initialiaze the AppCore...\n");
!   }
!     
!   var charset = wnd.document.characterSet;
   dump("Update current charset: " + charset + "\n");
  
      var menuitem = document.getElementById('charset.' + charset);
  
      if (menuitem) {
          menuitem.setAttribute('checked', 'true');
+ 
+         if (appCore != null) 
+         {
+           dump("*** SetDocumentCharset(" + charset + ")\n");
+           appCore.SetDocumentCharset(charset);
+         } 
+ 
      }
  }
  
***************
*** 131,136 ****
--- 178,184 ----
  
  function UpdateMenus(event)
  {
+     dump("Going to update current charset...\n");
      UpdateCurrentCharset();
      UpdateCharsetDetector();
  }

*****CVS exited normally with code 1*****

Blocks: 36136, 37543
Whiteboard: I have fix in my tree, you can see the diff here → fix in hand, all JS, see patch in the bug description
Putting on [nsbeta2-] radar. 
Whiteboard: fix in hand, all JS, see patch in the bug description → [nsbeta2-]fix in hand, all JS, see patch in the bug description
If the fix for this one fixes another +...let us know..we would make this a 
plus.  msanz to follow up.
Renominated.  This would also fix these bugs marked nsbeta2+:
   bug 36136
   bug 37543

Whiteboard: [nsbeta2-]fix in hand, all JS, see patch in the bug description → fix in hand, all JS, see patch in the bug description
As this bug is now renominated for beta2, I cleaned up the patch, and attached 
it bellow. It is a very low risk fix. Practically, all I do is add a fallback 
case. Very, very small - a one-liner. The gain on the other side is quite 
great. We really need the checkmark in the charset menu for providing feedback 
to the user. Without that, the users will be unable to exercise and test 
certain charset related features, critical for the international product. 

Here's the patch:

  function UpdateCurrentCharset()
  {
!     var charset = 
document.commandDispatcher.focusedWindow.document.characterSet;
        dump("Update current charset: " + charset + "\n");

      var menuitem = document.getElementById('charset.' + charset);
--- 98,107 ----

  function UpdateCurrentCharset()
  {
!     var wnd = document.commandDispatcher.focusedWindow;
!     if (window == wnd) wnd = window.content;
!
!     var charset = wnd.document.characterSet;
        dump("Update current charset: " + charset + "\n");

      var menuitem = document.getElementById('charset.' + charset);

*** Bug 41041 has been marked as a duplicate of this bug. ***
*** Bug 41042 has been marked as a duplicate of this bug. ***
No, it is not a PP bug, it appears on all platforms. Renominating again, as it's 
been over week and nobody examined this bug. What do I have to do to get this 
bug reviewed?
Severity: normal → major
Keywords: pp
OS: Windows 2000 → All
Summary: No check mark on selected character coding menu items → [nsbeta2] No check mark on selected character coding menu items
[nsbeta2+]
Whiteboard: fix in hand, all JS, see patch in the bug description → [nsbeta2+] fix in hand, all JS, see patch in the bug description
Fixed...
Status: ASSIGNED → RESOLVED
Closed: 24 years ago24 years ago
Resolution: --- → FIXED
Reopening - fixed in Win32(06-07-09-M17), but not quite in Mac(06-07-20-M17).
One way to see the problem in Mac:
1) Select EUC-KR on a JA page. 
"Korean" shows up with check mark at the bottom of Charcter Coding submenu.
2) Now select the proper JA charset from Multibyte submenu items.
==> The Japanese charset is added to the bottom of Charcter Coding submenu,
but the check mark still stays with "Korean". 
This happens whenever the charset is switched from submenu (under Multibyte), 
but doesn't when the charset is selected from dynamically displayed/added items.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → ASSIGNED
At this point, this is a duplicate of #11774.

*** This bug has been marked as a duplicate of 11774 ***
Status: ASSIGNED → RESOLVED
Closed: 24 years ago24 years ago
Resolution: --- → DUPLICATE
Closing for now.
Status: RESOLVED → CLOSED
You need to log in before you can comment on or make changes to this bug.